[Webkit-unassigned] [Bug 27895] [XSSAuditor] Inline Event Handler with single-line JavaScript comment can bypass XSSAuditor

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27895





--- Comment #5 from Daniel Bates <dbates at webkit.org>  2009-08-05 17:36:58 PDT ---
Let "Proposed Patch 1" be called Patch 1, and "Alternative patch" be called
Patch 2.

I am tending to favor the Patch 1, because it has the advantage of fixing the
issue without clobbering the value of the inline event handler or base href.
Thus, a JavaScript DOM traversal matches the structure seen when you View
Source, even though the inline event handler and base href are ignored.

Additionally, it does not produce any false positives compared to Patch 2
(though, from a conversation with Adam, we can probably get away with it since
such false positives are detected as part of an injection attempt). 

Examples:

1) Inline Event Handler
http://good.webblaze.org/dbates/xsstest-printDOM.php?q=%3Cimg%20src=%22about:blank%22%20onerror=%22alert(/XSS/)%22/%3E

2) Base Href
http://good.webblaze.org/dbates/xsstest-printDOM-basehref.php?q=<base
href="http://evil.webblaze.org/dbates/base-href/">

Notice, either patch prevents the execution of the above, but Patch 1 does not
nullify neither the onerror nor href attributes so the the DOM traversal
matches the View Source structure.

False positive for Patch 2:
http://good.webblaze.org/dbates/xsstest.php?q=<img src="about:blank"
ondummy="dummy"/>

Because Patch 2 only looks for attributes that start with "on" and whose length
is >= 5 characters (*), this is detected as an attack, even though ondummy is
not a valid attribute

Difference between "Proposed Patch 1" (Patch 1) and "Alternative patch" (Patch
2):

Patch 1 defines a subclass of MappedAttribute, called
MappedAttributeWithRawCharacters (I'm open to name suggestions) that stores 
the value of |m_rawAttributeBeforeValue| extracted in the HTMLTokenizer. The
HTMLTokenizer creates attributes of class MappedAttributeWithRawCharacter. So,
such attributes are passed along through the WebKit framework. If the attribute
A corresponds to an inline event handler and
A->isMappedAttributeWithRawCharacters() is true then in ScriptEventListener we
cast it to MappedAttributeWithRawCharacter and pass the stored |
m_rawAttributeBeforeValue| to the XSSAuditor. Similarly, we do the same in
HTMLBaseElement.

Patch 2 is implemented in the HTMLTokenizer. It detects attributes described by
(*) or detects the href attribute of a <base> tag. It then passes this
information to the XSSAuditor. If the XSSAuditor methods return false then the
corresponding attribute is set to the empty string (hence the clobbered
result).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.