338037 - chrome://global/content/jar: can load remote chrome

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Daniel Veditz [:dveditz]]

Spun off from bug 337744 comment 6, chrome URLs can syntactically wrap arbitrary protocols and confer chrome privs! some don't work, but wrapping them in jar: does.

Luckily we block web content from directly loading chrome URLs as content, but it would be pretty easy to get a fair number of people to copy or drag a "broken" link to the address bar to get at content they really wanted (download the latest movie or music, free pr0n, etc).

> chrome://global/content/http://www.google.com/ does *not* work, because
> of the channel-unwinding checks at
>http://lxr.mozilla.org/mozilla/source/chrome/src/nsChromeProtocolHandler.cpp#563
>
> But if remote content were loaded from a JAR file it would work, e.g.:
> chrome://global/content/jar:http://www.example.com/myjar.zip!/somefile

Comment 1

[Daniel Veditz [:dveditz]]

Attachment: zipped alert.html testcase

Comment 2

[Daniel Veditz [:dveditz]]

Load the testcase unprivileged as
jar:https://bugzilla.mozilla.org/attachment.cgi?id=222076!/alert.html

Load the testcase with chrome privileges as
chrome://global/content/jar:https://bugzilla.mozilla.org/attachment.cgi?id=222076!/alert.html

Assigning "moderate" severity because of the required user-interaction, but if someone discovered a way to bypass checkLoadURI and find a way for web content to load chrome this would be "critical".

Anyone know a better bugzilla component for the chrome registry code? The description for XP Miscellany says "COMPONENT RETIRED. Please do not file bugs here." But this isn't a XUL bug, nor does this code live in Networking although it is related to URL processing. The "Core" product does not support any other  general category.

Comment 3

[Benjamin Smedberg]

Mozilla 1.7 may not have this bug or it may be manifested differently (even ff 1.0.x may have different manifestations), because of the different revisions and forks of chrome registry code involved.

Comment 4

[Benjamin Smedberg]

Attachment: Make colon an illegal character

This should be pretty safe and simple. Darin, if you can review ASAP, we're thinking about respinning 1.8.0.4 for this.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Yeah, but FF2 definitely has this issue.

I can't seem to reproduce with that testcase in trunk Seamonkey, but I'm not sure whether that's just a matter of needing URI syntax tweaks or a genuine chrome registry difference.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Wouldn't that prevent the use of ':' in filenames too?  Would it make more sense to error out if NS_NewURI on some substring succeeds, since that's the case we really care about as far as I can tell?

Comment 7

[Benjamin Smedberg]

I believe colons aren't legal characters in windows filenames anyway, and I'm worried about performance and reentrancy issue having to call NS_NewURI all the time.

Comment 8

[Darin Fisher]

But don't chrome URLs support query and reference fragments?  If we are going to restrict ':' then that should only apply to the file path.  Isn't it possible that chrome apps are using "chrome://foo/bar.xul?x=y:z" for stuff?

Comment 9

[Darin Fisher]

> Isn't it possible that chrome apps are using "chrome://foo/bar.xul?x=y:z" for stuff?

Example:

  chrome://foo/content/bar.xul?url=http://www.foo.com/

Also, what about escaped colons.  Do we need to worry about those too?

Comment 10

[Benjamin Smedberg]

Attachment: Use NS_NewURI

This uses NS_NewURI to check for absolute URIs. I still prefer the first version, but I understand the concern with query strings.

I'll also look at colon-checkin just the path (not sure which would be more efficient).

Comment 11

[Benjamin Smedberg]

Attachment: Use GetFilePath

I think that this is probably the most-correct. BTW, the documentation of .filePath, .directory in nsIURL.idl is incorrect.

Comment 12

[Darin Fisher]

Comment on attachment 222154 [details] [diff] [review]
Use GetFilePath

I think you should add some comments explaining why these checks
are necessary.  In what way is nsIURL's documentation incorrect?

r=darin

Comment 13

[Benjamin Smedberg]

Fixed on trunk.

Comment 14

[Benjamin Smedberg]

Fixed on 1.8 branch

Comment 15

[Benjamin Smedberg]

This does not appear to be a problem in Firefox 1.0.x or the Mozilla 1.7 series. I caused this in bug 278534, where I changed nsChromeRegistry::ConvertChromeURL from using string manipulation to resolving against the baseuri.

Comment 16

[Darin Fisher]

perhaps we should verify that the URL resulting from URL resolution is still a chrome: URL?

Comment 17

[Benjamin Smedberg]

Except it's not, it's typically a jar:file:/// url.

Comment 18

[Daniel Veditz [:dveditz]]

Comment on attachment 222154 [details] [diff] [review]
Use GetFilePath

unfortunately missed the 1.8.0.4 train by a couple weeks, and we didn't do a respin we could slip this into.

Comment 19

[Tim Riley [:timr]]

Comment on attachment 222154 [details] [diff] [review]
Use GetFilePath

We need to get this into 1.8.0.5

Comment 20

[Daniel Veditz [:dveditz]]

Comment on attachment 222154 [details] [diff] [review]
Use GetFilePath

approved for 1.8.0 branch, a=dveditz for drivers

Comment 21

[Benjamin Smedberg]

Fixed on MOZILLA_1_8_0_BRANCH

Comment 22

[Jay Patel [:jay]]

v.fixed on 1.8.0 branch with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US;
rv:1.8.0.5) Gecko/20060626 Firefox/1.5.0.5, permission denied for alert testcase.