404252 - Potential XSS vulnerability because of U+0008 being treated as whitespace

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, defect, P2)

Description

[Alexey Proskuryakov]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en) AppleWebKit/525.1+ (KHTML, like Gecko) Safari/419.3
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; ru; rv:1.8.1.9) Gecko/20071025 Firefox/2.0.0.9

While investigating how bug 381412 affects WebKit, I've noticed a similar issue in Firefox. Because of U+0008 being treated as whitespace, it is possible to disguise dangerous attributes (and possibly elements) as unknown ones.

MSIE and HTML5 spec agree that U+0008 is not whitespace. WebKit had problems that I've just fixed, but I don't know when this fix will be released by Apple.

Reproducible: Always

Steps to Reproduce:
Open the attached test case. Line with "08" should be green. Ideally, all lines should be green, but the other two Firefox failures are in other direction (characters that should be whitespace aren't), so they cannot be used for such an exploit.

Comment 1

[Alexey Proskuryakov]

Attachment: test case

Comment 2

[Daniel Veditz [:dveditz]]

The HTML 4.01 spec agrees, back-space isn't whitespace:
http://www.w3.org/TR/html401/struct/text.html#whitespace

The spec also defines zero-width space (0x200b) as whitespace, but that sounds like it could be dangerous and unexpected for web sites.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Blake, let us know if you need help here.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v1

This appears to give us the desired compatibility with IE for backspace. I'm not sure what to do about \v (0x00B), but HTML4 says that \f (0x00C) is whitespace. Johnny, what do you think?

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 294095 [details] [diff] [review]
patch v1

r+sr=jst for this. I think for \f and \v we'd probably be best off matching what IE does, and then arguing for making the HTML 5 spec match that as well.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk. I filed bug 409436 on figuring out what to do with \f and \v.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Patch for the 1.8 branch

There were some whitespace differences between the two branches. I verified that this patch is the same by doing |diff -w 1.9patch thispatch| and noticing that none of the added or removed lines showed up.

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 294506 [details] [diff] [review]
Patch for the 1.8 branch

approved for 1.8.1.12, a=dveditz for release-drivers

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Fixed on the 1.8 branch.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Verified for 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. Repros with 2.0.0.11 cleanly.

Comment 11

[Alexander Sack]

Comment on attachment 294506 [details] [diff] [review]
Patch for the 1.8 branch

a=asac for 1.8.0.15

(unmodified distro patch)

Comment 12

[Reed Loden [:reed]]

MOZILLA_1_8_0_BRANCH:

Checking in parser/htmlparser/src/nsHTMLTokens.cpp;
/cvsroot/mozilla/parser/htmlparser/src/nsHTMLTokens.cpp,v  <--  nsHTMLTokens.cpp
new revision: 3.273.10.2; previous revision: 3.273.10.1
done
Checking in parser/htmlparser/src/nsScanner.cpp;
/cvsroot/mozilla/parser/htmlparser/src/nsScanner.cpp,v  <--  nsScanner.cpp
new revision: 3.145.12.2; previous revision: 3.145.12.1
done

Comment 13

[Carsten Book [:Tomcat]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050621 Firefox/3.0pre and the testcase

--> Verified fixed