- Revision
- 207848
- Author
- [email protected]
- Date
- 2016-10-25 15:07:20 -0700 (Tue, 25 Oct 2016)
Log Message
REGRESSION (r178265): XSS Auditor fails to block document.write() of incomplete tag
https://bugs.webkit.org/show_bug.cgi?id=163978
<rdar://problem/25962131>
Reviewed by Darin Adler.
Source/WebCore:
During the tokenization process of an HTML tag the start and end positions of each of its
attributes is tracked so that the XSS Auditor can request a snippet around a suspected
injected attribute. We need to take care to consider document.write() boundaries when
tracking the start and end positions of each HTML tag and attribute so that the XSS Auditor
receives the correct snippet. Following r178265 we no longer consider document.write()
boundaries when tracking the start and end positions of attributes. So, the substring
represented by the start and end positions of an attribute may correspond to some other
attribute in the tag. Therefore the XSS Auditor may fail to block an injection because the
snippet it requested may not be the snippet that it intended to request.
Tests: http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror.html
http/tests/security/xssAuditor/dom-write-location-open-img-onerror.html
http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror.html
* html/parser/HTMLSourceTracker.cpp:
(WebCore::HTMLSourceTracker::startToken): Set the attribute base offset to be the token
start position.
(WebCore::HTMLSourceTracker::source): Use the specified attribute start position as-is. We no
longer adjust it here because it was adjusted with respect to the attribute base offset, which
takes into account document.write() boundaries.
* html/parser/HTMLToken.h:
(WebCore::HTMLToken::setAttributeBaseOffset): Added.
(WebCore::HTMLToken::beginAttribute): Subtract attribute base offset from the specified offset.
(WebCore::HTMLToken::endAttribute): Ditto.
* html/parser/HTMLTokenizer.h:
(WebCore::HTMLTokenizer::setTokenAttributeBaseOffset): Added.
LayoutTests:
Add tests to ensure that the XSS Auditor blocks a document.write() of an incomplete HTML image tag.
* http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror-expected.txt: Added.
* http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror.html: Added.
* http/tests/security/xssAuditor/dom-write-location-open-img-onerror-expected.txt: Added.
* http/tests/security/xssAuditor/dom-write-location-open-img-onerror.html: Added.
* http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror-expected.txt: Added.
* http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror.html: Added.
* http/tests/security/xssAuditor/resources/echo-nested-dom-write-location.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (207847 => 207848)
--- trunk/LayoutTests/ChangeLog 2016-10-25 22:01:55 UTC (rev 207847)
+++ trunk/LayoutTests/ChangeLog 2016-10-25 22:07:20 UTC (rev 207848)
@@ -1,3 +1,21 @@
+2016-10-25 Daniel Bates <[email protected]>
+
+ REGRESSION (r178265): XSS Auditor fails to block document.write() of incomplete tag
+ https://bugs.webkit.org/show_bug.cgi?id=163978
+ <rdar://problem/25962131>
+
+ Reviewed by Darin Adler.
+
+ Add tests to ensure that the XSS Auditor blocks a document.write() of an incomplete HTML image tag.
+
+ * http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror-expected.txt: Added.
+ * http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror.html: Added.
+ * http/tests/security/xssAuditor/dom-write-location-open-img-onerror-expected.txt: Added.
+ * http/tests/security/xssAuditor/dom-write-location-open-img-onerror.html: Added.
+ * http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror-expected.txt: Added.
+ * http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror.html: Added.
+ * http/tests/security/xssAuditor/resources/echo-nested-dom-write-location.html: Added.
+
2016-10-25 Brady Eidson <[email protected]>
IndexedDB 2.0: Support IDBObjectStore openKeyCursor.
Added: trunk/LayoutTests/http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror-expected.txt (0 => 207848)
--- trunk/LayoutTests/http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror-expected.txt 2016-10-25 22:07:20 UTC (rev 207848)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 6: The XSS Auditor refused to execute a script in 'http://localhost:8000/security/xssAuditor/resources/echo-dom-write-location.html?q=%3Cscript%3Edocument.write(%22%3Cimg%20src=1%20%22);%3C/script%3Eonerror=alert(/XSS/)' because its source code was found within the request. The auditor was enabled because the server did not send an 'X-XSS-Protection' header.
+
Added: trunk/LayoutTests/http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror.html (0 => 207848)
--- trunk/LayoutTests/http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror.html 2016-10-25 22:07:20 UTC (rev 207848)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src=''>
+</iframe>
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/security/xssAuditor/dom-write-location-open-img-onerror-expected.txt (0 => 207848)
--- trunk/LayoutTests/http/tests/security/xssAuditor/dom-write-location-open-img-onerror-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/dom-write-location-open-img-onerror-expected.txt 2016-10-25 22:07:20 UTC (rev 207848)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 7: The XSS Auditor refused to execute a script in 'http://localhost:8000/security/xssAuditor/resources/echo-dom-write-location.html?q=%3Cimg%20src=1%20onerror=alert(/XSS/)' because its source code was found within the request. The auditor was enabled because the server did not send an 'X-XSS-Protection' header.
+
Added: trunk/LayoutTests/http/tests/security/xssAuditor/dom-write-location-open-img-onerror.html (0 => 207848)
--- trunk/LayoutTests/http/tests/security/xssAuditor/dom-write-location-open-img-onerror.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/dom-write-location-open-img-onerror.html 2016-10-25 22:07:20 UTC (rev 207848)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src=''>
+</iframe>
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror-expected.txt (0 => 207848)
--- trunk/LayoutTests/http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror-expected.txt 2016-10-25 22:07:20 UTC (rev 207848)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 7: The XSS Auditor refused to execute a script in 'http://localhost:8000/security/xssAuditor/resources/echo-nested-dom-write-location.html?q=%3Cimg%20src=1%20onerror=alert(/XSS/)' because its source code was found within the request. The auditor was enabled because the server did not send an 'X-XSS-Protection' header.
+
Added: trunk/LayoutTests/http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror.html (0 => 207848)
--- trunk/LayoutTests/http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror.html 2016-10-25 22:07:20 UTC (rev 207848)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src=''>
+</iframe>
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-nested-dom-write-location.html (0 => 207848)
--- trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-nested-dom-write-location.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-nested-dom-write-location.html 2016-10-25 22:07:20 UTC (rev 207848)
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<script>document.write("<script>document.write(unescape(window.location));</" + "script>");</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (207847 => 207848)
--- trunk/Source/WebCore/ChangeLog 2016-10-25 22:01:55 UTC (rev 207847)
+++ trunk/Source/WebCore/ChangeLog 2016-10-25 22:07:20 UTC (rev 207848)
@@ -1,3 +1,38 @@
+2016-10-25 Daniel Bates <[email protected]>
+
+ REGRESSION (r178265): XSS Auditor fails to block document.write() of incomplete tag
+ https://bugs.webkit.org/show_bug.cgi?id=163978
+ <rdar://problem/25962131>
+
+ Reviewed by Darin Adler.
+
+ During the tokenization process of an HTML tag the start and end positions of each of its
+ attributes is tracked so that the XSS Auditor can request a snippet around a suspected
+ injected attribute. We need to take care to consider document.write() boundaries when
+ tracking the start and end positions of each HTML tag and attribute so that the XSS Auditor
+ receives the correct snippet. Following r178265 we no longer consider document.write()
+ boundaries when tracking the start and end positions of attributes. So, the substring
+ represented by the start and end positions of an attribute may correspond to some other
+ attribute in the tag. Therefore the XSS Auditor may fail to block an injection because the
+ snippet it requested may not be the snippet that it intended to request.
+
+ Tests: http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror.html
+ http/tests/security/xssAuditor/dom-write-location-open-img-onerror.html
+ http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror.html
+
+ * html/parser/HTMLSourceTracker.cpp:
+ (WebCore::HTMLSourceTracker::startToken): Set the attribute base offset to be the token
+ start position.
+ (WebCore::HTMLSourceTracker::source): Use the specified attribute start position as-is. We no
+ longer adjust it here because it was adjusted with respect to the attribute base offset, which
+ takes into account document.write() boundaries.
+ * html/parser/HTMLToken.h:
+ (WebCore::HTMLToken::setAttributeBaseOffset): Added.
+ (WebCore::HTMLToken::beginAttribute): Subtract attribute base offset from the specified offset.
+ (WebCore::HTMLToken::endAttribute): Ditto.
+ * html/parser/HTMLTokenizer.h:
+ (WebCore::HTMLTokenizer::setTokenAttributeBaseOffset): Added.
+
2016-10-25 Chris Dumez <[email protected]>
IDBDatabase.transaction() should take a union in parameter
Modified: trunk/Source/WebCore/html/parser/HTMLSourceTracker.cpp (207847 => 207848)
--- trunk/Source/WebCore/html/parser/HTMLSourceTracker.cpp 2016-10-25 22:01:55 UTC (rev 207847)
+++ trunk/Source/WebCore/html/parser/HTMLSourceTracker.cpp 2016-10-25 22:07:20 UTC (rev 207848)
@@ -49,6 +49,7 @@
m_currentSource = currentInput;
m_tokenStart = m_currentSource.numberOfCharactersConsumed() - m_previousSource.length();
+ tokenizer.setTokenAttributeBaseOffset(m_tokenStart);
}
void HTMLSourceTracker::endToken(SegmentedString& currentInput, HTMLTokenizer& tokenizer)
@@ -92,7 +93,7 @@
String HTMLSourceTracker::source(const HTMLToken& token, unsigned attributeStart, unsigned attributeEnd)
{
- return source(token).substring(attributeStart - m_tokenStart, attributeEnd - attributeStart);
+ return source(token).substring(attributeStart, attributeEnd - attributeStart);
}
}
Modified: trunk/Source/WebCore/html/parser/HTMLToken.h (207847 => 207848)
--- trunk/Source/WebCore/html/parser/HTMLToken.h 2016-10-25 22:01:55 UTC (rev 207847)
+++ trunk/Source/WebCore/html/parser/HTMLToken.h 2016-10-25 22:07:20 UTC (rev 207848)
@@ -114,6 +114,9 @@
void setSelfClosing();
+ // Used by HTMLTokenizer on behalf of HTMLSourceTracker.
+ void setAttributeBaseOffset(unsigned attributeBaseOffset) { m_attributeBaseOffset = attributeBaseOffset; }
+
public:
// Used by the XSSAuditor to nuke XSS-laden attributes.
void eraseValueOfAttribute(unsigned index);
@@ -153,6 +156,8 @@
// For DOCTYPE
std::unique_ptr<DoctypeData> m_doctypeData;
+
+ unsigned m_attributeBaseOffset { 0 }; // Changes across document.write() boundaries.
};
const HTMLToken::Attribute* findAttribute(const Vector<HTMLToken::Attribute>&, StringView name);
@@ -315,7 +320,7 @@
m_attributes.grow(m_attributes.size() + 1);
m_currentAttribute = &m_attributes.last();
- m_currentAttribute->startOffset = offset;
+ m_currentAttribute->startOffset = offset - m_attributeBaseOffset;
}
inline void HTMLToken::endAttribute(unsigned offset)
@@ -322,7 +327,7 @@
{
ASSERT(offset);
ASSERT(m_currentAttribute);
- m_currentAttribute->endOffset = offset;
+ m_currentAttribute->endOffset = offset - m_attributeBaseOffset;
#if !ASSERT_DISABLED
m_currentAttribute = nullptr;
#endif
Modified: trunk/Source/WebCore/html/parser/HTMLTokenizer.h (207847 => 207848)
--- trunk/Source/WebCore/html/parser/HTMLTokenizer.h 2016-10-25 22:01:55 UTC (rev 207847)
+++ trunk/Source/WebCore/html/parser/HTMLTokenizer.h 2016-10-25 22:07:20 UTC (rev 207848)
@@ -43,6 +43,9 @@
class TokenPtr;
TokenPtr nextToken(SegmentedString&);
+ // Used by HTMLSourceTracker.
+ void setTokenAttributeBaseOffset(unsigned);
+
// Returns a copy of any characters buffered internally by the tokenizer.
// The tokenizer buffers characters when searching for the </script> token that terminates a script element.
String bufferedCharacters() const;
@@ -282,6 +285,11 @@
return TokenPtr(processToken(source) ? &m_token : nullptr);
}
+inline void HTMLTokenizer::setTokenAttributeBaseOffset(unsigned offset)
+{
+ m_token.setAttributeBaseOffset(offset);
+}
+
inline size_t HTMLTokenizer::numberOfBufferedCharacters() const
{
// Notice that we add 2 to the length of the m_temporaryBuffer to
