Last Comment Bug 338037 - chrome://global/content/jar: can load remote chrome
: chrome://global/content/jar: can load remote chrome
Status: RESOLVED FIXED
[sg:moderate]
: fixed1.8.1, verified1.8.0.5
Product: Toolkit
Classification: Components
Component: Startup and Profile System (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Benjamin Smedberg [:bsmedberg]
:
Mentors:
chrome://global/content/jar:https://b...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-05-15 12:13 PDT by Daniel Veditz [:dveditz]
Modified: 2008-07-31 03:02 PDT (History)
9 users (show)
dveditz: blocking‑aviary1.0.9-
benjamin: blocking1.9a1+
benjamin: blocking‑firefox2+
benjamin: blocking‑thunderbird2+
benjamin: blocking1.8.0.5+
jwalden+bmo: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
zipped alert.html testcase (347 bytes, application/octet-stream)
2006-05-15 12:14 PDT, Daniel Veditz [:dveditz]
no flags Details
Make colon an illegal character (1.06 KB, patch)
2006-05-15 14:15 PDT, Benjamin Smedberg [:bsmedberg]
no flags Details | Diff | Review
Use NS_NewURI (1.32 KB, patch)
2006-05-16 00:28 PDT, Benjamin Smedberg [:bsmedberg]
no flags Details | Diff | Review
Use GetFilePath (1.27 KB, patch)
2006-05-16 02:02 PDT, Benjamin Smedberg [:bsmedberg]
darin.moz: first‑review+
darin.moz: approval‑branch‑1.8.1+
dveditz: approval1.8.0.5+
Details | Diff | Review

Description Daniel Veditz [:dveditz] 2006-05-15 12:13:43 PDT
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] 2006-05-15 12:14:48 PDT
Created attachment 222076 [details]
zipped alert.html testcase
Comment 2 Daniel Veditz [:dveditz] 2006-05-15 12:25:30 PDT
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 [:bsmedberg] 2006-05-15 14:03:24 PDT
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 [:bsmedberg] 2006-05-15 14:15:26 PDT
Created attachment 222099 [details] [diff] [review]
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 [:bz] 2006-05-15 14:17:06 PDT
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 [:bz] 2006-05-15 14:19:53 PDT
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 [:bsmedberg] 2006-05-15 14:22:33 PDT
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 2006-05-15 14:29:48 PDT
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 2006-05-15 16:05:34 PDT
> 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 [:bsmedberg] 2006-05-16 00:28:13 PDT
Created attachment 222149 [details] [diff] [review]
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 [:bsmedberg] 2006-05-16 02:02:02 PDT
Created attachment 222154 [details] [diff] [review]
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 2006-05-18 07:20:50 PDT
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 [:bsmedberg] 2006-05-19 16:34:07 PDT
Fixed on trunk.
Comment 14 Benjamin Smedberg [:bsmedberg] 2006-05-22 12:17:16 PDT
Fixed on 1.8 branch
Comment 15 Benjamin Smedberg [:bsmedberg] 2006-05-22 13:16:48 PDT
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 2006-05-22 13:45:33 PDT
perhaps we should verify that the URL resulting from URL resolution is still a chrome: URL?
Comment 17 Benjamin Smedberg [:bsmedberg] 2006-05-22 15:47:18 PDT
Except it's not, it's typically a jar:file:/// url.
Comment 18 Daniel Veditz [:dveditz] 2006-05-26 11:27:38 PDT
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] 2006-05-26 11:28:33 PDT
Comment on attachment 222154 [details] [diff] [review]
Use GetFilePath

We need to get this into 1.8.0.5
Comment 20 Daniel Veditz [:dveditz] 2006-06-05 11:26:22 PDT
Comment on attachment 222154 [details] [diff] [review]
Use GetFilePath

approved for 1.8.0 branch, a=dveditz for drivers
Comment 21 Benjamin Smedberg [:bsmedberg] 2006-06-07 09:54:22 PDT
Fixed on MOZILLA_1_8_0_BRANCH
Comment 22 Jay Patel [:jay] 2006-06-26 16:31:52 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.