[Webkit-unassigned] [Bug 61053] New: Using NULL bytes when setting innerHTML in xhtml ASSERT and NULL ptr

Wed May 18 07:49:29 PDT 2011

https://bugs.webkit.org/show_bug.cgi?id=61053

           Summary: Using NULL bytes when setting innerHTML in xhtml
                    ASSERT and NULL ptr
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: PC
        OS/Version: Windows Vista
            Status: NEW
          Severity: Normal
          Priority: P1
         Component: HTML DOM
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: skylined at chromium.org
                CC: eric at webkit.org

Created an attachment (id=93913)
 --> (https://bugs.webkit.org/attachment.cgi?id=93913&action=review)
Repro ASSERT

Two very similar repros trigger an ASSERT and a NULL ptr:

Repro ASSERT:
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
  <body onload="document.body.innerHTML = 'x\x00\x3C';"></body>
</html>
(Ascii codes hint: that is "x\0<")

Source:
webkit\source\webcore\dom\xmldocumentparserlibxml2.cpp @ 1440:
bool XMLDocumentParser::appendFragmentSource(const String& chunk)
{
    ASSERT(!m_context);
    ASSERT(m_parsingFragment);

    CString chunkAsUtf8 = chunk.utf8();
    initializeParserContext(chunkAsUtf8.data());
    xmlParseContent(context());
    endDocument(); // Close any open text nodes.

    // FIXME: If this code is actually needed, it should probably move to finish()
    // XMLDocumentParserQt has a similar check (m_stream.error() == QXmlStreamReader::PrematureEndOfDocumentError) in doEnd().
    // 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.
        // If we hit this ASSERT, we've found a test case which demonstrates the need for this code.
        ASSERT(m_sawError);
        return false;
    }

    // No error if the chunk is well formed or it is not but we have no error.
    return context()->wellFormed || !xmlCtxtGetLastError(context());
}

@eric: You wrote that comment and added the ASSERT. I've found your test case; it seems NULL bytes terminate processing of the string, so you do not end up processing as many bytes as there are in the chunkAsUtf8 string :)

----

Repro NULL ptr:
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
  <body onload="document.body.innerHTML = '\x00\x3Cx';"></body>
</html>
(Ascii codes hint: that is "\0<x")

Source:
Let's see what happens in appendFragmentSource if chunk == ' <x':
    initializeParserContext(chunkAsUtf8.data());
    xmlParseContent(context());
==> "initializeParserContext" sets m_context, "xmlParseContent(context())" parses it. There is no check for a NULL m_context; this causes the NULL pointer.

webkit\source\webcore\dom\xmldocumentparserlibxml2.cpp @ 1279:
void XMLDocumentParser::initializeParserContext(const char* chunk)
{
<<<snip>>>
    if (m_parsingFragment)
        m_context = XMLParserContext::createMemoryParser(&sax, this, chunk);
<<<snip>>>
}

webkit\source\webcore\dom\xmldocumentparserlibxml2.cpp @ 504:
PassRefPtr<XMLParserContext> XMLParserContext::createMemoryParser(xmlSAXHandlerPtr handlers, void* userData, const char* chunk)
{
<<<snip>>>
    xmlParserCtxtPtr parser = xmlCreateMemoryParserCtxt(chunk, xmlStrlen((const xmlChar*)chunk));

    if (!parser)
        return 0;
<snip>

==> "xmlStrlen" (libxml\src\xmlstring.c @ 421) looks for the first '\0' and returns 0.
==> "xmlCreateMemoryParserCtxt" will return NULL if size == 0.
==> "createMemoryParser" will return NULL so "m_context" will be 0.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.