Bug 1278013 (CVE-2016-5265)

Same origin policy bypass in local document/Universal xss

RESOLVED FIXED in Firefox 48

Status

()

Core
Networking: File
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: Abdulrahman Alqabandi, Assigned: ckerschb)

Tracking

({csectype-sop, regression, sec-moderate})

38 Branch
mozilla50
csectype-sop, regression, sec-moderate
Points:
---
Dependency tree / graph
Bug Flags:
sec-bounty +

Firefox Tracking Flags

(firefox46 wontfix, firefox47 wontfix, firefox48 fixed, firefox49 fixed, firefox-esr38 wontfix, firefox-esr4548+ fixed, firefox50 fixed)

Details

(Whiteboard: [adv-main48+][adv-esr45.3+])

Attachments

(8 attachments)

(Reporter)

Description

2 years ago
Created attachment 8759911 [details]
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.
(Reporter)

Comment 1

2 years ago
Created attachment 8759912 [details]
Screenshot of bypass being used to read full directory
(Reporter)

Comment 2

2 years ago
Created attachment 8759914 [details]
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.
(Reporter)

Comment 3

2 years ago
Created attachment 8759915 [details]
Screenshot of using this bypass to read facebook whilst logged in
(Reporter)

Comment 4

2 years ago
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.
(Reporter)

Comment 5

2 years ago
Created attachment 8759920 [details]
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.)
(Reporter)

Comment 6

2 years ago
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.
(Reporter)

Updated

2 years ago
Summary: Same origin policy bypass in local document → Same origin policy bypass in local document/Universal xss
(Reporter)

Comment 7

2 years ago
Could you check this please?
Flags: needinfo?(jaws)
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?
Flags: needinfo?(jaws) → needinfo?(dveditz)
(Reporter)

Comment 9

2 years ago
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
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.
Blocks: 1099296
Group: firefox-core-security → core-security
Status: UNCONFIRMED → NEW
Component: Untriaged → Networking: File
Ever confirmed: true
Flags: needinfo?(sworkman)
Flags: needinfo?(qab)
Flags: needinfo?(ckerschb)
Keywords: regression
Product: Firefox → Core
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/
(Reporter)

Comment 12

2 years ago
(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.
(Reporter)

Updated

2 years ago
Flags: needinfo?(qab)
(Reporter)

Comment 13

2 years ago
Created attachment 8760441 [details]
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.
Thanks, that confirms it is bug 1099296.
Has Regression Range: --- → yes
status-firefox46: --- → affected
status-firefox47: --- → affected
status-firefox48: --- → affected
status-firefox49: --- → affected
status-firefox-esr38: --- → affected
Version: 49 Branch → 38 Branch
Chris - can you look into this, please?
Flags: needinfo?(sworkman)
Group: core-security → network-core-security
Flags: needinfo?(dveditz)
(Assignee)

Comment 16

2 years ago
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
Flags: needinfo?(ckerschb) → needinfo?(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.
Flags: needinfo?(bzbarsky)
(Assignee)

Comment 18

2 years ago
(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
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.
(Reporter)

Comment 20

2 years ago
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.
(Reporter)

Comment 21

2 years ago
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_)
(Reporter)

Comment 22

2 years ago
Filed Bug 1279126 for the 'save full web page' behavior described in Comment 21
(Assignee)

Comment 23

2 years ago
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)
Flags: needinfo?(bzbarsky)

Comment 24

2 years ago
(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?
> 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.
Flags: needinfo?(bzbarsky)

Updated

2 years ago
Flags: sec-bounty?
(Reporter)

Comment 26

2 years ago
Any update on this? Is this a sec-high bug?
Flags: needinfo?(ckerschb)
(Assignee)

Comment 27

2 years ago
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.
Flags: needinfo?(ckerschb)
> 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.
Flags: needinfo?(ckerschb)
Created attachment 8763946 [details]
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.
(Assignee)

Comment 30

2 years ago
(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]
Flags: needinfo?(ckerschb) → needinfo?(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>).
Flags: needinfo?(bzbarsky)
(Assignee)

Comment 32

2 years ago
(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
}
(Assignee)

Comment 33

2 years ago
Created attachment 8764290 [details] [diff] [review]
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"
Attachment #8764290 - Flags: review?(bzbarsky)
Comment on attachment 8764290 [details] [diff] [review]
bug_1278013_remove_force_inherit_flag_in_basechannel_redirect.patch

r=me
Attachment #8764290 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 35

2 years ago
(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?
Flags: needinfo?(dveditz)
sec-moderate seems appropriate, but that doesn't mean this is unimportant.
Flags: needinfo?(dveditz)
Keywords: csectype-sop, sec-moderate
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
(Assignee)

Comment 37

2 years ago
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
Attachment #8764290 - Flags: approval-mozilla-beta?
Attachment #8764290 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

2 years ago
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/6fcf253b6712
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Comment hidden (off-topic)
Comment hidden (off-topic)
status-firefox46: affected → wontfix
status-firefox47: affected → wontfix
status-firefox-esr38: affected → wontfix
status-firefox-esr45: --- → affected
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.
Attachment #8764290 - Flags: approval-mozilla-beta?
Attachment #8764290 - Flags: approval-mozilla-beta+
Attachment #8764290 - Flags: approval-mozilla-aurora?
Attachment #8764290 - Flags: approval-mozilla-aurora+
Group: network-core-security → core-security-release
(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.
Flags: sec-bounty?
Flags: sec-bounty+
Flags: needinfo?(dveditz)
(Reporter)

Comment 46

2 years ago
(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!
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).
tracking-firefox-esr45: --- → ?
Flags: needinfo?(ckerschb)
(Assignee)

Comment 48

2 years ago
(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
Flags: needinfo?(ckerschb)
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.
Attachment #8764290 - Flags: approval-mozilla-esr45+
Looks quite safe and potentially high impact in term of security.
tracking-firefox-esr45: ? → 48+
Hi Abdulrahman, can you verify that this is fixed on your end?
Flags: needinfo?(qab)

Updated

2 years ago
Alias: CVE-2016-5265
Whiteboard: [adv-main48+][adv-esr45.3+]
(Reporter)

Comment 53

2 years ago
Yupp, looks like its fixed from my side.
Flags: needinfo?(qab)
The patch using "^" to "remove" a flag is likely causing bug 1292159...  :(  I can't believe I missed that.
Depends on: 1292159
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.