1427448 - Some web extension API's do not account for shorthand form of file:// url

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P2)

Description

[Abdulrahman Alqabandi]

Attachment: detect-file-PoC.zip

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

Steps to reproduce:

Using a shorthand(?) form of a file:// url seems to bypass the existing security that prevents tabs.create from opening locally stored files. 

The attached PoC uses this to detect locally stored files/folders (with the help of the 'tabs' permission). Just install it and modify 'background.js' with random files in your disk beforehand (especially if you're not on windows).


Actual results:

We can use the following to open file: urls:
browser.tabs.create({url:"\C:\\\\test.xml"});


Expected results:

"\C:\\\\test.xml" should be treated the same as "file://C:\\\\test.xml"

Comment 1

[Abdulrahman Alqabandi]

Looks like other extension API's are impacted by this.

For example:
browser.browserAction.setPopup({popup:"\C:\\\\test.xml"}); 

So I think this is a wider issue than just tabs.create

Comment 2

[Abdulrahman Alqabandi]

I should have been more thorough with this one. Looks like we can sort of do this from the web as well:

1. open('\C:\\\\test.xml'); Will open a tab with "The address wasn’t understood" error page.
2. Click on refresh
3. File is loaded

I will let you guys decided whether to open another bug for that or handle it all here. But initially seems like Firefox never treats that form as a file:// url.

Comment 3

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Thanks for the reports here. I need to test this for myself (on vacation, don't have access to windows right now).

But the main issue that I can think of is the extension being able to read files from the file system. Due to 1420296, extensions with <all_urls> already can do this. However reading your PoC, I see that it doens't have <all_urls>. So that sounds like a bypass. See [1] for a public description of the issue.

As for comment 1, extensions shouldn't be able to include a file:// uri in something like browser.browserAction.setPopup. But I'll need to test this in order to determine the impact here. Worst case is parts of the extension running in a file:// uri but I don't know if that would happen (in the case above, what is the actual URL of the popup? If its just 'moz-extension:<uuid>/\C:\\\\test.xml' then this part isn't an issue. 


[1] https://stackoverflow.com/questions/42108782/firefox-webextensions-get-local-files-content-by-path/44516256#44516256


(In reply to Abdulrahman Alqabandi from comment #2)
> I should have been more thorough with this one. Looks like we can sort of do
> this from the web as well:
> 
> 1. open('\C:\\\\test.xml'); Will open a tab with "The address wasn’t
> understood" error page.
> 2. Click on refresh
> 3. File is loaded
> 
> I will let you guys decided whether to open another bug for that or handle
> it all here. But initially seems like Firefox never treats that form as a
> file:// url.

I'm on vacation and I don't have a windows machine handy to test this but this sounds like mostly expected behavior. The first error is because the "scheme" of "\C:" isn't registered. When you reload the page I assume there is some logic which passes the url to some native path resolver which coverts the \:C to some canonical path. That said, open('file:///...') definitely errors, and I wonder if this behavior could be abused. I'm inclined to move this into a separate bug, but I'll have to figure out what exactly is going on here first.

I'm needinfo'ing myself here to look into this when I'm back at work in a few days. Ultimately I think 1420296 is a higher priority, but this might also be something that needs a fix

Comment 4

[Abdulrahman Alqabandi]

> above, what is the actual URL of the popup? If its just
> 'moz-extension:<uuid>/\C:\\\\test.xml' then this part isn't an issue. 
>

The popup directly opens the local file, exactly like the tabs.create one.

Thanks for the extra information, bug 1420296 definitely looks like a much more severe issue. I always assumed file access was granted because I was loading my extensions unpacked (through about:debugging). Though it is probably not related to the behavior here.
browser.tabs.create({url:"file://C:\\\\test.xml"}); is explicitly blocked but not browser.tabs.create({url:"\C:\\\\test.xml"});

So theoretically if bug 1420296 was fixed it may still be bypassed using the 'shorthand' version as it seems to be overlooked in cases where file:// is specifically blocked. 

Also I think your explanation for comment #2 is exactly whats happening in the main PoC. You can open non existent URI schemes (using tabs.create) and a page will open attempting to navigate to it. However, it looks like in firefox if the 'non existent' URI scheme is a single character, it will assume its a windows drive and convert the url into a file://{insert single char uri scheme here} url. So this seems to be a windows only problem.

Also, I understand this is vacation time and of course take all the time you need I am in no rush.

Comment 5

[:Gijs (he/him)]

Actually ni'ing Paul. :-)

(Also moving and CC'ing some webextensions folks)

Comment 6

[Abdulrahman Alqabandi]

One more:
 browser.sidebarAction.setPanel({panel: "\C:\\\\test.xml"});

Comment 7

[Kris Maglione [:kmag]]

(In reply to Abdulrahman Alqabandi from comment #0)
> We can use the following to open file: urls:
> browser.tabs.create({url:"\C:\\\\test.xml"});

This is not a valid URL. If you try to open a URL like this, it resolves to something like String.raw`moz-extension://.../C:\\test.xml`, which should pass a security check, but not open a local file.

Comment 8

[Abdulrahman Alqabandi]

(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #7)
> This is not a valid URL. If you try to open a URL like this, it resolves to
> something like String.raw`moz-extension://.../C:\\test.xml`, which should
> pass a security check, but not open a local file.

The PoC should only work on windows.

The url resolves to a 'C' uri scheme ("\C"==="C"):

new URL("\C:\\\\test.xml")
URL { href: "c:\\test.xml", origin: "null", protocol: "c:", username: "", password: "", host: "", hostname: "", port: "", pathname: "\\test.xml", search: "" }

And on windows it seems like if a URI scheme is a single character it will assume its a windows drive and resolve it as a file:// url.

Comment 9

[Abdulrahman Alqabandi]

Attachment: poc2.zip

Attached PoC will showcase all three instances where we can open file uri schemes (on windows)

Video of it in action: https://www.youtube.com/watch?v=2bGz_VzYDv0

Comment 10

[Andy McKay]

> This is not a valid URL. If you try to open a URL like this, it resolves to
> something like String.raw`moz-extension://.../C:\\test.xml`, which should
> pass a security check, but not open a local file.

I can reproduce this on Windows 10.

Comment 11

[Paul Theriault [:pauljt] (no longer reading bugmail)]

I'm not sure sec-high is appropriate here. We don't have a clear policy on file system access with Web Extensions yet (as chrome does). For example, Web Extensions are allowed to inject content scripts in the file: origins and this is publicly documented[1]. 
If I understand this bug correctly, it only really allows a web extension to navigate various windows to a file:// uri.  That's not great, but I think the impact (at least direct impact) is limited to setting various windows to file:// uris. I think we do definitely do need to fix this, and we are definitely planning on restricting access to files from web extensions, but I'm not sure of the actual here. My guess is that with effort a malicious extension _might_ be able read an arbtirary file but its not as simple as just using this bug. I would probably say this is only moderate (due to the need for it to be chained with other issues) but thats a guess. 

Freddy can you pick this up for me and figure out the actual impact here? 





[1] https://developer.mozilla.org/en-US/Add-ons/WebExtensions/match_patterns

Comment 12

[:Gijs (he/him)]

(In reply to Andy McKay [:andym] from comment #10)
> > This is not a valid URL. If you try to open a URL like this, it resolves to
> > something like String.raw`moz-extension://.../C:\\test.xml`, which should
> > pass a security check, but not open a local file.
> 
> I can reproduce this on Windows 10.

FWIW, the root cause here is likely that somehow the webextension code is calling into an API that calls into nsIURIFixup, which will "fix" URLs like this (but only on Windows, because it only makes sense on Windows).

Comment 13

[Frederik Braun [:freddy]]

First of all, thank you for filing this bug Abdulrahman Alqabandi!

I can confirm that it's possible to load local files in a tab (panel, etc.), which we need to fix.

Of all those methods to open the file, only the promise returned by tabs.create() seems to provide something useful. The result, however, does *NOT* contain a URL attribute or any other sensitive information (which is normal for extensions without the required permissions, see https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/Tab).
Notably, the limited information in this object doesn't tell whether the file exists at all. Pointing to c:\\doesntexist.xml would give an attacker the very same information.
There's also no way to inspect the DOM or the content of the tab itself.
In summary, there's no direct leak as to what files exist or what they contain.

So, what's missing to make this bad is for the add-on to download a file (or trigger downloading of files) into a well-known directory and open it. The way our Same Origin Policy works for JavaScript would allow the attacker supplied file on the local disk to access all files in the same directory and its subdirectories.

Abdulrahman Alqabandi, if you can find a way to do this attack, as described above, I'll bump the security rating back to high.

Comment 14

[Abdulrahman Alqabandi]

I assumed this was sec-high due to it sort of being a wider issue than just web extensions. Single character uri schemes should be treated as file:// which they aren't even from web (in some cases). So I hope you can see this as one bug with multiple sec-moderate issues in one. But if not, I assume the fix for browser.tabs.create would fix every issue which is why this is treated as one sec-moderate bug?  


(In reply to Frederik Braun [:freddyb] from comment #13)
> First of all, thank you for filing this bug Abdulrahman Alqabandi!
> 
> Abdulrahman Alqabandi, if you can find a way to do this attack, as described
> above, I'll bump the security rating back to high.

I am up for the challenge, am I allowed to use any extra permissions at all? Because if you do allow me then I could use the 'find' permission to read files.

Comment 15

[Frederik Braun [:freddy]]

Rating is according to <https://wiki.mozilla.org/Security_Severity_Ratings>: There's no way to obtain confidential data (which would be sec-high), but there would be if you found another bug. Maybe you'll find it? I can't properly rate a sec-bug before you present it to me ;)

Regarding "find", I'm not sure I've grokked the permission completely, but is it actually useful without "tabs"? Note that the "tabs" permission wouldn't be allowed for a sec-high, as it is an intended way to access a tab.

Hope that helps!

Comment 16

[Abdulrahman Alqabandi]

Attachment: findRead.zip

(In reply to Frederik Braun [:freddyb] from comment #15)
> Regarding "find", I'm not sure I've grokked the permission completely, but
> is it actually useful without "tabs"? Note that the "tabs" permission
> wouldn't be allowed for a sec-high, as it is an intended way to access a tab.

Using only 'find' permission I am able to read entire file, view attached PoC. Hope this will change the rating :)

Comment 17

[Abdulrahman Alqabandi]

Note: When you install the PoC click on 'debug' to open the debug window for the PoC extension. Wait for a couple of minutes and it should show you full contents.

Comment 18

[:Gijs (he/him)]

Note: no opinion on the webextension sec-severity issue here, but wanted to make sure the following didn't slip through the cracks.

(In reply to Abdulrahman Alqabandi from comment #14)
> Single character uri schemes should be treated as file://
> which they aren't even from web (in some cases). So I hope you can see this
> as one bug with multiple sec-moderate issues in one.

These 2 sentences don't make sense to me. I think there's a negation missing, but even then I'm confused, esp. about what you mean by "in some cases" - behaviour on the web should be consistent. We should *not* be letting web content link to files on disk by listing "C:/foo" and letting something else uri-fixup it after the security checks so that it "works". If we do, that's a separate bug.

Can you clarify?

Comment 19

[Abdulrahman Alqabandi]

As I mentioned in comment 2 you can execute thing=open('\C:\\\\test.xml') and after refreshing the error page you trigger the fixup and are left with the opener having an object (thing) pointing to a file:// url (the object is restricted ofc). 

I tested other web JS apis that deal with HTTP requests and so far I couldn't find anything exploitable. I pretty much get 'unknown protocol' or 'network error' where I should be getting a security error which may be a problem in the future as it is a sign single char uri's arent treated as file:// even in web.

I hope this clarifies.

Comment 20

[Abdulrahman Alqabandi]

FWIW, we have a partial file detection method using 'browser.tabs.detectLanguage' which does not require any special permission.

I found that the error page shown when a folder does not exist is always 'en', however if we try to detect language for a binary (view-source:\C:\\\\Windows\\notepad.exe) it will return 'und' always. I say partial detection because html files and some other cases also return 'en'.

Comment 21

[:Gijs (he/him)]

(In reply to Abdulrahman Alqabandi from comment #19)
> As I mentioned in comment 2 you can execute thing=open('\C:\\\\test.xml')
> and after refreshing the error page you trigger the fixup and are left with
> the opener having an object (thing) pointing to a file:// url (the object is
> restricted ofc). 

I don't see any good way to prevent this. We could make refresh not trigger any fixup, but that likely breaks legitimate usecases, and in any case the same thing would still happen if you just clicked in the url bar and hit enter (and that behaviour we *really* can't change without breaking legitimate usecases).

> I should be getting a security error which may be a
> problem in the future as it is a sign single char uri's arent treated as
> file:// even in web.

Giving JS a security error trying to link to single-char protocols on the web seems risky in terms of webcompat / breaking legitimate usecases. A priori, there's no reason those schemes are any more risky than longer ones, for which we helpfully show the "wanna open this in an external program" dialog...

Comment 22

[Abdulrahman Alqabandi]

(In reply to :Gijs from comment #21)
> I don't see any good way to prevent this. We could make refresh not trigger
> any fixup, but that likely breaks legitimate usecases, and in any case the
> same thing would still happen if you just clicked in the url bar and hit
> enter (and that behaviour we *really* can't change without breaking
> legitimate usecases).
> 

I think the better approach to think of is to have converted the 'C:' into 'file:///C:' (AKA trigger urifixup) in the web content (or extension webcontent) before firefox can even think of opening a single char uri scheme. Because such a scheme should be invalid, otherwise it would collide with an existing window drive if registered. If a user opens a single char uri scheme outside of the web content (bookmark, copy paste, refresh) it does do the right thing and triggers the fixup. 

> Giving JS a security error trying to link to single-char protocols on the
> web seems risky in terms of webcompat / breaking legitimate usecases. A
> priori, there's no reason those schemes are any more risky than longer ones,
> for which we helpfully show the "wanna open this in an external program"
> dialog...

It shouldn't break anything since in chrome single char uri schemes are converted to file:// schemes automatically, edge throws a security error if you even try to create a single char URL. Could you provide a legitimate use case that is widely used?

Executing 'new URL("\C:\\\\").protocol' gives the following:

Chrome: 'file:'
Edge: 'Permission denied'
Firefox: 'c:'


This bug showcases at least one instance where the urifixup failed, it fixed the uri _after_ a security check was made. Had single uri schemes been blocked from web/extension content (or treated/converted to file: schemes) this bug wouldn't exist. Now I showed you that the same is observed (that causes this bug) within web content and although no exploit has been found, it is just waiting to happen IMO. Better be safe than sorry.

Comment 23

[:Gijs (he/him)]

(In reply to Abdulrahman Alqabandi from comment #22)
> It shouldn't break anything since in chrome single char uri schemes are
> converted to file:// schemes automatically

This seems super dodgy from a URL standard perspective... Maybe Anne can comment.

> , edge throws a security error if
> you even try to create a single char URL. Could you provide a legitimate use
> case that is widely used?

Mozilla (and Google and MS, really) have years and years of experience of saying "everyone thinks <thing> is dead/unused, and our telemetry agrees", discontinuing support for the thing, and then having to go back because it turns out that actually, people *were* using it. Even if other browsers had already broken support or never supported it in the first place (because intranet, disabled telemetry, etc. etc.). Outright blocking of every single scheme of 1 latin character seems pretty scary to me from a compat perspective, though I feel slightly better if Edge and Chrome have already broken it.

> This bug showcases at least one instance where the urifixup failed, it fixed
> the uri _after_ a security check was made. Had single uri schemes been
> blocked from web/extension content (or treated/converted to file: schemes)
> this bug wouldn't exist. Now I showed you that the same is observed (that
> causes this bug) within web content and although no exploit has been found,
> it is just waiting to happen IMO. Better be safe than sorry.

But your suggested mitigation is to apply more fixup to things that ought not to have fixup in the first place, which is why I'm so skeptical. It feels like going in the wrong direction, and in the worst case like it would enable worse abuse than the status quo.

Comment 24

[Anne (:annevk)]

It's up for consideration still over at https://github.com/whatwg/url/issues/271, though I'm somewhat opposed to do this for all single code point schemes. My idea was that we could consider it if the first code point after the scheme is a backslash.

(I also would like us to have a single URL parser across platforms, including Windows.)

Comment 25

[Kris Maglione [:kmag]]

Will fix this in bug https://bugzil.la/1430431. Please do not link this bug to that one until we make it public.

Comment 26

[Kris Maglione [:kmag]]

As a side note, this is all much uglier than I expected. nsIWebNavigation currently doesn't provide any way to load an nsIURI directly, or to turn off URL fixup. It accepts flags to restrict the fixup behavior, but the fixup service doesn't have any flags to turn off native-file-path-to-URL fixup.

Comment 27

[Abdulrahman Alqabandi]

Given comment 16 and comment 20, would that make this a sec-high? 

Also, is what is described in those two comments intended behavior? Or should I file new bugs for them?

Comment 28

[Kris Maglione [:kmag]]

At this point, I'd say it's sec-moderate. Extensions being able to load file: URLs is not a serious concern. Allowing them access to their contents is, but we already allow that to extensions with the <all_urls> or file: permissions, and need a plan for how we want to lock that down in the future.

Comment 29

[Abdulrahman Alqabandi]

Ok I think I understand now. I have to specifically find a way to download and open a local file.

But to be clear is there a bug with comment 20 and comment 16

Comment 30

[Abdulrahman Alqabandi]

Attachment: PoC3.zip

I think I figured out a way to do what you've described in comment 13.

The only permission I have is "downloads" and as a result I can read the data of any cross origin website with credentials. I'm relying on the bug described here mainly. 

I simply download a remote website and save it as "fb.dat" then I download a custom html file and finally I open the html file from within file: uri scheme. I get the path of the downloads folder by querying the first downloaded file (fb.dat) and then the object returned contains full path to downloads folder.

Do you think this satisfied the parameters set?

Comment 31

[Frederik Braun [:freddy]]

I can't reproduce this in either Firefox Nightly or Firefox Release.

In release, the downloads.search() Promise does not yield a response object, just an id.
In Nightly, it opens a new tab but that only shows an error.
The URL is moz-extension://extension-id/home/username/Downloads/qfile.html, but the browser refuses to load it.

Please clarify the steps to repeat and the version in which you tested this.

Comment 32

[Abdulrahman Alqabandi]

Make sure you are on Windows since the bug described here is a windows only problem. 
It works fine for me on latest Nightly. Video: https://youtu.be/T6yQQ8CnY5A
Also, the PoC sometimes will try to open a downloaded file too soon, if a new tab appears with nothing loaded just refresh it (I fixed this by increasing the time waited in the setTimeout of background.js)

Also, downloads.search returns a promise with an array of downloadItem ( https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/downloads/search )

Hope this helps to reproduce.

Comment 33

[Frederik Braun [:freddy]]

Hm, neat.
This expands the attack to read local files, as I challenged you in comment 15.
I'll accept that as a privilege escalation.

It is still restricted to the download directory, but your exploit also comes with a way to access cross-origin data that it wouldn't get with the just the download permission.

Thank you for your submission!

Comment 34

[Frederik Braun [:freddy]]

(In reply to Frederik Braun [:freddyb] (unreachable from Feb 17th to March 5th) from comment #33)
> your exploit also
> comes with a way to access cross-origin data that it wouldn't get with the
> just the download permission.

To be more specific: This exploit *reads* cross-origin data, when the API should only allow saving it to disk.

Comment 35

[Kris Maglione [:kmag]]

(In reply to Frederik Braun [:freddyb] (unreachable from Feb 17th to March 5th) from comment #33)
> Hm, neat.
> This expands the attack to read local files, as I challenged you in comment
> 15.
> I'll accept that as a privilege escalation.

WebExtensions *already* have the ability to read local files, assuming they have the <all_urls> permission.

Comment 36

[Frederik Braun [:freddy]]

That's the point. This one doesn't.

Comment 37

[Rob Wu [:robwu]]

Not reproducible any more on Windows, Firefox 70.0.1.

Services.io.newURI("moz-extension://foo/").resolve("\C:") still results in c: and is internally resolved to a file:-URL, but the browser refused to load it. I believe that this is because triggeringPrincipal is set to the extension's principal, which was added in bug 1466801.

After a manual reload, the local file is shown, but that also applies to window.open("\C:") from the web, as mentioned in comment 2.