Received the following via mail: -------------------------------- The issue is that Firefox will follow 302 redirects on <LINK REL="stylesheet" HREF="..."> requests, *and* then allow the target URL to be read by accessing element.sheet.href property. This sounds like a non-issue, but has profound implications for a number of contemporary websites, most notably: 1) Many webpages use SSO mechanisms and pass temporary authentication tokens in URLs to set cookies for the target domain, then safely consume the token. By pointing an already authenticated user to 'sso.example.com/auth?cont=fun.example.com' and reading back 'fun.example.com/auth_ok?secret_token' target URL, user accounts can be compromised on a large number of high-profile sites. 2) Many webpages redirect an already authenticated user from main page (fun.example.com) by appending URL information that may temporarily contain sensitive data, such as session IDs, mailbox names, user names, and the like (fun.example.com/home/john.doe/hot_movies;jsessionid=foo). These would be disclosed to third parties, severely violating user's privacy, especially if done at a large scale. Because the problem affects a number of sites and is hard to fix on server side, we would prefer not to have to explicitly explain these attack scenarios in a Bugzilla entry (and without that additional explanation, the impact is not very obvious). (We have no problem with the existence of a fix being communicated to users once it is available, naturally; Martin Straka should be credited as the original reporter if you wish to do so). ---------------------------------------------------- The above did not come with a PoC, but the attached file shows the difference between the link href and the sheet href. Mozilla.com's support page isn't a real stylesheet, but it's a handy redirected page.

The DOM 2 spec is unhelpfully vague about the meaning of the sheet's href property. We have chosen to show the redirected URI much as we do for HTML pages loaded at the top-level and in iframes. We use the redirected URI as the codebase principal used to determine if the loading page can access the CSSRules and other content of the sheet. We could extend that blocking to the sheet's href property if it's on a different domain. That would throw an exception however, and code written for other browsers might not expect that. IE and Opera simply copy the href value from the link element.

What do IE and Opera do for @import-ed sheets if that URI redirects?

And I guess I don't understand why this is not a problem for iframes? Or is it that for iframes after such a redirect we don't allow reading the URI? We should really bring this up in whatever working group is working on standardizing this (webapi)?

Put another way, I'm happy to implement the IE/Opera behavior if someone can describe it and if we propose it for standardization. I'm frankly also happy to just implement nsIScriptObjectPrincipal on stylesheets. That would have some issues though, so I'd probably rather do the former.

Personally I think we're doing the right thing here. I would imagine that a website that relies on the redirected uri to be secret is already on thin ice. For example, what happens if you in such a stylesheet have a rule like div { background: url(#a); } and then call getComputedStyle(myDiv, "")?

The idea here, as I undestand it, is that we have three sites: evil.example.com, sso.example.com, and fun.example.com. evil.example.com includes a link to a stylesheet URI like 'sso.example.com/auth?cont=fun.example.com'. Then it reads the sheet URI, which post-redirect is 'fun.example.com/auth_ok?secret_token'. It doesn't have any control over the sheet (which is not even valid CSS in this case, being just the fun.example.com website). I agree that this is not a particularly secure setup on the face of it, but I can't think of obvious holes in it other than this one... so far.

Note that the case I described can be addressed on sso.example.com by doing a referrer check, from what I can see. The second case listed in comment 0 is more a matter of "don't do that", but if sites are doing it and it's safe in all other UAs we may just need to make it safe in Gecko too. Also note that it's pure luck that the URI scripts get compiled with as the "filename" with is the pre-redirect URI and not the post-redirect one. Otherwise this same issue would pop up with onerror handlers too. Quite frankly, trying to catch all places where a post-redirect URI (which is, after all the real URI of the resource) might accidentally be exposed to script seems like a losing battle to me. Especially because there's nothing a priori insecure about it except for assumptions certain sites make about such post-redirect URIs not being available to scripts...

(In reply to comment #3) > And I guess I don't understand why this is not a problem for iframes? Or is it > that for iframes after such a redirect we don't allow reading the URI? Yeah, one thing I suggested to dveditz when we discussed this in person before he filed the bug was that maybe we should expand the cross-site scripting check to cover reading the URI of the sheet. (That said, if nobody else has an XSS check there, it could be a problem.)

(In reply to comment #2) > What do IE and Opera do for @import-ed sheets if that URI redirects? Consistent with what they do for directly linked sheets. Given a sheet whose only rule is an import of a redirected sheet Gecko: sheet.cssRules[0].href as written in the sheet ("sheet302.css") sheet.cssRules[0].styleSheet.href redirected URI Opera: sheet.cssRules[0].href as written but expanded to absolute sheet.cssRules[0].styleSheet.href ditto (http://localhost/test/sheet302.css) IE7: sheet.imports[0].href as written ("sheet302.css")

Daniel, do you have a testcase for comment 9 by any chance?

(In reply to comment #6) > The idea here, as I undestand it, is that we have three sites: > evil.example.com, sso.example.com, and fun.example.com. evil.example.com > includes a link to a stylesheet URI like > 'sso.example.com/auth?cont=fun.example.com'. Then it reads the sheet URI, > which post-redirect is 'fun.example.com/auth_ok?secret_token'. It doesn't > have any control over the sheet (which is not even valid CSS in this case, > being just the fun.example.com website). What if you use the CSSOM to insert a rule into the stylesheet like the one I described? Or if the stylesheet contains rules like: div { background-image: attr(background, url); } Though no UA currently support that. In any case, I'm fine with limiting support to the .href property if it's coming from a different origin than the loading page. Or we could maybe even disallow access to the entire stylesheet object in that case?

> What if you use the CSSOM to insert a rule into the stylesheet In Gecko, you can only do that with sheets that you control, effectively. See bug 310165 comment 19. But yes, if the result is actually a stylesheet, you might be able to get info out of it. I agree that's a problem these servers need to solve with their authentication setup, and a potential security hole. For them. In the interests of interop, I actually think we should do what Opera/IE do as far as .href goes. Esp. because it's easy and pretty much guaranteed to not break pages. Patch coming up.

David, would you review the CSS parts of this? Christian, would you look over the nsNetUtil change and its callers? Daniel, thanks for the testcases!

Comment on attachment 282356 [details] [diff] [review] Proposed fix Oh, and the key changes here, of course, are in nsCSSStyleSheet::GetHref and in nsCSSLoader. The rest is cosmetic.

So does anyone know if it's possible to use the CSSOM to reach into the stylesheet and insert a rule like the one in comment 5? If so that's a security problem. If the problem is with the site or the browser is a matter of debate.

Actually it seems to me that the hole sicking points out (pointing a <link rel=stylesheet> at a redirecting page, and then poking a rule into it that uses a relative URI, and then getting the computed style of an element using that rule) is a real hole. It doesn't even have to be a stylesheet on the other end, since we ignore Content-Type headers in quirks mode. (Yet another security hole caused by content type sniffing, woo.)

Don't we have XSS restrictions on the rules in a style sheet? (I was hoping the way to fix this would be adding XSS restrictions on the location, too, but oh well...)

Yeah, for the mutation issue it seems like we need restrictions that prevent writing (at least) to a stylesheet cross-origin, if we don't already.

> So does anyone know if it's possible to use the CSSOM That's exactly what you asked in comment 12. I answered it in comment 13. Ian, you probably want to read that part too. I'm open to an XSS restriction on the location, but it has a higher chance of breaking sites due to unexpected security exceptions...

Comment on attachment 282356 [details] [diff] [review] Proposed fix >- // If the channel's original URI is "chrome:", we want that, since >- // the observer code in nsXULPrototypeCache depends on chrome stylesheets >- // having a chrome URI. (Whether or not chrome stylesheets come through >- // this codepath seems nondeterministic.) >- // Otherwise we want the potentially-HTTP-redirected URI. It seems to me that you want to keep some form of this comment (assuming it's still relevant at all). I'm assuming that you're still enforcing it, since the way we handle chrome: URIs doesn't set LOAD_REPLACE, right? Looks fine otherwise. r+sr=dbaron

Hi guys, Noticed no activity for a while; what's the plan on this one?

It's waiting for approval to land on trunk as well as review from Christian. After this lands on trunk, I will probably try to port this patch to branch.

Comment on attachment 282356 [details] [diff] [review] Proposed fix Approved to land for beta. Please land ASAP.

Comment on attachment 282356 [details] [diff] [review] Proposed fix r=biesi on the nsNetUtil change

Checked in on trunk. I'll try to work on branch patch, I guess...

This includes the fix for bug 74281 (but not any of the other cleanup I did on trunk), but I could take that out too, if desired.

Comment on attachment 285964 [details] [diff] [review] Branch patch > baseURI = aLinkingContent->GetBaseURI(); > sheetURI = aLinkingContent->GetDocument()->GetDocumentURI(); >- } >+ originalURI = nsnull; I'm thinking it's probably better to stick to originalURI = sheetURI for the branch, and not take that fix. > NS_IMETHODIMP > nsCSSStyleSheet::GetHref(nsAString& aHref) > { >- nsCAutoString str; >- >- // XXXldb The DOM spec says that this should be null for inline style sheets. >- if (mInner && mInner->mSheetURI) { >- mInner->mSheetURI->GetSpec(str); >+ if (mInner->mOriginalSheetURI) { Maybe we should leave the mInner null check? We probably ought to figure out a better allocation failure strategy for mInner for the trunk... >@@ -655,17 +655,19 @@ CSSParserImpl::Parse(nsIUnicharInputStre Seems like this is a somewhat dangerous entry point -- we probably want to document this better on the trunk, and check the callers to make sure that the URI's they're passing in are appropriate for this use. r+sr=dbaron

(In reply to comment #28) > I'm thinking it's probably better to stick to originalURI = sheetURI for the > branch, and not take that fix. ...although I don't feel at all strongly about it.

> I'm thinking it's probably better to stick to originalURI = sheetURI Done. > Maybe we should leave the mInner null check? Done. > Seems like this is a somewhat dangerous entry point -- we probably want to > document this better on the trunk, and check the callers to make sure that the > URI's they're passing in are appropriate for this use. The only caller of nsICSSParser::Parse() is the CSS loader... It's doing the right thing (in fact it hands the parser a sheet to parse into). On trunk, I think we should remove the ability to not pass a sheet in to the CSS parser, and remove the out param on Parse(). Seem ok?

Sounds good.

Filed bug 404315 on Parse() improvement.

Comment on attachment 289039 [details] [diff] [review] Updated to comments approved for 1.8.1.12, a=dveditz for release-drivers

Fixed on branch.

Verified on branch with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.12pre) Gecko/2008011803 BonEcho/2.0.0.12pre. Both the of the test cases work properly now.

distro patches block 1.8.0.15

Comment on attachment 289039 [details] [diff] [review] Updated to comments looks like we ship this unmodified in distros already. caillon, please sign off for 1.8.0.15.

Comment on attachment 289039 [details] [diff] [review] Updated to comments a=caillon for 1.8.0.15

committed this to the branch