New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 710088 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

memory corruption in libxml2 uri.c

Reported by vvva...@gmail.com, Apr 10 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.98 Safari/537.36

Steps to reproduce the problem:
uri_h.c
```
#include <iostream>
#include <libxml/uri.h>
#include <fstream>

using namespace std;

int main(int argc, char* argv[]) {

    char *buffer = NULL;
    int string_size, read_size;
    FILE *handler = fopen(argv[1], "r");
    fseek(handler, 0, SEEK_END);
    string_size = ftell(handler);
    rewind(handler);
    buffer = (char*) malloc(sizeof(char) * (string_size + 1) );
    read_size = fread(buffer, sizeof(char), string_size, handler);
    buffer[string_size] = '\0';
    if (string_size != read_size)
    {
        free(buffer);
        buffer = NULL;
    }
    fclose(handler);
    xmlChar *aa = new xmlChar;
    *aa = *buffer;

    xmlFreeURI(xmlParseURI(buffer));
    cout<<"buffer: "<<buffer<<endl;
    xmlURIUnescapeString(buffer,read_size,(char *)aa);
    cout<<"buffer: "<<buffer<<endl;
    cout<<"aa: "<<aa<<endl;
    xmlCreateURI();
}
```

root@ubuntu:/home/vah13/Desktop/crashes# valgrind ./uri_h/uri_h ./uri_h/out/crashes/testcase 
==2475== Memcheck, a memory error detector
==2475== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==2475== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==2475== Command: ./uri_h/uri_h ./uri_h/out/crashes/testcase
==2475== 
buffer: m�aa~TTTTTTTTTTTTTTTTTTT�@aa
==2475== Invalid write of size 1
==2475==    at 0x41CF275: xmlURIUnescapeString (uri.c:1651)
==2475==    by 0x8048DE6: main (writer.cpp:88)
==2475==  Address 0x4965209 is 0 bytes after a block of size 1 alloc'd
==2475==    at 0x402A6DC: operator new(unsigned int) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==2475==    by 0x8048D8B: main (writer.cpp:32)
==2475== 

valgrind: m_mallocfree.c:304 (get_bszB_as_is): Assertion 'bszB_lo == bszB_hi' failed.
valgrind: Heap block lo/hi size mismatch: lo = 56, hi = 24929.
This is probably caused by your program erroneously writing past the
end of a heap block and corrupting heap metadata.  If you fix any
invalid writes reported by Memcheck, this assertion failure will
probably go away.  Please try that before reporting this as a bug.

host stacktrace:
==2475==    at 0x3805A504: ??? (in /usr/lib/valgrind/memcheck-x86-linux)
==2475==    by 0x3805A656: ??? (in /usr/lib/valgrind/memcheck-x86-linux)
==2475==    by 0x3805A7B9: ??? (in /usr/lib/valgrind/memcheck-x86-linux)
==2475==    by 0x38068EF2: ??? (in /usr/lib/valgrind/memcheck-x86-linux)
==2475==    by 0x380533A6: ??? (in /usr/lib/valgrind/memcheck-x86-linux)
==2475==    by 0x380502DF: ??? (in /usr/lib/valgrind/memcheck-x86-linux)
==2475==    by 0x38051CD7: ??? (in /usr/lib/valgrind/memcheck-x86-linux)
==2475==    by 0x38056101: ??? (in /usr/lib/valgrind/memcheck-x86-linux)
==2475==    by 0x38050F4D: ??? (in /usr/lib/valgrind/memcheck-x86-linux)
==2475==    by 0x380002C3: ??? (in /usr/lib/valgrind/memcheck-x86-linux)
==2475==    by 0x380333B2: ??? (in /usr/lib/valgrind/memcheck-x86-linux)
==2475==    by 0x62CB2798: ???

sched status:
  running_tid=1

Thread 1: status = VgTs_Runnable
==2475==    at 0x41CF698: xmlURIUnescapeString (uri.c:1655)
==2475==    by 0x8048DE6: main (writer.cpp:88)

What is the expected behavior?

What went wrong?
all information in reproduce step

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version: 57.0.2987.98  Channel: n/a
OS Version: 10.0
Flash Version: Shockwave Flash 25.0 r0
 
testcase
30 bytes View Download

Comment 2 by kojii@chromium.org, Apr 21 2017

Cc: kojii@chromium.org
Labels: Needs-Feedback
NextAction: 2017-05-05
As far as I searched for, all our usage set target == NULL, in which case xmlURIUnescapeString allocates memory for len+1.

From that IIUC, the program in #0 is testing the code in our third_party repo that will not run in Chromium.

Could you confirm if my understanding is correct?

Comment 3 by vvva...@gmail.com, Apr 21 2017

yes, that's right
Project Member

Comment 4 by sheriffbot@chromium.org, Apr 21 2017

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "kojii@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 5 by kojii@chromium.org, Apr 21 2017

Cc: -kojii@chromium.org dominicc@chromium.org
Owner: kojii@chromium.org
Status: WontFix (was: Unconfirmed)
Thank you for the prompt reply and report.

I'd then resolve as WontFIx. Maybe it's great if you could report to the master of libxml for more secure API (e.g., that receives target length) but I'm not familiar with how libxml upstream is working.

/cc dominicc@ in case he has any.

Comment 6 by vvva...@gmail.com, Apr 21 2017

yeap, 
thank you guys
I think filing these upstream would be great so everyone can benefit. If you like you can CC dominicc@chromium.org.

If you want to gauge impact to Chromium, try to reproduce them by loading XML content in Chrome (you can do ASAN builds of Chromium! Probably valgrind too.) Or with a libfuzzer target (see tests/libfuzzer/fuzzers/libxml_xml_read_memory_fuzzer.cc for example) and if so, then it would be great to file a Chromium bug.

That code you're pointing to is slightly out of date; tip is now this which looks for the terminating null:

https://chromium.googlesource.com/chromium/src.git/+/master/third_party/libxml/src/uri.c#1652

You can learn a ton about Chrome releasing by looking at this site:

https://omahaproxy.appspot.com/

In particular it looks like 59.0.3067.2 was never released because it doesn't show up in https://omahaproxy.appspot.com/history ; why did you pick that version? HTH, have a nice day.

Comment 8 by kojii@chromium.org, May 2 2017

NextAction: ----

Sign in to add a comment