Changeset 60802 in webkit

Timestamp:

Jun 7, 2010, 2:53:13 PM (15 years ago)

Author:

abarth@webkit.org

Message:

2010-06-07 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

Fix XFrameOptions and xssAuditor crashes in HTML5 parser
​https://bugs.webkit.org/show_bug.cgi?id=40265

We're not supposed to call end() while the tokenizer's write() method.
This causes a bunch of LayoutTests to crash. In particular, this patch
fixes crashes in the following tests:

Tests:

• http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-in-body.html
• http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-parent-same-origin-deny.html
• http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag.html
• http/tests/security/xssAuditor/full-block-base-href.html
• http/tests/security/xssAuditor/full-block-get-from-iframe.html
• http/tests/security/xssAuditor/full-block-iframe-javascript-url.html
• http/tests/security/xssAuditor/full-block-link-onclick.html
• http/tests/security/xssAuditor/full-block-post-from-iframe.html
• http/tests/security/xssAuditor/full-block-script-tag.html
• http/tests/security/xssAuditor/xss-protection-parsing-01.html

• html/HTML5Tokenizer.cpp: (WebCore::): (WebCore::HTML5Tokenizer::HTML5Tokenizer): (WebCore::HTML5Tokenizer::write): (WebCore::HTML5Tokenizer::attemptToEnd): (WebCore::HTML5Tokenizer::endIfDelayed): (WebCore::HTML5Tokenizer::finish): (WebCore::HTML5Tokenizer::resumeParsingAfterScriptExecution):
• html/HTML5Tokenizer.h: (WebCore::HTML5Tokenizer::inWrite):

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/HTML5Tokenizer.cpp (modified) (6 diffs)
• html/HTML5Tokenizer.h (modified) (2 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-06-07  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        Fix XFrameOptions and xssAuditor crashes in HTML5 parser
+        https://bugs.webkit.org/show_bug.cgi?id=40265
+
+        We're not supposed to call end() while the tokenizer's write() method.
+        This causes a bunch of LayoutTests to crash.  In particular, this patch
+        fixes crashes in the following tests:
+
+        Tests:
+          * http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-in-body.html
+          * http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag-parent-same-origin-deny.html
+          * http/tests/security/XFrameOptions/x-frame-options-deny-meta-tag.html
+          * http/tests/security/xssAuditor/full-block-base-href.html
+          * http/tests/security/xssAuditor/full-block-get-from-iframe.html
+          * http/tests/security/xssAuditor/full-block-iframe-javascript-url.html
+          * http/tests/security/xssAuditor/full-block-link-onclick.html
+          * http/tests/security/xssAuditor/full-block-post-from-iframe.html
+          * http/tests/security/xssAuditor/full-block-script-tag.html
+          * http/tests/security/xssAuditor/xss-protection-parsing-01.html
+
+        * html/HTML5Tokenizer.cpp:
+        (WebCore::):
+        (WebCore::HTML5Tokenizer::HTML5Tokenizer):
+        (WebCore::HTML5Tokenizer::write):
+        (WebCore::HTML5Tokenizer::attemptToEnd):
+        (WebCore::HTML5Tokenizer::endIfDelayed):
+        (WebCore::HTML5Tokenizer::finish):
+        (WebCore::HTML5Tokenizer::resumeParsingAfterScriptExecution):
+        * html/HTML5Tokenizer.h:
+        (WebCore::HTML5Tokenizer::inWrite):
+
 2010-06-07  Eric Seidel  <eric@webkit.org>
 

trunk/WebCore/html/HTML5Tokenizer.cpp

@@ -38 +38 @@
 namespace WebCore {
 
+namespace {
+
+class NestingLevelIncrementer : public Noncopyable {
+public:
+    NestingLevelIncrementer(int& counter)
+        : m_counter(&counter)
+    {
+        ++(*m_counter);
+    }
+    
+    ~NestingLevelIncrementer()
+    {
+        --(*m_counter);
+    }
+
+private:
+    int* m_counter;
+};
+
+} // namespace
+
 HTML5Tokenizer::HTML5Tokenizer(HTMLDocument* document, bool reportErrors)
     : Tokenizer()
@@ -44 +65 @@
     , m_scriptRunner(new HTML5ScriptRunner(document, this))
     , m_treeBuilder(new HTML5TreeBuilder(m_lexer.get(), document, reportErrors))
-    , m_wasWaitingOnScriptsDuringFinish(false)
+    , m_endWasDelayed(false)
+    , m_writeNestingLevel(0)
 {
     begin();
@@ -51 +73 @@
 HTML5Tokenizer::~HTML5Tokenizer()
 {
+    ASSERT(!m_endWasDelayed);
 }
 
@@ -81 +104 @@
 void HTML5Tokenizer::write(const SegmentedString& source, bool)
 {
+    NestingLevelIncrementer nestingLevelIncrementer(m_writeNestingLevel);
+
     // HTML5Tokenizer::executeScript is responsible for handling saving m_source before re-entry.
     m_source.append(source);
     if (!m_treeBuilder->isPaused())
         pumpLexer();
+
+    endIfDelayed();
 }
 
@@ -95 +122 @@
 }
 
-void HTML5Tokenizer::finish()
+void HTML5Tokenizer::attemptToEnd()
 {
     // finish() indicates we will not receive any more data. If we are waiting on
     // an external script to load, we can't finish parsing quite yet.
-    if (isWaitingForScripts()) {
-        // FIXME: We might want to use real state enum instead of a bool here.
-        m_wasWaitingOnScriptsDuringFinish = true;
-        return;
-    }
+
+    if (inWrite() || isWaitingForScripts()) {
+        m_endWasDelayed = true;
+        return;
+    }
+
     // We can't call m_source.close() yet as we may have a <script> execution
     // pending which will call document.write().  No more data off the network though.
     end();
+}
+
+void HTML5Tokenizer::endIfDelayed()
+{
+    if (!m_endWasDelayed || !m_source.isEmpty() || isWaitingForScripts() || executingScript())
+        return;
+
+    m_endWasDelayed = false;
+    end();
+}
+
+void HTML5Tokenizer::finish()
+{
+    attemptToEnd();
 }
 
@@ -135 +177 @@
     pumpLexer();
     ASSERT(m_treeBuilder->isPaused() || m_source.isEmpty());
-    if (m_source.isEmpty() && m_wasWaitingOnScriptsDuringFinish)
-        end(); // The document already finished parsing we were just waiting on scripts when finished() was called.
+    // The document already finished parsing we were just waiting on scripts when finished() was called.
+    endIfDelayed();
 }
 

trunk/WebCore/html/HTML5Tokenizer.h

@@ -73 +73 @@
     void resumeParsingAfterScriptExecution();
 
+    void attemptToEnd();
+    void endIfDelayed();
+    bool inWrite() const { return m_writeNestingLevel > 0; }
+
     SegmentedString m_source;
 
@@ -82 +86 @@
     OwnPtr<HTML5ScriptRunner> m_scriptRunner;
     OwnPtr<HTML5TreeBuilder> m_treeBuilder;
-    bool m_wasWaitingOnScriptsDuringFinish;
+    bool m_endWasDelayed;
+    int m_writeNestingLevel;
 };