Re: [xml] Heap use after free in parser.c

[Jay Civelli <jcivelli google com>]

On Mon, Jan 22, 2018 at 7:53 AM, Nick Wellnhofer <wellnhofer aevum de> wrote:

On 08/01/2018 22:43, Jay Civelli wrote:

On Mon, Jan 8, 2018 at 11:27 AM, Nick Wellnhofer <wellnhofer aevum de <mailto:wellnhofer aevum de>> wrote:

    On 02/01/2018 20:08, Jay Civelli via xml wrote:

        We ran into a heap use after free in Chromium http://crbug.com/793715
        <http://crbug.com/793715> that I think I tracked down.

    I don't have access to this page.

You should have access now.

I still don't have access to the original Clusterfuzz report. I only found your reduced test case "bad_xml" but I couldn't reproduce the issue with xmllint. Given the stack trace and Chromium sources, it seems that you're using xmlReaderForMemory in recovery mode:


https://chromium.googlesource.com/chromium/src/+/master/third_party/libxml/chromium/libxml_utils.cc

Note that it's discouraged to use XML_PARSE_RECOVER in production code. This flag hides errors in invalid XML and exercises some less-tested code paths in libxml2.

Thanks, I'll look into removing it. 

For future reports, it would be helpful to provide test cases that show the problem with xmllint. The following flags should make xmllint behave similar to the Chromium code in question:

    xmllint --stream --memory --recover file.xml

Got it, will do in the future. 

Good idea, done in new attached patch. Note that I changed the error from the existing from XML_ERR_INVALID_ENCODING to XML_ERR_INVALID_CHAR which seemed to make more sense.

I committed a minimal fix that only adds a call to xmlHaltParser.

Great, thanks. I tested locally, it does address the issue. 

https://git.gnome.org/browse/libxml2/commit/?id=ab362ab0ad3af54406ae8237a525405c6e2a705b

Nick