732 bytes, text/html; charset=UTF-7
47 bytes, text/html; charset=ISO-8859-1
47 bytes, text/html; charset=
47 bytes, text/html; charset=
1.02 KB, text/html; charset=UTF-7
7.89 KB, patch
|Details | Diff | Splinter Review|
1.65 KB, patch
|Details | Diff | Splinter Review|
1.07 KB, patch
|Details | Diff | Splinter Review|
I think this is actually covering two independent issues. From the summary "(i)Frames that do not specify a charset inherit it from parent" May or may not be a bug. Is there a spec that covers this case? Probably not, and using the parent charset is a reasonable inference that would tend to make sloppy sites work. We could also justify a strict "no charset == ISO-8859-1" policy, possibly breaking sites but making life easier for site filters. A second solution would be to completely drop support for UTF-7 in the browser. RFC 2152 says "UTF-7 should only be used with 7 bit transports such as mail. In other contexts, use of straight Unicode or UTF-8 is preferred." But we'd still have to support it in e-mail so we might be able to compile it out and have Firefox/Thunderbird work, but SeaMonkey poses additional problems.
Is UTF-7 the only charset that causes this kind of problem? I would have thought that forcing any strange charset could cause problems for content filtering. Seems to me that it is the responsibility of "other server" to make sure it sets its own charset, or it can be abused.
I guess the other browser developers think a little bit different, because Firefox seems to be the only one affected by this bug. There is no reason why a webpage should be allowed to change the charset of another webpage. (Atleast on other domains). It doesn't matter that this only occurs when the other server/page is not sending a charset. (I am still not convinced that there is not another trick that will allow this even when a charset is set) Keep in mind that this little "no-charset" problem was also present in google.com not long ago. Luckily for you at that time all people blamed only IE for autoguessing the charset. With the trick described here it would have been trivial to steal google credentials from Firefox users also... In that case even IE is more secure because it is only affected if you explicitly allow autodetection.
> using the parent charset is a reasonable inference that would tend to make > sloppy sites work. More importantly, without this a lot of non-European sites break. At least last we checked (a few years ago). > A second solution would be to completely drop support for UTF-7 in the > browser. There's no sane way to make that fly with XULRunner, unless we want random XULRunner apps that need that support packaging the UTF-7 encoder. I think the right fix is to not use the parent document charset in cross-domain situations, since then there's a good chance that it's not relevant anyway.
Created attachment 242032 [details] [diff] [review] So something like this (trunk patch) Sadly, the bug has no testcase, so I have no idea whether this helps. Can someone check whether it does? Also, this will be a bit of fun to merge to branches... Not terrible, but annoying.
That sounds like a good approach.
When comment 3 says that "Firefox is the only one affected by this bug" does that mean the only Gecko-based browser? If so, it seems odd that the fix would be in core code.
Forgive me my bad expression... I meant "browsers of the mozilla family" are the only ones affected by this. Internet Explorer, Opera, Safari do not inherit the charset from the parent. (I don't have konqueror here to test)
(In reply to comment #2) > Is UTF-7 the only charset that causes this kind of problem? I would have > thought that forcing any strange charset could cause problems for content > filtering. ISO-2022-JP and related encodings are likely to have a similar issue. (In reply to comment #4) > > using the parent charset is a reasonable inference that would tend to make > > sloppy sites work. > > More importantly, without this a lot of non-European sites break. At least > last we checked (a few years ago). Probably, it's not non-European sites but sites in languages for which multiple legacy encodings have been widely used (e.g. Japanese and Russian). I also think that attachment 242032 [details] [diff] [review] is a good compromise.
See comment 5. I'm not going to commit an untested patch, and this bug has no way to test attached to it, and I haven't the resources to make use of comment 0 (no web server for example). Need a test set up for this to proceed.
Stefan Esser, if there is a test server I could test this with that you don't want to make public, please feel free to send me the URI in private mail...
Created attachment 247715 [details] utf7 page framing a page with a utf-7 XSS attack Here's a testcase of a page that is itself UTF-7 framing a page on another site that does not specify a character set. If that page is interpreted as UTF-7 you'll get an alert. There's also a frame pointing at the identical content on a server that sends an ISO-8859-1 charset header (but not my server so subject to change) I'm not convinced this is more than a minor problem since sites that allow user-generated content to the extent that they're trying to filter things ought to specify the character encoding they think they're filtering in. But if there is such a site then this could be used to create an attack page against users of that site (the victims would have to be lured to your framing page, you couldn't leave it entirely on the target site).
So... I can't reproduce the reported problem with that testcase in a trunk build.
Turns out the default charset Bugzilla sends as an HTTP header overrides the <meta> tag in the document; looks like you can override that in the attachment details, though, and then the testcase shows the problem. Our normal precedence can be seen in the order of the #defines here: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/parser/htmlparser/public/nsIParser.h&rev=1.41#91 (In reply to comment #3) > I guess the other browser developers think a little bit different, because > Firefox seems to be the only one affected by this bug. Contrary to this assertion the behavior is exactly the same in both IE7 and Opera -- the frame with the explicit charset is fine, the other shows the xss alert. That makes the proposed fix a lot more scary because there could be a large installed-base relying on this behavior. > In that case even IE is more secure because it is only affected if you > explicitly allow autodetection. Not my experience. It's possible my testcase is for a different case than the one you were describing -- do you have one of your own we could try?
IE7 does introduce this security problem. Yes. Previous versions of IE unlike Firefox did not have this security bug. It is unbelievable that two month have passed and there is still no security fix for this. I will release an advisory during the next days and then we can let the community decide if they consider this a security bug or not.
> That makes the proposed fix a lot more scary because there could be a > large installed-base relying on this behavior. But if IE6 didn't have this security hole, there can't be *that* many sites relying on charset inheritance.
BTW: I just verified that Opera 9 falls for this problem, too Opera 8 is not affected
> It is unbelievable that two month have passed Well. I posted a patch on 2006-10-11, but had no way to test it (as I pointed out then, the bug report doesn't have a link to a page showing the problem or anything). So about two months were spent waiting for confirmation that this is in fact a problem and creation of a testcase that could be used to test the fix. I note that you never responded to my requests for a testcase (even just to the point of saying "I have no plans to provide you guys with one, sort it out yourselves").
Boris, are you able to reproduce with dveditz' testcase now that he tweaked its content-type?
So I tried, and this patch does seem to help that latest testcase. I'll try to set up a testcase that's completely within bugzilla later today, I guess...
Comment on attachment 242032 [details] [diff] [review] So something like this (trunk patch) jst, what do you think? Note about testing this: Once you have loaded it in a buggy build, we'll cache the last charset we used for this page, as far as I can tell, and use that as the last fallback for the charset. So to test the fix, clear your cache first.
Boris, the intial report contains Proof Of Concept Code. In other words a testcase existed from day 0. You only had to copy 3 files from within my bugreport onto a webserver. It is not my duty to provide you with a server and it is also not my duty to react on questions for a testcase when such a testcase was already provided. So don't blame the inactivity of your security response team on me.
Stefan, I have no web server to work with, sorry. I'm not blaming you for anything, just pointing out why this took so long to happen. A testcase, in this bug database, is something that can actually be loaded to test the bug, for what it's worth.
Comment on attachment 251181 [details] Testcase subframe 1 (served as ISO-8859-1) Er, this should be served as ISO-8859-1
Comment on attachment 242032 [details] [diff] [review] So something like this (trunk patch) + // content viewer set up yet, and therefore do not have a usedul Typo in "useful". r+sr=jst
Created attachment 251245 [details] [diff] [review] Updated to comments I'll land on the trunk once the tree reopens.
Let's get this landed on the Trunk for some bake time over the weekend and we can land on the branches next week if things look good.
Fixed on trunk. Need multiple-host support in mochitest to land tests.
Comment on attachment 251245 [details] [diff] [review] Updated to comments Approved for both branches, a=jay for drivers. Please land asap. Thanks!
Comment on attachment 251245 [details] [diff] [review] Updated to comments Please do NOT land this on the branches yet -- sorry for any confusion. We need to better understand why IE7 and Opera 9 switched to mimic our original behavior first.
Opera 8.54 on Linux on my testcase shows no alerts at all, not even the "SSS" one. So it doesn't inherit the charset even for same-site pages. Thus, I would think they made the change for the reasons listed in comment 4 -- sites broke without it. What behavior did IE6 use to have?
> We need to better understand why IE7 and Opera 9 switched to mimic our original > behavior first. Has anyone asked the IE / Opera teams? And/or informed them that it leads to XSS on some sites? Did the change in IE / Opera affect all framing or only the cross-site case?
The same issue was reported by me to Microsoft several month ago. At the 16.10. I received an reply, that they received my mail. The e-mail was actually an accusation that I had reported this issue public before having talked to Microsoft, which is amusing, because I do not know a public report about this issue (especially not by me). Additionally I emailed Microsoft a few minutes after I realized that Internet Explorer 7 changed the behaviour. Until that point I did not even know that IE 7 was affected. Opera on the other hand. One of the people here in the thread said that Opera 9 is affected and I tested it. It turned out that I previously had tested with an Opera 8 installation that does not show this behaviour. I reported this issue to Opera via their security bugtracker but have not gotten any kind of reply.
oh great. so i'm working on a web browser and we've been relying on the only documentation we can find about charset fallback. that URL is here: http://www.mozilla.org/projects/intl/uidocs/browsercharmenu.html if someone has a newer document on the subject, please let me know (and please update the old document to link to it). If not, given that you're now changing the fallback behavior, I respectfully request that you document it somewhere useful including listing a note about this change. Specifically had I not seent his bug fly by last night, I'd have pretty much suffered from a frozen UI requirement to implement something which gecko was about to unimplement.
> Did the change in IE / Opera affect all framing or only the cross-site case? For Opera, all sites. See comment 36. For IE6, I asked someone to test on this testcase, but IE's somehow getting back the "you didn't pick an attachment to view" page from Bugzilla. Someone who actually has IE6 might need to modify Dan's testcase to run all on a single site so that can be tested. Or something... timeless, I'll add documenting this to my to-do list, but it's a lower priority item than crash fixes, security fixes, and layout fixes and regression tests. Which means I doubt I'll ever get to it. You may want to just file a bug on the documentation component requesting this documentation...
(In reply to comment #39) > so i'm working on a web browser and we've been relying on the only > documentation we can find about charset fallback. > > http://www.mozilla.org/projects/intl/uidocs/browsercharmenu.html That's very old (1999); I'm not sure it describes Mozilla accurately. In particular I think we ended up following the Inheritance behavior described for 4.x, probably because we broke sites.
OK, gavin tested IE6 and IE6 doesn't inherit for same-site pages, just like Opera 8.
Comment on attachment 251245 [details] [diff] [review] Updated to comments Moving approval flags out to 126.96.36.199/188.8.131.52. We need to decide what we want to do with this bug for next time.
Um... So what are "we" deciding and who is the "we"? What other information are you looking for? Or just more trunk baking time? I'm pretty certain that we should take this on branch, for what it's worth...
bz: We = everyone in this bug... and it sounded like we were not sure which behavior we wanted to have with 184.108.40.206 (given the behavior of older versions of Opera 8/IE6 vs the latest Opera 9/IE7). If we are sure this patch is what we want, then let's try to get this in. Dveditz: Who should make the call on this one?
Comment on attachment 251245 [details] [diff] [review] Updated to comments we've not gotten responses from MS or Opera. Damn the torpedoes let's do this. a=dveditz for 1.8/1.8.1 branches Without the patch our users are vulnerable. With the patch some sites look bad. Wish we knew how many of each there were going to be :-(
Fixed on branches.
Verified fixed on trunk, using the "Testcase, hopefully. Log in to test1.bugzilla.mozilla.org first" testcase. I can see the XSS alert with the 2007-01-12 build, but not anymore with the 2007-01-13 build. Also verified fixed that the testcase doesn't show the XSS alert, using the latest branch builds.
Before I write this up, I'd like to double-check my (very quick) reading of the code: This patch appears to me to adjust things so that the parent's charset can only be inherited if the frame and parent were loaded from the site origin. Correct, or no?
Yes, with s/site/same/.
This is now documented on both the Fx3 for developers page and the page on updating sites for Firefox 3. Marking as documentation complete, since we do not presently have our own docs for frames or iframes. Thanks for confirming my read of the code, Boris (and pointing out my typo ;).
I found that Firefox 220.127.116.11 will inherit the charset of the parent page, when that had been selected manually (does not inherit the charset specified in headers or meta). I found this inheritance to work both with [a href] links and [iframe src] in the parent page. There are still many Apache sites that allow UTF-7 XSS and do not set explicit charset: see CVE-2007-4465.
> when that had been selected manually Charset overrides are applied to the entire frame tree, yes.
So I can XSS by having a hidden iframe, making my page "look broken", and asking users to override the charset to UTF-7 to "unbreak" it? :/
> So I can XSS by ... Yes, exactly. That was going to be my "example" if I was asked to demonstrate this is unsafe. This "charset override applies to entire frame tree" seems to contradict the earlier statement of "parent's charset can only be inherited if the frame and parent were loaded from the same origin".
We can try to change the override behavior, but the issue then is that there will be no way to apply the override to a subframe. Of course if you allow things like "ask users to X", I suspect we have all sorts of XSS vectors (drag and drop, etc, etc).
By the way, of more concern to me is whether the page can affect the default docshell charset (e.g. by doing a window.open from a UTF8 page). That seems like a much more serious situation, if it works... Can someone test?
> ... there will be no way to apply the override to a subframe. There should be no way to affect a subframe that is not "ours" (same origin). I do not know what is the best way to handle "our" subframes: I guess let the override propagate. The override/inherit code should first check whether we are same-origin, and quit (not do anything) unless so.
That doesn't address the concern I raised.
In any case, this discussion belongs in a separate bug, with some UI folks cced.
> That doesn't address the concern I raised. No, I did not reply to your window.open query: I do not know whether anything would affect, and do not think I would know how to test with any confidence. I only replied to your "apply override to a subframe" comment. > ... separate bug, with some UI folks cced. Make it into a separate bug if you wish, do not have a preference either way. However, please do not let the UI people "in": this is a security issue. Make sure no "foreign" subframes/windows/anything can be affected. Only ask the UI people what to do with "our" subframes: but there is no issue there, leave it as is now.
> However, please do not let the UI people "in": this is a security issue. The fix for the security issue would break the UI. Therefore they need to be in on the discussion about how to best fix the security issue while changing the UI at the same time so it continues to work. > what to do with "our" subframes: Note that whether a subframe is "our" is not an immutable value. It can change.
I now see that Firefox (tested on Debian etch) makes iframes inherit the charset of the parent, even when the frame defines its own. So this is really buggy... and allows XSS against practically any target.
This is still about the charset override, yes? The charset override... overrides the charset. That's what it's supposed to do. Again, that's a separate issue from this bug.
Please see message http://lists.grok.org.uk/pipermail/full-disclosure/2007-December/058814.html and demo http://www.maths.usyd.edu.au/u/psz/ff-utf7-uxss.html Whether this is all "this same" bug or something else, I wouldn't know; please make it into new bug anytime if you wish.
I'm not sure why you expect someone else to do the work for you, but I filed bug 406777.