Diff
Modified: trunk/LayoutTests/ChangeLog (89117 => 89118)
--- trunk/LayoutTests/ChangeLog 2011-06-17 05:49:11 UTC (rev 89117)
+++ trunk/LayoutTests/ChangeLog 2011-06-17 05:54:14 UTC (rev 89118)
@@ -1,3 +1,17 @@
+2011-06-16 Jeffrey Pfau <[email protected]>
+
+ 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 <[email protected]>
Unreviewed, mark more Chromium test failures for tests added in r89091.
Added: trunk/LayoutTests/fast/parser/xhtml-innerhtml-null-byte-expected.txt (0 => 89118)
--- trunk/LayoutTests/fast/parser/xhtml-innerhtml-null-byte-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/parser/xhtml-innerhtml-null-byte-expected.txt 2011-06-17 05:54:14 UTC (rev 89118)
@@ -0,0 +1 @@
+PASS: An exception was raised, no crashing.
Added: trunk/LayoutTests/fast/parser/xhtml-innerhtml-null-byte-first-expected.txt (0 => 89118)
--- trunk/LayoutTests/fast/parser/xhtml-innerhtml-null-byte-first-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/parser/xhtml-innerhtml-null-byte-first-expected.txt 2011-06-17 05:54:14 UTC (rev 89118)
@@ -0,0 +1 @@
+PASS: An exception was raised, no crashing.
Added: trunk/LayoutTests/fast/parser/xhtml-innerhtml-null-byte-first.xhtml (0 => 89118)
--- trunk/LayoutTests/fast/parser/xhtml-innerhtml-null-byte-first.xhtml (rev 0)
+++ trunk/LayoutTests/fast/parser/xhtml-innerhtml-null-byte-first.xhtml 2011-06-17 05:54:14 UTC (rev 89118)
@@ -0,0 +1,22 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/Strict.dtd">
+<html xml:lang="en" lang="en" xmlns="http://www.w3.org/1999/xhtml">
+ <head>
+ <title>XHTML innerHTML null byte test</title>
+ <script><![CDATA[
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+ ]]></script>
+ </head>
+ <body>
+ <p id="p0">FAIL: No exception raised.</p>
+ <script><![CDATA[
+ try {
+ document.getElementById("p0").innerHTML = "\x00FAIL: Nulls mishandled.";
+ } catch(e) {
+ document.getElementById("p0").innerHTML = "PASS: An exception was raised, no crashing.";
+ if (e.code != 11)
+ console.log("Unexpected error thrown: " + e.name + ": " + e.message);
+ }
+ ]]></script>
+ </body>
+</html>
Added: trunk/LayoutTests/fast/parser/xhtml-innerhtml-null-byte.xhtml (0 => 89118)
--- trunk/LayoutTests/fast/parser/xhtml-innerhtml-null-byte.xhtml (rev 0)
+++ trunk/LayoutTests/fast/parser/xhtml-innerhtml-null-byte.xhtml 2011-06-17 05:54:14 UTC (rev 89118)
@@ -0,0 +1,22 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/Strict.dtd">
+<html xml:lang="en" lang="en" xmlns="http://www.w3.org/1999/xhtml">
+ <head>
+ <title>XHTML innerHTML null byte test</title>
+ <script><![CDATA[
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+ ]]></script>
+ </head>
+ <body>
+ <p id="p0">FAIL: No exception raised.</p>
+ <script><![CDATA[
+ try {
+ document.getElementById("p0").innerHTML = "FAIL: Nulls mishandled.\x00";
+ } catch(e) {
+ document.getElementById("p0").innerHTML = "PASS: An exception was raised, no crashing.";
+ if (e.code != 11)
+ console.log("Unexpected error thrown: " + e.name + ": " + e.message);
+ }
+ ]]></script>
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (89117 => 89118)
--- trunk/Source/WebCore/ChangeLog 2011-06-17 05:49:11 UTC (rev 89117)
+++ trunk/Source/WebCore/ChangeLog 2011-06-17 05:54:14 UTC (rev 89118)
@@ -1,3 +1,21 @@
+2011-06-16 Jeffrey Pfau <[email protected]>
+
+ 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 <[email protected]>
Reviewed by Kevin Ollivier.
Modified: trunk/Source/WebCore/dom/XMLDocumentParser.h (89117 => 89118)
--- trunk/Source/WebCore/dom/XMLDocumentParser.h 2011-06-17 05:49:11 UTC (rev 89117)
+++ trunk/Source/WebCore/dom/XMLDocumentParser.h 2011-06-17 05:54:14 UTC (rev 89118)
@@ -32,6 +32,7 @@
#include "SegmentedString.h"
#include <wtf/HashMap.h>
#include <wtf/OwnPtr.h>
+#include <wtf/text/CString.h>
#include <wtf/text/StringHash.h>
#if USE(QXMLSTREAM)
@@ -56,8 +57,8 @@
#if !USE(QXMLSTREAM)
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; }
@@ -160,7 +161,7 @@
void endDocument();
#endif
private:
- void initializeParserContext(const char* chunk = 0);
+ void initializeParserContext(const CString& chunk = CString());
void pushCurrentNode(ContainerNode*);
void popCurrentNode();
Modified: trunk/Source/WebCore/dom/XMLDocumentParserLibxml2.cpp (89117 => 89118)
--- trunk/Source/WebCore/dom/XMLDocumentParserLibxml2.cpp 2011-06-17 05:49:11 UTC (rev 89117)
+++ trunk/Source/WebCore/dom/XMLDocumentParserLibxml2.cpp 2011-06-17 05:54:14 UTC (rev 89118)
@@ -501,7 +501,7 @@
// 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) {
xmlInitParser();
@@ -511,7 +511,8 @@
didInit = true;
}
- 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)
return 0;
@@ -1276,7 +1277,7 @@
// http://bugs.webkit.org/show_bug.cgi?id=5792
}
-void XMLDocumentParser::initializeParserContext(const char* chunk)
+void XMLDocumentParser::initializeParserContext(const CString& chunk)
{
xmlSAXHandler sax;
memset(&sax, 0, sizeof(sax));
@@ -1308,7 +1309,7 @@
if (m_parsingFragment)
m_context = XMLParserContext::createMemoryParser(&sax, this, chunk);
else {
- ASSERT(!chunk);
+ ASSERT(!chunk.data());
m_context = XMLParserContext::createStringParser(&sax, this);
}
}
@@ -1443,7 +1444,12 @@
ASSERT(m_parsingFragment);
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.
@@ -1452,9 +1458,9 @@
// Check if all the chunk has been processed.
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;
}
