1429881 - view-source: pages can be used to gain cross-origin access to restricted domains

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect, P2)

Description

[Abdulrahman Alqabandi]

Attachment: AMO.zip

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.132 Safari/537.36

Steps to reproduce:

1. Install attached extension
2. Click on the green browser action
3. Click on the button that appears

I noticed that the browserAction popup can execute any code on 'view-source:https://addons.mozilla.org/' and then I noticed that the view-source'd AMO page can link to the non-view-source'd one and has access to it. 

I use this to my advantage as the 'navigator.mozAddonManager' is not available in the view-source version but is in the normal web page one. 


Actual results:

We have access to navigator.mozAddonManager


Expected results:

Block view-source:https://addons.mozilla.org/ from being accessed by browser.tabs.executeScript (coming from browserAction popup) as it is blocked within 'https://addons.mozilla.org/' (without view-source)

Comment 1

[Abdulrahman Alqabandi]

This trick seems to work on view-source:about:cache not on system privileged ones though. However, you're not supposed to be able to automatically open view-source:about:cache as I have reported that in bug 1429379. So something weird is happening with view-source'd urls. 

Just to be clear, this is still a separate bug, as you are supposed to be able to open web urls using 'view-source', the report in bug 1429379 is in regards to opening file: and privileged about: pages.

Comment 2

[Abdulrahman Alqabandi]

Ok, I commented too soon. Even on 'about:cache' (no view source) browser.tabs.executeScript from browserAction popup works. Is this supposed to be allowed? I noticed executeScript doesn't work coming from a normal background page script. 

According to https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/executeScript 
"You can only inject code into pages whose URL can be expressed using a match pattern: meaning, its scheme must be one of "http", "https", "file", "ftp""

Though there is clearly an exception somewhere for browserAction popup scripts.

Comment 3

[Abdulrahman Alqabandi]

We can also inject JS on other extension pages using the browserAction popup background script.

Comment 4

[Kris Maglione [:kmag]]

We should really restrict mozAddonManager to SecureContexts. That should prevent this kind of abuse.

Comment 5

[Kris Maglione [:kmag]]

(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #4)
> We should really restrict mozAddonManager to SecureContexts. That should
> prevent this kind of abuse.

Apparently that's no longer the case, since bug 1410364 landed... which is unfortunate.

Boris, is there anything standards-compatible we can do to prevent a non-SecureContext opener from getting access to a SecureContext API? It would be nice if we could add an origin attribute to prevent cross-origin access in those cases, or something like that.

Comment 6

[Kris Maglione [:kmag]]

(In reply to Kris Maglione [:kmag] from comment #5)
> It would be nice if we could add an origin attribute to prevent cross-origin
> access in those cases, or something like that.

That clearly wouldn't work, since it would break communication between pages
that don't rely on SecureContext APIs at all. Hm.

Comment 7

[Kris Maglione [:kmag]]

So, given the above, I think we should probably do two things:

1) Consider adding a new SecureContext flag that considers the opener, which is basically the reverse of the old isSecureContextIfOpenerIgnored flag.

2) Give view-source: pages principals that don't allow them cross-origin access. That probably means null principals, which may have the unfortunate side-effect of breaking some links.

Comment 8

[:Gijs (he/him)]

(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #7)
> 2) Give view-source: pages principals that don't allow them cross-origin
> access. That probably means null principals, which may have the unfortunate
> side-effect of breaking some links.

Coincidentally, bz, bholley and me talked about this on Friday and we have a long-term plan for this, see bug 1171853. But we didn't think we needed to do anything in a great hurry... :-\

Null principals will break all links on non-http/https pages (e.g. view-source:file ). That seems pretty unfortunate, though if the alternative is a severe issue, I guess it would be survivable short-term... I'm not really able to assess how bad this bug in and of itself is and if it's severe enough to warrant such a short-term fix. Alternatively, we could expedite the other work... I don't think any of it is hugely complex in and of itself.

Comment 9

[:Gijs (he/him)]

Can't we just restrict webextensions from opening view-source: for anything other than http/https (bug 1429379), as well as restricting it from opening view-source: for any pages that have any "special" privileges (UITour, mozAddonsManager, whatever) ? If necessary, we can add that to checkloaduri and that should be enough assuming we get the right principals in there.

Comment 10

[:Gijs (he/him)]

Or, very-short-term, we could just back out bug 1261289. It'll make dveditz sad, but I think the alternative has been worse so far...

Comment 11

[Kris Maglione [:kmag]]

(In reply to :Gijs from comment #9)
> Can't we just restrict webextensions from opening view-source: for anything
> other than http/https (bug 1429379), as well as restricting it from opening
> view-source: for any pages that have any "special" privileges (UITour,
> mozAddonsManager, whatever) ? If necessary, we can add that to checkloaduri
> and that should be enough assuming we get the right principals in there.

We can. That's bug 1429379. But I'd rather we also fix the fact the the API winds up being accessible in this case at all. We already restrict it pretty severely so that it can only be accessed by top-level AMO windows and their sub-frames. We really should have taken the opener into account from the beginning.

Comment 12

[Boris Zbarsky [:bzbarsky]]

> I noticed that the browserAction popup can execute any code on 'view-source:https://addons.mozilla.org/'

Assuming that it can't execute code on https://addons.mozilla.org/ itself, that seems like a fundamental bug to me.  Our current security model for view-source: is that view-source:foo is same-origin with foo.  We're working on changing that, but it's not quite simple to do without breaking all sorts of stuff.

> Boris, is there anything standards-compatible we can do to prevent a non-SecureContext opener from getting
> access to a SecureContext API?

When the non-SecureContext opener is same-origin with the thing the SecureContext API is coming from?  Not really.

> It would be nice if we could add an origin attribute to prevent cross-origin access in those cases, or
> something like that.

We _could_ throw an origin attribute into all view-source: things, maybe.  That would still leave them same-origin with each other in some cases, but not same-origin with non-viewsource things, and would not affect CheckLoadURI checks, at least...

Comment 13

[Abdulrahman Alqabandi]

Is what I described in comment 2 a different bug I should file? It has nothing to do with view-source.

Comment 14

[:Gijs (he/him)]

(In reply to Abdulrahman Alqabandi from comment #13)
> Is what I described in comment 2 a different bug I should file? It has
> nothing to do with view-source.

Yes. Please file a separate bug in Toolkit :: WebExtensions :: somewhere. :-)

Comment 15

[Abdulrahman Alqabandi]

(In reply to :Gijs from comment #14)
> (In reply to Abdulrahman Alqabandi from comment #13)
> > Is what I described in comment 2 a different bug I should file? It has
> > nothing to do with view-source.
> 
> Yes. Please file a separate bug in Toolkit :: WebExtensions :: somewhere. :-)

Filed bug 1431371

Comment 16

[:Gijs (he/him)]

This bug is still open, but from looking at the comments I think all the individual components (being able to open view-source:about:*, running code on it) have been fixed elsewhere. Is that right? Is there something left to do here?

Comment 17

[Abdulrahman Alqabandi]

The original PoC indeed does not work anymore, however, 'view-source:addons.mozilla.org' can still access mozAddons by linking to 'addons.mozilla.org' which I believe is the purpose of this bug. Not sure what the plan is here, is this behavior intended or are we looking to make that not a thing?

Comment 18

[:Gijs (he/him)]

(In reply to Abdulrahman Alqabandi from comment #17)

The original PoC indeed does not work anymore, however, 'view-source:addons.mozilla.org' can still access mozAddons by linking to 'addons.mozilla.org' which I believe is the purpose of this bug. Not sure what the plan is here, is this behavior intended or are we looking to make that not a thing?

This is bug 1430257. See also https://bugzilla.mozilla.org/show_bug.cgi?id=1171853#c35 . TL;DR: yes we would like to change this but it hasn't been top of our priority list...

My understanding is that with add-ons as well as the web being unable to link to view-source:addons.mozilla.org, this bug isn't practically exploitable anymore (ie yes the PoC doesn't work anymore, but there isn't really a way to make it work from the same assumptions anymore, either (that we're aware of), right)?

Comment 19

[Abdulrahman Alqabandi]

(In reply to :Gijs (he/him) from comment #18)

this bug isn't practically exploitable anymore (ie yes the PoC doesn't work anymore, but there isn't really a way > to make it work from the same assumptions anymore, either (that we're aware of), right)?

Correct. As far as I'm aware this cannot be exploited anymore.

Comment 20

[Daniel Veditz [:dveditz]]

It'd be nice to make this bug depend on the things that actually fixed it, but marking FIXED anyway (rather than the usual WORKSFORME) because we have decided to award a bounty for it.