Changeset 89118 in webkit

Timestamp:

Jun 16, 2011 10:54:14 PM (13 years ago)

Author:

commit-queue@webkit.org

Message:

2011-06-16 Jeffrey Pfau <​jpfau@apple.com>

Reviewed by Alexey Proskuryakov.

Using null bytes when setting innerHTML in XTHML results in assertion and a crash due to null-pointer dereference
​https://bugs.webkit.org/show_bug.cgi?id=61053

Added test cases covering two cases of using innerHTML with null bytes in XHTML.

• fast/parser/xhtml-innerhtml-null-byte-expected.txt: Added.
• fast/parser/xhtml-innerhtml-null-byte-first-expected.txt: Added.
• fast/parser/xhtml-innerhtml-null-byte-first.xhtml: Added.
• fast/parser/xhtml-innerhtml-null-byte.xhtml: Added.

2011-06-16 Jeffrey Pfau <​jpfau@apple.com>

Reviewed by Alexey Proskuryakov.

Using null bytes when setting innerHTML in XTHML results in assertion and a crash due to null-pointer dereference
​https://bugs.webkit.org/show_bug.cgi?id=61053

XML parsing in-memory XML chunks now passes around a string object instead of a C string, ensuring null characters are properly handled.

Tests: fast/parser/xhtml-innerhtml-null-byte-first.xhtml

fast/parser/xhtml-innerhtml-null-byte.xhtml

• dom/XMLDocumentParser.h:
• dom/XMLDocumentParserLibxml2.cpp: (WebCore::XMLParserContext::createMemoryParser): (WebCore::XMLDocumentParser::initializeParserContext): (WebCore::XMLDocumentParser::appendFragmentSource):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/parser/xhtml-innerhtml-null-byte-expected.txt (added)
• LayoutTests/fast/parser/xhtml-innerhtml-null-byte-first-expected.txt (added)
• LayoutTests/fast/parser/xhtml-innerhtml-null-byte-first.xhtml (added)
• LayoutTests/fast/parser/xhtml-innerhtml-null-byte.xhtml (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/XMLDocumentParser.h (modified) (3 diffs)
• Source/WebCore/dom/XMLDocumentParserLibxml2.cpp (modified) (6 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-06-16  Jeffrey Pfau  <jpfau@apple.com>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Using null bytes when setting innerHTML in XTHML results in assertion and a crash due to null-pointer dereference
+        https://bugs.webkit.org/show_bug.cgi?id=61053
+
+        Added test cases covering two cases of using innerHTML with null bytes in XHTML.
+
+        * fast/parser/xhtml-innerhtml-null-byte-expected.txt: Added.
+        * fast/parser/xhtml-innerhtml-null-byte-first-expected.txt: Added.
+        * fast/parser/xhtml-innerhtml-null-byte-first.xhtml: Added.
+        * fast/parser/xhtml-innerhtml-null-byte.xhtml: Added.
+
 2011-06-16  Yuta Kitamura  <yutak@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-06-16  Jeffrey Pfau  <jpfau@apple.com>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Using null bytes when setting innerHTML in XTHML results in assertion and a crash due to null-pointer dereference
+        https://bugs.webkit.org/show_bug.cgi?id=61053
+
+        XML parsing in-memory XML chunks now passes around a string object instead of a C string, ensuring null characters are properly handled.
+
+        Tests: fast/parser/xhtml-innerhtml-null-byte-first.xhtml
+               fast/parser/xhtml-innerhtml-null-byte.xhtml
+
+        * dom/XMLDocumentParser.h:
+        * dom/XMLDocumentParserLibxml2.cpp:
+        (WebCore::XMLParserContext::createMemoryParser):
+        (WebCore::XMLDocumentParser::initializeParserContext):
+        (WebCore::XMLDocumentParser::appendFragmentSource):
+
 2011-06-16  Robin Dunn  <robin@alldunn.com>
 

trunk/Source/WebCore/dom/XMLDocumentParser.h

@@ -33 +33 @@
 #include <wtf/HashMap.h>
 #include <wtf/OwnPtr.h>
+#include <wtf/text/CString.h>
 #include <wtf/text/StringHash.h>
 
@@ -57 +58 @@
     class XMLParserContext : public RefCounted<XMLParserContext> {
     public:
-        static PassRefPtr<XMLParserContext> createMemoryParser(xmlSAXHandlerPtr, void*, const char*);
-        static PassRefPtr<XMLParserContext> createStringParser(xmlSAXHandlerPtr, void*);
+        static PassRefPtr<XMLParserContext> createMemoryParser(xmlSAXHandlerPtr, void* userData, const CString& chunk);
+        static PassRefPtr<XMLParserContext> createStringParser(xmlSAXHandlerPtr, void* userData);
         ~XMLParserContext();
         xmlParserCtxtPtr context() const { return m_context; }
@@ -161 +162 @@
 #endif
     private:
-        void initializeParserContext(const char* chunk = 0);
+        void initializeParserContext(const CString& chunk = CString());
 
         void pushCurrentNode(ContainerNode*);

trunk/Source/WebCore/dom/XMLDocumentParserLibxml2.cpp

@@ -502 +502 @@
 
 // Chunk should be encoded in UTF-8
-PassRefPtr<XMLParserContext> XMLParserContext::createMemoryParser(xmlSAXHandlerPtr handlers, void* userData, const char* chunk)
+PassRefPtr<XMLParserContext> XMLParserContext::createMemoryParser(xmlSAXHandlerPtr handlers, void* userData, const CString& chunk)
 {
     if (!didInit) {
@@ -512 +512 @@
     }
 
-    xmlParserCtxtPtr parser = xmlCreateMemoryParserCtxt(chunk, xmlStrlen((const xmlChar*)chunk));
+    // appendFragmentSource() checks that the length doesn't overflow an int.
+    xmlParserCtxtPtr parser = xmlCreateMemoryParserCtxt(chunk.data(), chunk.length());
 
     if (!parser)
@@ -1277 +1278 @@
 }
 
-void XMLDocumentParser::initializeParserContext(const char* chunk)
+void XMLDocumentParser::initializeParserContext(const CString& chunk)
 {
     xmlSAXHandler sax;
@@ -1309 +1310 @@
         m_context = XMLParserContext::createMemoryParser(&sax, this, chunk);
     else {
-        ASSERT(!chunk);
+        ASSERT(!chunk.data());
         m_context = XMLParserContext::createStringParser(&sax, this);
     }
@@ -1444 +1445 @@
 
     CString chunkAsUtf8 = chunk.utf8();
-    initializeParserContext(chunkAsUtf8.data());
+    
+    // libxml2 takes an int for a length, and therefore can't handle XML chunks larger than 2 GiB.
+    if (chunkAsUtf8.length() > INT_MAX)
+        return false;
+
+    initializeParserContext(chunkAsUtf8);
     xmlParseContent(context());
     endDocument(); // Close any open text nodes.
@@ -1453 +1459 @@
     long bytesProcessed = xmlByteConsumed(context());
     if (bytesProcessed == -1 || ((unsigned long)bytesProcessed) != chunkAsUtf8.length()) {
-        // FIXME: I don't believe we can hit this case without also having seen an error.
+        // FIXME: I don't believe we can hit this case without also having seen an error or a null byte.
         // If we hit this ASSERT, we've found a test case which demonstrates the need for this code.
-        ASSERT(m_sawError);
+        ASSERT(m_sawError || (bytesProcessed >= 0 && !chunkAsUtf8.data()[bytesProcessed]));
         return false;
     }