1278013 - (CVE-2016-5265) Same origin policy bypass in local document/Universal xss

Status: RESOLVED FIXED

(Core :: Networking: File, defect)

Description

[Abdulrahman Alqabandi]

Attachment: bypass.zip

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.102 Safari/537.36

Steps to reproduce:

This only works on local HTML files that have been extracted into a folder, a specially crafted .URL needs to be in the same folder as the locally executed HTML document. User interaction is required (a click)

I have attached a zip file which contains the PoC code, please extract both files into the same folder and open 'PoC.html' _locally_

Additionally, you should create a new file (and folder) located at 'C:/secret/secret.txt' 

Open the PoC file and you should see two anchor tag, one uses the .url bypass another doesnt. This is to clearly show there is a bypass.

On top of that, if we change the target file to a folder like 'C:/secret/' we will be able to read the list of files.

Works on FF-ESR 45.1.1 && Nightly 49.0a1 (2016-06-03)

I have not played with this much yet, so I'm not sure if more dangerous bypasses or other bugs could be performed using the .URL redirects. Will test more.


Actual results:

Using a .URL file as a redirector, it seems like it breaks the SOP protection. 


Expected results:

I believe local HTML files are only allowed to read files within the same folder the html file has been opened in. What ever protection mechanism exists needs to keep the .url file in mind and block the ability to read arbitrary files.

Comment 1

[Abdulrahman Alqabandi]

Attachment: Screenshot of bypass being used to read full directory

Comment 2

[Abdulrahman Alqabandi]

Attachment: PoC of reading facebook.com with full credentials (aka logged in)

Its now also possible to read any website with full credentials.

Tested on latest nightly.

The difference here is we are using two layers of redirection using the .URL files.

Comment 3

[Abdulrahman Alqabandi]

Attachment: Screenshot of using this bypass to read facebook whilst logged in

Comment 4

[Abdulrahman Alqabandi]

Note: Forgot to change the directories in the .URL files in both PoC's please edit them and change them according to which file you want to test the bypass on.

Also keep in mind the hashtag in both PoCs (ln~9 open(XXX,"qab") is crucial in this bug working, please dont remove but feel free to change the target to whatever youre testing.

Comment 5

[Abdulrahman Alqabandi]

Attachment: bypass3.zip

User interaction is not required.

The PoC above will automatically read the file page of C:// using an iframe without requiring the user to click anything. Please change the URL file according to the path where you unzipped the PoC.

Would like to see someone test this using other OS than windows, as the .URL file mentioned here is a Windows only feature. However, the same implementation is used on other OS's which I have no ability to test at the moment.

ref:
- https://help.ubuntu.com/community/UnityLaunchersAndDesktopFiles
- http://fileinfo.com/extension/webloc


(note: apologies for the constant updates, I usually report a bug before doing any further testing.)

Comment 6

[Abdulrahman Alqabandi]

It's worth pointing out this is probably more accurately a UXSS (universal xss,) as if you notice in the PoCs we actually end up with a window object that points to an external website (the q variable). So we have the ability to execute arbitrary javascript in that window.

And we can do things like execute q.document.body.innerHTML='<b>@qab</b>'; and completely take over that page.

Comment 7

[Abdulrahman Alqabandi]

Could you check this please?

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

I'm not familiar with the code that parses and loads .URL files. Daniel, do you know who may be a better person to redirect this to?

Comment 9

[Abdulrahman Alqabandi]

This is the only source code I could find that I think deals with .URL files. 

https://github.com/mozilla/system-addons/blob/master/netwerk/protocol/file/nsFileProtocolHandler.cpp

Comment 10

[Boris Zbarsky [:bzbarsky]]

So the basic issue is that we decide whether to inherit the principal based on the pre-redirect URI.  Then nsBaseChannel::Redirect copies over the loadinfo to the post-redirect channel, so we end up inheriting the principal into the new thing too.

Looks to me at first glance like this behavior introduced in the patch in bug 1099296....  Just to check, is it the case that Firefox 37 does not show this behavior?

Anyway, one possibly annoying thing here is that for something like XHR to a local file we probably _do_ want to propagate the "inherit the principal" bits, unless XHR just stamps on the principal of the result anyway (needs checking).  If we don't, then the obvious thing here is to not propagate the "inherit the principal" flag across this redirect.  Possibly restricted to the .url case, of course; need to check what other things go through nsBaseChannel::Redirect.

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Abdulrahman, would you be able to run mozregression to narrow down what changeset introduced this bug? More details about mozregression can be found here: http://mozilla.github.io/mozregression/

Comment 12

[Abdulrahman Alqabandi]

(In reply to Boris Zbarsky [:bz] from comment #10)

> Just to check, is it the case that Firefox 37 does not show
> this behavior?

Just downloaded FF 37.0 and it does not work.


(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11)
> Abdulrahman, would you be able to run mozregression to narrow down what
> changeset introduced this bug? More details about mozregression can be found
> here: http://mozilla.github.io/mozregression/

Yes I'll be happy to. Will reply when I pin point it.

Comment 13

[Abdulrahman Alqabandi]

Attachment: mozregression

Alright so I ran mozregression and it appears that the bug works on 'mozilla-central build: 2015-02-18' and does not work on 'mozilla-central build: 2015-02-17'
note that this is for nightly builds
Screenshot provided for what its worth.

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Thanks, that confirms it is bug 1099296.

Comment 15

[Steve Workman [:sworkman] (INACTIVE)]

Chris - can you look into this, please?

Comment 16

[Christoph Kerschbaumer [:ckerschb]]

Boris, I am slightly confused about what's going on here, maybe you can help me clarify so I can debug further.

I am using the PoC from comment 0 using a redir.desktop file instead of the redir.url file, which is, according to the internet, the equivalent on Ubuntu (also inlined that file underneath in case someone wants to reproduce the problem on Linux). I am launching the testfile using [1] and then click the link which points to redir.desktop. The URL gets redirected and the contents of [2] is displayed. That part is fine, besides the fact that the contents of [2] should not be displayed.


Now, the odd part: When I click the second link (which directly points to [2]) then the contents of [2] is also displayed. LoadInfo of the channel looks like the following:

> LoadInfo {
>  channelURI: file:///secret/secret.txt
>  triggeringPrincipal: file:///home/ckerschb/Desktop/sop_bug/bypass/PoC.html
>  contentPolicyType: 6 [TYPE_DOCUMENT]
>  securityMode: SEC_NORMAL
> }

Now my question, is my test setup wrong? I don't see what security mechanism would block that load. What am I doing wrong?

[1] file:///home/ckerschb/Desktop/sop_bug/bypass/PoC.html
[2] file:///secret/secret.txt
[3] http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#10726

%---snip----
[Desktop Entry]
Encoding=UTF-8
Name=secretLink
Type=Link
URL=file:///secret/secret.txt
Icon=text-html

Comment 17

[Boris Zbarsky [:bzbarsky]]

> I am using the PoC from comment 0 using a redir.desktop file instead of the redir.url file

Yes, looks to me like it should be morally equivalent.

> The URL gets redirected and the contents of [2] is displayed.

Displayed where?  When you click either link, I would expect a new tab to open showing the "secret" file.  The question is whether the contents of that "secret" file are also placed in the textarea in the original page.  _That_ should not happen.

Note that to test this properly you want one link or the other, but not both, to be present in your HTML: they have the same "id" and the testcase adds a click event listener to the _first_ element with that id in the document.  So really, there should be two testcases, one with each of the <a> elements.

> I don't see what security mechanism would block that load

Nothing should block the load.  It's the access to q.document.body.innerHTML that should be blocked, by same-origin checks.

Note that I expect you can replace file:///secret/secret.txt with http://example.org and end up reading the contents of random pages on the web using this.

Comment 18

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Boris Zbarsky [:bz] from comment #17)
> Displayed where?  When you click either link, I would expect a new tab to
> open showing the "secret" file.  The question is whether the contents of
> that "secret" file are also placed in the textarea in the original page. 
> _That_ should not happen.

Got you.
 
> Note that to test this properly you want one link or the other, but not
> both, to be present in your HTML

Thanks!

> Nothing should block the load.  It's the access to q.document.body.innerHTML
> that should be blocked, by same-origin checks.

Yes, I see. That's what confused me because the only call to CheckMayLoad() I got was for ChannelShouldInteritPrincipal within docshell [1], so I suppose there needs to be a second call to CheckMayLoad whenever we try to access the innterHTML.


Thanks for the clarification!

[1] http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#10726

Comment 19

[Boris Zbarsky [:bzbarsky]]

Having a call to CheckMayLoad any time we try to access the innerHTML is how we did things 10 years ago.  It's super-slow.

The way we do things now is that the two compartments have whatever principals they have, and the cross-compartment wrapper picks a security policy based on those principals.  The problem is that we're inheriting the principal when we shouldn't be, in this case.

Comment 20

[Abdulrahman Alqabandi]

I would like to point out that (in windows OS atleast) when you re-edit the .URL files URL which it should redirect to will still point to the URL that was on the pre-edited .URL file. Some some sort of cache is done with .URL files. The only way to 'refresh' the new edited .URL is to rename the .URL file itself.

Comment 21

[Abdulrahman Alqabandi]

Thinking more about the attack scenario, I think the fact that a victim would need to download a zip file then unzip and finally execute the malicious html file locally could be shortened greatly.

So after some more testing I think the best attack scenario would be to have the user save the page from an attacker controlled website locally and simply open the index.html file from the downloads page, we can also trick the browser to download and save a .URL file using the (I think by default) option of saving a web page complete.

All we need to do is reference a .URL file using something like:

<link rel="stylesheet" href="http://localhost/q.url" />

Despite the 'stylesheet' being invalid, its still saved when performing a complete web page save. (this might be another bug in itself)

The location of the .URL file locally is also easily obtained (usually inside the folder 'index_files') and the bypass could be even more easily exploited.


So to summarize:


1. Victim must visit attacker owned website and be prompted to save the page hitting (CTRL+S) and save the page in web complete type and finally open the downloaded html file locally.

2. All local files can be read, as well as full file listings by reading something like 'file://C://' (no user interaction)

3. Binary files can also be read if we use view-source:file://thing.exe

4. We can read external domains like 'https://mozilla.org' with credentials (user interaction _required_)

Comment 22

[Abdulrahman Alqabandi]

Filed Bug 1279126 for the 'save full web page' behavior described in Comment 21

Comment 23

[Christoph Kerschbaumer [:ckerschb]]

Boris, I am still having trouble figuring out what's going on with this bug. Running the example from comment 0 using the redir.url testcase I see the contents of secret.txt to be displayed within the textarea. (Using the regular link it's not displayed within the textarea). So at least I can reproduce the problem. However, I don't get any call to nsBaseChannel::Redirect().

Total there are two channels created when clicking solely the link which uses the redir.url:

NewChannelFromURIWithProxyFlagsInternal {
  channelURI: file:///Users/ckerschb/Desktop/sop_bug/bypass/secret.txt#1
  triggeringPrincipal: file:///Users/ckerschb/Desktop/sop_bug/bypass/PoC.html
  contentPolicyType: TYPE_DOCUMENT
  securityMode: SEC_NORMAL
  initalSecurityChecksDone: no
  enforceSecurity: no
}


NewChannelFromURIWithProxyFlagsInternal {
  channelURI: file:///Users/ckerschb/Desktop/sop_bug/bypass/secret.txt#1
  loadingPrincipal: SystemPrincipal
  triggeringPrincipal: SystemPrincipal
  contentPolicyType: TYPE_OTHER
  securityMode: SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL, 
  initalSecurityChecksDone: no
  enforceSecurity: no
}

Where the first originates from docshell and the second from browser.js [1] (see also stacktrace underneath). It seems the channel created within browser.js is only used to resolve the URI and not used otherwise. In general, if the initial channel would be redirected, then we should create a new channel internally somewhere, right? But I don't see any other channel creation besides the two listed above. Any suggestions what I could try next to debug this?


[1] http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#7109


%---snip----
0 NetUtil_newChannel(aWhatToLoad = [object Object]) ["resource://gre/modules/NetUtil.jsm":403]
    this = [object Object]
1 setURI(uri = [xpconnect wrapped nsIURI @ 0x11bfa7740 (native @ 0x18373e308)]) ["chrome://browser/content/browser.js":7109]
    this = [object Object]
2 updateIdentity(state = 4, uri = [xpconnect wrapped nsIURI @ 0x11bfa7740 (native @ 0x18373e308)]) ["chrome://browser/content/browser.js":6748]
    this = [object Object]
3 XULBrowserWindow.onSecurityChange(aWebProgress = [xpconnect wrapped (nsISupports, nsIDocShell, nsIWebNavigation, nsIInterfaceRequestor, nsIWebProgress, nsIDocumentLoader, nsILoadContext) @ 0x137d56160 (native @ 0x134868008)], aRequest = [xpconnect wrapped (nsISupports, nsIRequest, nsIChannel) @ 0x1837d6e20 (native @ 0x13138b030)], aState = 4) ["chrome://browser/content/browser.js":4593]
    this = [object Object]
4 callListeners(listeners = [object Object], args = [xpconnect wrapped (nsISupports, nsIDocShell, nsIWebNavigation, nsIInterfaceRequestor, nsIWebProgress, nsIDocumentLoader, nsILoadContext) @ 0x137d56160 (native @ 0x134868008)],[xpconnect wrapped (nsISupports, nsIRequest, nsIChannel) @ 0x1837d6e20 (native @ 0x13138b030)],4) ["chrome://browser/content/tabbrowser.xml":490]
    this = [object ChromeWindow]
5 _callProgressListeners(aBrowser = [object XULElement], aMethod = "onSecurityChange", aArguments = [xpconnect wrapped (nsISupports, nsIDocShell, nsIWebNavigation, nsIInterfaceRequestor, nsIWebProgress, nsIDocumentLoader, nsILoadContext) @ 0x137d56160 (native @ 0x134868008)],[xpconnect wrapped (nsISupports, nsIRequest, nsIChannel) @ 0x1837d6e20 (native @ 0x13138b030)],4) ["chrome://browser/content/tabbrowser.xml":505]
    this = [object XULElement]
6 mTabProgressListener/<._callProgressListeners([object XULElement], "onSecurityChange") ["chrome://browser/content/tabbrowser.xml":560]
    this = [object Object]
7 mTabProgressListener/<.onSecurityChange(aWebProgress = [xpconnect wrapped (nsISupports, nsIDocShell, nsIWebNavigation, nsIInterfaceRequestor, nsIWebProgress,(gdb)

Comment 24

[:Gijs (he/him)]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #23)
> Boris, I am still having trouble figuring out what's going on with this bug.
> Running the example from comment 0 using the redir.url testcase I see the
> contents of secret.txt to be displayed within the textarea. (Using the
> regular link it's not displayed within the textarea). So at least I can
> reproduce the problem. However, I don't get any call to
> nsBaseChannel::Redirect().
> 
> Total there are two channels created when clicking solely the link which
> uses the redir.url:
> 
> NewChannelFromURIWithProxyFlagsInternal {
>   channelURI: file:///Users/ckerschb/Desktop/sop_bug/bypass/secret.txt#1
>   triggeringPrincipal: file:///Users/ckerschb/Desktop/sop_bug/bypass/PoC.html
>   contentPolicyType: TYPE_DOCUMENT
>   securityMode: SEC_NORMAL
>   initalSecurityChecksDone: no
>   enforceSecurity: no
> }

I defer to Boris on the details here, but in case this is easy to check: what's the channel's originalURI?

Comment 25

[Boris Zbarsky [:bzbarsky]]

> However, I don't get any call to nsBaseChannel::Redirect().

Is this in an e10s build?  Are you checking in the right process?  The way .url files work is that nsFileChannel::OpenContentStream is called, does ReadURLFile, gets an nsIURI, creates a new channel via NS_NewChannel, hands that channel out.  The caller of OpenContentStream is nsBaseChannel::BeginPumpingData which sees a non-null channel and does a RedirectRunnable, and RedirectRunnable::Run calls nsBaseChannel::HandleAsyncRedirect which calls BaseChannel::Redirect.  So if the .url file is working at all, you should be hitting all those things.  I don't know whether that would happen in the child process or parent process in this case.

Comment 26

[Abdulrahman Alqabandi]

Any update on this? Is this a sec-high bug?

Comment 27

[Christoph Kerschbaumer [:ckerschb]]

On MacOS using the setup including the .url file I get:

a) same origin .url redirect
   * displays content of secret file in the textarea
   * nsBaseChannel::Redirect() is *not* hit at all; neither using e10s nor when using regular mode.

b) cross origin .url redirect:
   * does *not* display content of secret file in textarea
   * nsBaseChannel::Redirect() is *not* hit at all; neither using e10s nor when using regular mode.
   * in the console I see:
     || Error: Permission denied to access property "document"
     || qAnchor.onclick/</<()
     || PoC.html:11


The .url redirection seems to work, but I don't get a call to nsBaseChannel::Redirect(). I put a MOZ_ASSERT() within that function which should crash the browser in regular mode and crash the tab in e10s mode.

I don't know where to go from here, probably it's wise if someone who has a windows machine handy can try to reproduce the issue. I am really happy to debug more, but I don't know what I could try next.

Comment 28

[Boris Zbarsky [:bzbarsky]]

> On MacOS using the setup including the .url file

Uh... what setup including the .url file on MacOS?  We only do interesting things with .url files on Windows.  On Mac, we _do_ do the .desktop thing.

On Mac, If I just use the original testcase attached to this bug and change from using a .url file to using a .desktop file so I'm actually testing something meaningful, then open a non-e10s window and click the link to the .desktop file from the testcase, my breakpoint in nsBaseChannel::Redirect gets hit, as expected.

I have no idea what you mean by "same origin .url redirect" and "cross origin .url redirect" in comment 27...

I guess I'll attach my testcase using a .desktop file, just in case that's useful.

Comment 29

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase that you can just unzip and run on Mac

Clicking the first link in that testcase _will_ trigger nsBaseChannel::Redirect for you on Mac.  It really will.

Comment 30

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Boris Zbarsky [:bz] (Out June 25-July 6) from comment #29)
> Clicking the first link in that testcase _will_ trigger
> nsBaseChannel::Redirect for you on Mac.  It really will.

Thanks for the clarification - hitting nsBaseChannel::Redirect now.

(In reply to Boris Zbarsky [:bz] (Out June 25-July 6) from comment #10)
> then the obvious thing here is to not propagate the
> "inherit the principal" flag across this redirect.

What 'inherit the principal' flag are you referring to exactly? As far as I can tell this is a TYPE_DOCUMENT load (see info underneath) for which we don't pass any 'securityMode' other than SEC_NORMAL which shouldn't influence principal inheritance, right?

(Please note that for TYPE_DOCUMENT loads we are not passing a loadingPrincipal, only a TriggeringPrincipal).

NewChannelFromURIWithProxyFlagsInternal {
  channelURI: file:///Users/ckerschb/Desktop/sop_bug/bypass_boris/redir.desktop
  triggeringPrincipal: file:///Users/ckerschb/Desktop/sop_bug/bypass_boris/PoC.html
  contentPolicyType: 6
  securityMode: 
  initalSecurityChecksDone: no
  enforceSecurity: no
}

NewChannelFromURIWithProxyFlagsInternal {
  channelURI: file:///private/etc/hosts
  loadingPrincipal: SystemPrincipal
  triggeringPrincipal: SystemPrincipal
  contentPolicyType: 1
  securityMode: SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL, 
  initalSecurityChecksDone: no
  enforceSecurity: no
}

nsBaseChannel::Redirect {
  channelURI: file:///private/etc/hosts
  finalURI  : file:///private/etc/hosts
  triggeringPrincipal: file:///Users/ckerschb/Desktop/sop_bug/bypass_boris/PoC.html
  contentPolicyType: 6
  securityMode: 
Assertion failure: false, at /Users/ckerschb/Documents/moz/mc/netwerk/base/nsBaseChannel.cpp:160
#01: nsBaseChannel::Redirect(nsIChannel*, unsigned int, bool)[/Users/ckerschb/Documents/moz/mc-obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x30634c]
#02: nsBaseChannel::HandleAsyncRedirect(nsIChannel*)[/Users/ckerschb/Documents/moz/mc-obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x306e1c]
#03: nsBaseChannel::RedirectRunnable::Run()[/Users/ckerschb/Documents/moz/mc-obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x31f520]

Comment 31

[Boris Zbarsky [:bzbarsky]]

> What 'inherit the principal' flag are you referring to exactly?

The one set at https://dxr.mozilla.org/mozilla-central/rev/51377a64158941f89ed73f388ae437cfa494c030/docshell/base/nsDocShell.cpp#10759

> for which we don't pass any 'securityMode' other than SEC_NORMAL

Totally not true.  See link above, and also line 10762 in the same file: TYPE_DOCUMENT loads can be SEC_SANDBOXED as well.

I don't know what you're logging and in which process, just like I don't know why you were not seeing us reach the code that is totally obviously being reached, but I can assure you that a load of file:///Users/ckerschb/Desktop/sop_bug/bypass_boris/redir.desktop triggered by file:///Users/ckerschb/Desktop/sop_bug/bypass_boris/PoC.html will have SEC_FORCE_INHERIT_PRINCIPAL.  See https://dxr.mozilla.org/mozilla-central/rev/51377a64158941f89ed73f388ae437cfa494c030/dom/base/nsContentUtils.cpp#6593 (which is passed the triggering principal for aLoadingPrincipal when docshell calls it at <https://dxr.mozilla.org/mozilla-central/rev/51377a64158941f89ed73f388ae437cfa494c030/docshell/base/nsDocShell.cpp#10744>).

Comment 32

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Boris Zbarsky [:bz] (Out June 25-July 6) from comment #31)
> > for which we don't pass any 'securityMode' other than SEC_NORMAL
> 
> Totally not true.  See link above, and also line 10762 in the same file:
> TYPE_DOCUMENT loads can be SEC_SANDBOXED as well.

Sorry for all the confusion - already banging my head at the keyboard - you are obviously right.

nsBaseChannel::Redirect {
  channelURI: file:///private/etc/hosts
  finalURI  : file:///private/etc/hosts
  triggeringPrincipal: file:///Users/ckerschb/Desktop/sop_bug/bypass_boris/PoC.html
  contentPolicyType: 6
  securityMode: SEC_FORCE_INHERIT_PRINCIPAL
}

Comment 33

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1278013_remove_force_inherit_flag_in_basechannel_redirect.patch

Boris, thanks for your help with setting up the test environment to reproduce the problem! It works now as expected and we are blocking the contents of the secret file to appear in the textarea; displaying the following error in the browser console:
> Error: Permission denied to access property "document"

Comment 34

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8764290 [details] [diff] [review]
bug_1278013_remove_force_inherit_flag_in_basechannel_redirect.patch

r=me

Comment 35

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Abdulrahman Alqabandi from comment #21)
> Thinking more about the attack scenario, I think the fact that a victim
> would need to download a zip file then unzip and finally execute the
> malicious html file locally could be shortened greatly.
> 
> So after some more testing I think the best attack scenario would be to have
> the user save the page from an attacker controlled website locally and
> simply open the index.html file from the downloads page, we can also trick
> the browser to download and save a .URL file using the (I think by default)
> option of saving a web page complete.

Dan, given that the page must be downloaded before the attack can be executed makes me think that this is at most a sec moderate, but I would like to get your rating before moving forward with this bug. What do you think?

Comment 36

[Daniel Veditz [:dveditz]]

sec-moderate seems appropriate, but that doesn't mean this is unimportant.

Comment 37

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8764290 [details] [diff] [review]
bug_1278013_remove_force_inherit_flag_in_basechannel_redirect.patch

Approval Request Comment
Users which can be tricked into downloading a webpage and open it locally are vulnerable to that attack which bypasses the Same origin policy.

[Feature/regressing bug #]:
Bug 1099296

[User impact if declined]:
SOP bypass.

[Describe test coverage new/current, TreeHerder]:
Tested manually using the crafted page from the reporter, Boris and myself.

[Risks and why]: 
Low, since it does not affect http redirects, but only redirects passing through nsBaseChannel::Redirect().

[String/UUID change made/needed]:
no

Comment 38

[Carsten Book [:Tomcat]]

 https://hg.mozilla.org/integration/mozilla-inbound/rev/6fcf253b6712

Comment 39

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6fcf253b6712

Comment 40

[Abdulrahman Alqabandi]

Does requiring the user to save page as really make this sec-moderate? 

I had a previous bug (https://bugzilla.mozilla.org/show_bug.cgi?id=1233936) which also required an html file to be downloaded and opened but it only resulted in detecting arbitrary files locally.

The bug here is a full SOP bypass with a fully accessible external window object available (including file: files/folders)

I just dont feel these two bugs are equivalent :/

On the official Mozilla security rating guide, under sec-moderate it does say "Client bugs that might have high or critical results but require the user perform unusual or complex actions to trigger."

But I dont think having the user hit ctrl+s and opening a file locally is 'unusual or complex'

Would you agree and possibly reconsider the rating Daniel?

Comment 41

[Abdulrahman Alqabandi]

Also, to further add to my argument,

There was a bug that enabled the reading of external domains as well as files (same as here) but this was done without user interactions (presumably) which was given sec-critical (from the usual sec-high for SOP bypasses) 

I can do the same except from local documents so logically it should follow that this gets a sec-high bounty  (since it does require some user interaction)

ref: https://www.mozilla.org/en-US/security/advisories/mfsa2015-149/

Comment 42

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8764290 [details] [diff] [review]
bug_1278013_remove_force_inherit_flag_in_basechannel_redirect.patch

Already landed in nightly, so, we want this in aurora & beta too.
For now, it doesn't match the esr criteria but if the severity increases, we should take it in esr45 too.

Comment 43

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/15cc9baf2b6f

Comment 44

[Carsten Book [:Tomcat]]

 https://hg.mozilla.org/releases/mozilla-beta/rev/b7f4630c2e64

Comment 45

[Daniel Veditz [:dveditz]]

(In reply to Abdulrahman Alqabandi[test] from comment #40)
> On the official Mozilla security rating guide, under sec-moderate it does
> say "Client bugs that might have high or critical results but require the
> user perform unusual or complex actions to trigger."
> 
> But I dont think having the user hit ctrl+s and opening a file locally is
> 'unusual or complex'

high and critical are for bugs that can be used for "drive-by" attacks on users, perhaps requiring them to click on something but that's extremely common on a web page (e.g. the web is made of links). Yes, users do save some pages and then open them later, but to create a successful attack out of this the attacker has to create a page that lots of users will want to save and re-open. I'm sure it can be done for a handful of victims making this a security bug and worth fixing, but not mass pwnage that rates a sec-high.

Comment 46

[Abdulrahman Alqabandi]

(In reply to Daniel Veditz [:dveditz] from comment #45)
> (In reply to Abdulrahman Alqabandi[test] from comment #40)
> > On the official Mozilla security rating guide, under sec-moderate it does
> > say "Client bugs that might have high or critical results but require the
> > user perform unusual or complex actions to trigger."
> > 
> > But I dont think having the user hit ctrl+s and opening a file locally is
> > 'unusual or complex'
> 
> high and critical are for bugs that can be used for "drive-by" attacks on
> users, perhaps requiring them to click on something but that's extremely
> common on a web page (e.g. the web is made of links). Yes, users do save
> some pages and then open them later, but to create a successful attack out
> of this the attacker has to create a page that lots of users will want to
> save and re-open. I'm sure it can be done for a handful of victims making
> this a security bug and worth fixing, but not mass pwnage that rates a
> sec-high.

Fair enough. Thank you for the bounty!

Comment 47

[Daniel Veditz [:dveditz]]

Christoph: I believe the ESR-45 branch contains the security flags changes this builds one. Any reason this patch wouldn't work on that branch?

[Tracking Requested - why for this release]:
Although we only promise sec-high/sec-critical support on ESR, because this vulnerability is easily explained and duplicated we should take this one. The patch in this bug is small and safe if the requisite support already exists on that branch (it looks like the underlying required change landed in Firefox 42).

Comment 48

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Daniel Veditz [:dveditz] from comment #47)
> Christoph: I believe the ESR-45 branch contains the security flags changes
> this builds one. Any reason this patch wouldn't work on that branch?

I don't fully understand what that sentence means. But let me explain: The problem was introduced within Bug 1099296, which landed in Firefox 38. The patch within this bug should apply (more or less) cleanly to any branch that builds on Firefox 38 or later.

If you are talking about the SEC_FORCE_INHERIT_PRINCIPAL security flag, that was introduced within Firefox 35 [1].

To sum it up, that patch should apply and work fine on ESR-45.

[1] https://hg.mozilla.org/mozilla-central/rev/d0f7f15e15fb#l3.47

Comment 49

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8764290 [details] [diff] [review]
bug_1278013_remove_force_inherit_flag_in_basechannel_redirect.patch

ok, let's take it to esr 45 too.

Comment 50

[Sylvestre Ledru [:Sylvestre]]

Looks quite safe and potentially high impact in term of security.

Comment 51

[Matt Wobensmith [:mwobensmith][:matt:]]

Hi Abdulrahman, can you verify that this is fixed on your end?

Comment 52

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-esr45/rev/3e8a4fa8cb04

Comment 53

[Abdulrahman Alqabandi]

Yupp, looks like its fixed from my side.

Comment 54

[Boris Zbarsky [:bzbarsky]]

The patch using "^" to "remove" a flag is likely causing bug 1292159...  :(  I can't believe I missed that.

Comment 55

[denverli koye]

Yes, looks quite safe and potentially high impact in term of security. The PoC above will automatically read the file page of C:// using an frame without requiring the user to click anything. Please change the URL file according to the path where you unzipped the PoC.
Ref: https://fileinfo.io/open-bin-file/

Comment 56

[Ronika kane]

APK file is an abbreviation of (Android Package Kit). APK is a file extension utilized for appropriating the applications on the Android OS from Google. These are much like the EXE documents in Windows OS utilized for introducing the product. https://openapkfiles.co/

Comment 57

[:Gijs (he/him)]

Restricting comments because of spam.