1424261 - (CVE-2018-5140) moz-icon images can be linked and read from the web

Status: RESOLVED FIXED

(Firefox :: Untriaged, defect)

Description

[Abdulrahman Alqabandi]

Attachment: svger.html

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

Steps to reproduce:

Tested on Nightly 59.0a1 (2017-12-07) (64-bit).

View attached PoC.


Actual results:

It seems that we can still link moz-icon urls if we include the current pages URL within it like so: moz-icon:http://localhost/poc.html?size=64&contentType=application/xml 

I then noticed that we can read the image through canvas.toDataURL. So we are now able to determine which application is used to open certain file types, as well as potential fingerprinting of user within private browsing (?).


Expected results:

moz-icon should be blocked from linking on the web. Also I think its worth checking why canvas.toDataURL is working here.

Comment 1

[:Gijs (he/him)]

Doesn't work on macOS with e10s because of sandboxing (see bug 1419811).

Prior to bug 1222924, you could just do this the easy way by using moz-icon://.xml?size=16 , so it seems this being broken on 58 beta is kind of expected - but I think being able to read it doesn't work on 57/58 with just moz-icon://.xml?size=16 .

Either way, it's unexpected that it's still possible to do this on nightly. On the other hand, I'm a bit confused/worried that it seems in/after bug 1173214, we didn't restrict moz-icon to only be have nested file: URIs.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I haven't managed to reproduce on Linux so far...  Also sandboxing?

Comment 3

[Abdulrahman Alqabandi]

It seems like the URL is treated completely same origin, we can even frame it and execute js in it.

<iframe src="moz-icon:http://localhost/evil.ani" onload="window[0].eval('alert(location)')"></iframe>

Perhaps there is something checking to see if a given url contains {uri-scheme}:http://{same-origin-domain}.. and automatically gives it same-origin status?

Comment 4

[:Gijs (he/him)]

There are at least 4 ways of fixing this:

- https://searchfox.org/mozilla-central/rev/281d99b3d342b71d0653b872345761372d38f5c1/caps/nsScriptSecurityManager.cpp#843-845

should be checking aTargetURI instead of aTargetBaseURI .

- https://dxr.mozilla.org/mozilla-central/rev/a461fe03fdb07218b7f50e92c59dde64b8f8a5b0/image/decoders/icon/nsIconURI.cpp#208

and any other entrypoints into moz-icon URI constructors should only allow 'file' inner URIs.

- the contentType query parameter shouldn't do anything when there's a nested URI (not 100% sure if this is a red herring - I'd expect "foo.com/bar.<extension>" might also get you an icon for that extension, without the contentType param)

- https://bugzilla.mozilla.org/show_bug.cgi?id=1171853#c6

I'll see what I can do about getting patches for these. At least the moz-icon inner http URI one should be easy to fix. I don't know how much fallout there would be from fixing the first issue. From what I can tell quickly, moz-icon is the only UI_RESOURCE URI type that is ever nested, so it should be fine, but perhaps I'm missing something. Like, perhaps that'll break file: directory listing icons again (sigh...).

bug 1171853 is going to be more work to address, and isn't generally an issue because if you've got a nested URI that points to foo.com, it's mostly sensible that it's same-origin with "actual" foo.com . But for that to be the case, the data that a channel to that URI provides would actually want to come from foo.com, which clearly isn't the case here.

It would help if there was a sec rating on this bug so I had an idea about how cautious I need to be about pushing to try. While I'm annoyed that I missed this, I don't know how severe "know what applications the user has installed" is, in the general case. Sec-moderate, from a quick look at the severity ratings page?

Comment 5

[Abdulrahman Alqabandi]

Attachment: dump-icon.txt

I tried to create a blob URL within the moz-icon URL and apparently I got a possible stack corruption tab crash:
Possible Stack Corruption starting at xul!mozilla_dump_image+0x0000000000928669 (Hash=0xa1943e15.0xf278a43e)

https://crash-stats.mozilla.com/report/index/b8b3001b-e0b2-42ec-957d-463540171212

Also I attached the dump log where possible stack corruption was seen (using 'exploitable' module)

To repro:


<iframe src="moz-icon:http://localhost/evil.ani" onload="window[0].eval(`URL.createObjectURL(new Blob([''],{type:'text/html'}))`)"></iframe>


Doesn't work within view-source: page so it seems unique to moz-icon. This may be a sign that this context will result in unexpected behavior. Will test more.

Comment 6

[Abdulrahman Alqabandi]

New crash report using similar poc as above (except I loaded a moz-icon uri with a lot of junk in it)

Notice some of the frames are hex with no name, isn't that bad?:

https://crash-stats.mozilla.com/report/index/87bf5f3a-51f0-41fc-a831-4364b0171212

Should I open a new bug for that? or deal with it all here?

Comment 7

[Abdulrahman Alqabandi]

PoC for Comment 6:

a=open('moz-icon:http://localhost/'+unescape('%aa%ab%ac%ad%ae%af%a1%a2%a3%a4%a5%a6%a7%a8%a9%20%ba%bb%bc%bd%be%bf%b1%b2%b3%b4%b5%b6%b7%b8%b9%b0%ca%cb%cc%cd%ce%cf%c1%c2%c3%c4%c5%c6%c7%c8%c9%c0%da%db%dc%dd%de%df%d1%d2%d3%d4%d5%d6%d7%d8%d9%d0%ea%eb%ec%ed%ee%ef%e1%e2%e3%e4%e5%e6%e7%e8%e9%e0%fa%fb%fc%fd%fe%ff%f1%f2%f3%f4%f5%f6%f7%f8%f9%f0%1a%1b%1c%1d%1e%1f%11%12%13%14%15%16%17%18%19%10%2a%2b%2c%2d%2e%2f%21%22%23%24%25%26%27%28%29%20%3a%3b%3c%3d%3e%3f%40%5b%5c%5d%5e%5f%60%7b%7c%7d%7e%7f%8a%8b%8c%8d%8e%8f%81%82%83%84%85%86%87%88%89%80%9a%9b%9c%9d%9e%9f%91%92%93%94%95%96%97%98%99%90%0b%0c%0e%0f%01%02%03%04%05%06%07%08%09%0a'));

Then attempt to create a blob URL within it. 

a.eval(`URL.createObjectURL(new Blob([''],{type:'text/html'}))`)

Comment 8

[:Gijs (he/him)]

(In reply to Abdulrahman Alqabandi from comment #6)
> Should I open a new bug for that? or deal with it all here?

File a new bug. Looks like issues with blob serialization/deserialization.

Comment 9

[Abdulrahman Alqabandi]

(In reply to :Gijs from comment #8)
> (In reply to Abdulrahman Alqabandi from comment #6)
> > Should I open a new bug for that? or deal with it all here?
> 
> File a new bug. Looks like issues with blob serialization/deserialization.

Done. Bug 1424891 filed.

Comment 10

[:Gijs (he/him)]

Attachment: Fix CheckLoadURI

Comment 11

[:Gijs (he/him)]

Attachment: Stop accepting 'whatever' for moz-icon URIs

Comment 12

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8936428 [details] [diff] [review]
Stop accepting 'whatever' for moz-icon URIs

Review of attachment 8936428 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

::: image/decoders/icon/nsIconURI.cpp
@@ +219,5 @@
>    nsAutoCString iconSpec(aSpec);
>    if (!Substring(iconSpec, 0,
> +                 MOZICON_SCHEME_LEN).EqualsLiteral(MOZICON_SCHEME) ||
> +      (!Substring(iconSpec, MOZICON_SCHEME_LEN, 7).EqualsLiteral("file://") &&
> +       !Substring(iconSpec, MOZICON_SCHEME_LEN, 8).EqualsLiteral("//stock/") &&

You don't need this line. if the substring doesn't start with //, it definitely doesn't start with //stock. We can mention it in a comment, if you want to.

@@ +304,5 @@
> +    // The inner URI should be a 'file:' one. If not, bail.
> +    bool isFile = false;
> +    if (!NS_SUCCEEDED(mIconURL->SchemeIs("file", &isFile)) || !isFile) {
> +      return NS_ERROR_MALFORMED_URI;
> +    }

We should probably add a test to test_moz_icon_uri.js in a follow up patch.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8936427 [details] [diff] [review]
Fix CheckLoadURI

r=me

Comment 14

[:Gijs (he/him)]

bug 1173214 was sec-low, and this doesn't even confirm the existence of actual local files, just whether specific mime types are set to be handled by specific programs, which seems less serious (given that up until pretty recently, you could just ask the 'plugins' object whether the adobe acrobat npapi plugin was installed, for instance). So going to mark as sec-low and land the patches. Ni'ing Dan so he can change severity if necessary.

I'll try to address point 4 from comment 4 in bug 1171853 either before or after the Christmas / New Year's break, depending on the rest of my queue. I don't currently have any reason to believe it's urgent given the other restrictions that are in place.

Comment 15

[Abdulrahman Alqabandi]

Bug 1173214's reporter never provided a working PoC thus the sec-low.

Comment 16

[:Gijs (he/him)]

(In reply to Abdulrahman Alqabandi from comment #15)
> Bug 1173214's reporter never provided a working PoC thus the sec-low.

It's true that they didn't provide a working PoC, but even detecting arbitrary local files is only sec-moderate, and as I already said, this bug seems less severe (it doesn't detect arbitrary local files - it detects application handlers that are (or were!) installed, and not even their paths).

Either way, whether it's sec-low or sec-moderate, this can land without sec-approval, which is the salient point for me landing it right now... Seems pretty clear this isn't sec-high/crit.

Comment 17

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/236ad715d7548c2a0ac400aeadc12627885e764b
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/dc198f688d39013ec8fb15d831e02f7e9f0f1505

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/dc198f688d39
https://hg.mozilla.org/mozilla-central/rev/236ad715d754

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Doesn't seem worth backporting to me, but feel free to set the status to affected and request approval if you feel strongly otherwise.