multiple memory leak in dict.c
Reported by
vvva...@gmail.com,
Apr 10 2017
|
|||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36 Steps to reproduce the problem: https://chromium.googlesource.com/chromium/src.git/+/59.0.3067.2/third_party/libxml/src/dict.c the vulnerability present in the dict.c file in xmlDictLookup function. xmlDictLookup function has 3 parameters and xmlDictPtr dict, const xmlChar *name, int len. And in the https://chromium.googlesource.com/chromium/src.git/+/59.0.3067.2/third_party/libxml/src/dict.c#839 don't checked this len > strlen((const char *) name) condition ``` root@ubuntu:/home/vah13/Desktop/libxml-cpp-wrapper# valgrind --leak-check=full --show-leak-kinds=all ./uri_h in/textfile.txt ==6206== Memcheck, a memory error detector ==6206== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. ==6206== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info ==6206== Command: ./uri_h in/textfile.txt ==6206== ==6206== Invalid read of size 1 ==6206== at 0x402D3F3: strlen (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so) ==6206== by 0x8048D56: main (writer.cpp:72) ==6206== Address 0x49651f1 is 0 bytes after a block of size 1 alloc'd ==6206== at 0x402A6DC: operator new(unsigned int) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so) ==6206== by 0x8048D43: main (writer.cpp:30) ==6206== ==6206== Invalid read of size 1 ==6206== at 0x458D0DC: xmlDictComputeFastKey (dict.c:451) ==6206== by 0x458D0DC: xmlDictLookup (dict.c:851) ==6206== by 0x8048D73: main (writer.cpp:72) ==6206== Address 0x49861e1 is 132,273 bytes inside an unallocated block of size 4,190,904 in arena "client" ==6206== ==6206== Invalid read of size 1 ==6206== at 0x458D11C: xmlDictComputeFastKey (dict.c:455) ==6206== by 0x458D11C: xmlDictLookup (dict.c:851) ==6206== by 0x8048D73: main (writer.cpp:72) ==6206== Address 0x49651f9 is 8 bytes after a block of size 1 alloc'd ==6206== at 0x402A6DC: operator new(unsigned int) (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so) ==6206== by 0x8048D43: main (writer.cpp:30) ``` for the fix, I think, need change code like this src.git/+/59.0.3067.2/third_party/libxml/src/dict.c#839 ``` xmlDictLookup(xmlDictPtr dict, const xmlChar *name, int len) { unsigned long key, okey, nbi = 0; xmlDictEntryPtr entry; xmlDictEntryPtr insert; const xmlChar *ret; unsigned int l; if ((dict == NULL) || (name == NULL)) return(NULL); -- if (len < 0) ++ if (len < 0 || len > strlen((const char *) name)) l = strlen((const char *) name); else l = len; if (((dict->limit > 0) && (l >= dict->limit)) || (l > INT_MAX / 2)) return(NULL); /* * Check for duplicate and insertion location. */ okey = xmlDictComputeKey(dict, name, l); ``` What is the expected behavior? What went wrong? I reported information about this bug to bugzilla.gnome.org https://bugzilla.gnome.org/show_bug.cgi?id=781120 Did this work before? No Does this work in other browsers? N/A Chrome version: 57.0.2987.133 Channel: stable OS Version: 6.1 (Windows 7, Windows Server 2008 R2) Flash Version: Shockwave Flash 25.0 r0
,
Apr 10 2017
upd: os->Linux, Ubuntu 14.04
,
Apr 10 2017
,
Apr 11 2017
The caller of xmlDictLookup is clearly buggy by passing a length that is much longer than "name". While that optimization is there--pass the length if known--also using strlen doesn't make sense. It would be a nice simplification to remove the len parameter. It would be best to do this sort of work upstream in libxml2 and then Chromium will adopt the change by rolling libxml2 changes. See the libxml2 site for instructions about how to do that. |
|||
►
Sign in to add a comment |
|||
Comment 1 by vvva...@gmail.com
, Apr 10 2017825 bytes
825 bytes View Download
769 bytes
769 bytes View Download
1 bytes
1 bytes View Download