Changeset 207848 in webkit


Ignore:
Timestamp:
Oct 25, 2016 3:07:20 PM (7 years ago)
Author:
dbates@webkit.org
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.
Location:
trunk
Files:
7 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r207845 r207848  
     12016-10-25  Daniel Bates  <dabates@apple.com>
     2
     3        REGRESSION (r178265): XSS Auditor fails to block document.write() of incomplete tag
     4        https://bugs.webkit.org/show_bug.cgi?id=163978
     5        <rdar://problem/25962131>
     6
     7        Reviewed by Darin Adler.
     8
     9        Add tests to ensure that the XSS Auditor blocks a document.write() of an incomplete HTML image tag.
     10
     11        * http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror-expected.txt: Added.
     12        * http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror.html: Added.
     13        * http/tests/security/xssAuditor/dom-write-location-open-img-onerror-expected.txt: Added.
     14        * http/tests/security/xssAuditor/dom-write-location-open-img-onerror.html: Added.
     15        * http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror-expected.txt: Added.
     16        * http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror.html: Added.
     17        * http/tests/security/xssAuditor/resources/echo-nested-dom-write-location.html: Added.
     18
    1192016-10-25  Brady Eidson  <beidson@apple.com>
    220
  • trunk/Source/WebCore/ChangeLog

    r207847 r207848  
     12016-10-25  Daniel Bates  <dabates@apple.com>
     2
     3        REGRESSION (r178265): XSS Auditor fails to block document.write() of incomplete tag
     4        https://bugs.webkit.org/show_bug.cgi?id=163978
     5        <rdar://problem/25962131>
     6
     7        Reviewed by Darin Adler.
     8
     9        During the tokenization process of an HTML tag the start and end positions of each of its
     10        attributes is tracked so that the XSS Auditor can request a snippet around a suspected
     11        injected attribute. We need to take care to consider document.write() boundaries when
     12        tracking the start and end positions of each HTML tag and attribute so that the XSS Auditor
     13        receives the correct snippet. Following r178265 we no longer consider document.write()
     14        boundaries when tracking the start and end positions of attributes. So, the substring
     15        represented by the start and end positions of an attribute may correspond to some other
     16        attribute in the tag. Therefore the XSS Auditor may fail to block an injection because the
     17        snippet it requested may not be the snippet that it intended to request.
     18
     19        Tests: http/tests/security/xssAuditor/dom-write-location-dom-write-open-img-onerror.html
     20               http/tests/security/xssAuditor/dom-write-location-open-img-onerror.html
     21               http/tests/security/xssAuditor/nested-dom-write-location-open-img-onerror.html
     22
     23        * html/parser/HTMLSourceTracker.cpp:
     24        (WebCore::HTMLSourceTracker::startToken): Set the attribute base offset to be the token
     25        start position.
     26        (WebCore::HTMLSourceTracker::source): Use the specified attribute start position as-is. We no
     27        longer adjust it here because it was adjusted with respect to the attribute base offset, which
     28        takes into account document.write() boundaries.
     29        * html/parser/HTMLToken.h:
     30        (WebCore::HTMLToken::setAttributeBaseOffset): Added.
     31        (WebCore::HTMLToken::beginAttribute): Subtract attribute base offset from the specified offset.
     32        (WebCore::HTMLToken::endAttribute): Ditto.
     33        * html/parser/HTMLTokenizer.h:
     34        (WebCore::HTMLTokenizer::setTokenAttributeBaseOffset): Added.
     35
    1362016-10-25  Chris Dumez  <cdumez@apple.com>
    237
  • trunk/Source/WebCore/html/parser/HTMLSourceTracker.cpp

    r178265 r207848  
    5050    m_currentSource = currentInput;
    5151    m_tokenStart = m_currentSource.numberOfCharactersConsumed() - m_previousSource.length();
     52    tokenizer.setTokenAttributeBaseOffset(m_tokenStart);
    5253}
    5354
     
    9394String HTMLSourceTracker::source(const HTMLToken& token, unsigned attributeStart, unsigned attributeEnd)
    9495{
    95     return source(token).substring(attributeStart - m_tokenStart, attributeEnd - attributeStart);
     96    return source(token).substring(attributeStart, attributeEnd - attributeStart);
    9697}
    9798
  • trunk/Source/WebCore/html/parser/HTMLToken.h

    r199735 r207848  
    115115    void setSelfClosing();
    116116
     117    // Used by HTMLTokenizer on behalf of HTMLSourceTracker.
     118    void setAttributeBaseOffset(unsigned attributeBaseOffset) { m_attributeBaseOffset = attributeBaseOffset; }
     119
    117120public:
    118121    // Used by the XSSAuditor to nuke XSS-laden attributes.
     
    154157    // For DOCTYPE
    155158    std::unique_ptr<DoctypeData> m_doctypeData;
     159
     160    unsigned m_attributeBaseOffset { 0 }; // Changes across document.write() boundaries.
    156161};
    157162
     
    316321    m_currentAttribute = &m_attributes.last();
    317322
    318     m_currentAttribute->startOffset = offset;
     323    m_currentAttribute->startOffset = offset - m_attributeBaseOffset;
    319324}
    320325
     
    323328    ASSERT(offset);
    324329    ASSERT(m_currentAttribute);
    325     m_currentAttribute->endOffset = offset;
     330    m_currentAttribute->endOffset = offset - m_attributeBaseOffset;
    326331#if !ASSERT_DISABLED
    327332    m_currentAttribute = nullptr;
  • trunk/Source/WebCore/html/parser/HTMLTokenizer.h

    r178265 r207848  
    4343    class TokenPtr;
    4444    TokenPtr nextToken(SegmentedString&);
     45
     46    // Used by HTMLSourceTracker.
     47    void setTokenAttributeBaseOffset(unsigned);
    4548
    4649    // Returns a copy of any characters buffered internally by the tokenizer.
     
    283286}
    284287
     288inline void HTMLTokenizer::setTokenAttributeBaseOffset(unsigned offset)
     289{
     290    m_token.setAttributeBaseOffset(offset);
     291}
     292
    285293inline size_t HTMLTokenizer::numberOfBufferedCharacters() const
    286294{
Note: See TracChangeset for help on using the changeset viewer.