Security: Brotli heap corruption issues for large custom dictionary sizes.
Reported by
guidovra...@gmail.com,
Apr 25 2016
|
||||||||||
Issue description
import brotli
with open('plrabn12.txt.compressed', 'rb') as fp:
data = fp.read()
custom_dictionary = 'x' * (0x80000010)
d = brotli.decompress(data, dictionary=custom_dictionary)
This will cause a segmentation fault.
On 64 bit this happens because it attempts a memcpy of 18446744071562067968 bytes, so unlikely to be exploited.
This happens because s->custom_dict_size, which contains a negative value at this point, is cast to size_t:
memcpy(&s->ringbuffer[(-s->custom_dict_size) & s->ringbuffer_mask],
s->custom_dict, (size_t)s->custom_dict_size);
See the attached bro.cc file. It is a slightly customized version of tools/bro.cc in which you can configure the dictionary size and that doesn't output decompressed data.
When compiled with -m32 -fsanitize=address and using a dictionary size of 0x80000100, then this happens:
$ ./bro -d -i lcet10.txt.compressed
ASAN:SIGSEGV
=================================================================
==16947== ERROR: AddressSanitizer: SEGV on unknown address 0xf5e00000 (pc 0xf617da9c sp 0xffb9ca54 bp 0xffb9cea8 T0)
AddressSanitizer can not provide additional info.
#0 0xf617da9b (/usr/lib32/libasan.so.0.0.0+0x1ba9b)
#1 0xf616fc0b (/usr/lib32/libasan.so.0.0.0+0xdc0b)
#2 0x80ec5c2 (/media/ssdehv/br-report/2/brotli-master/tools/bro+0x80ec5c2)
#3 0x80fc953 (/media/ssdehv/br-report/2/brotli-master/tools/bro+0x80fc953)
#4 0x80502dd (/media/ssdehv/br-report/2/brotli-master/tools/bro+0x80502dd)
#5 0xf5e81a82 (/lib/i386-linux-gnu/libc-2.19.so+0x19a82)
#6 0x8051171 (/media/ssdehv/br-report/2/brotli-master/tools/bro+0x8051171)
==16947== ABORTING
I don't think it's possible to supply such a large dictionary size through 32 bit Python, although I'm not entirely sure as different Python versions/configurations
may allow different max object sizes.
This report is based on a very recent copy of the Brotli master off Github.
Let me know if you need more information.
Guido
,
Apr 25 2016
Hello I spoke to "Josh" and "Tim" of Google Security via security-patches@google.com. They advised me to post this report here in response to me inquiry regarding the the eligibility of a Brotli vulnerability under any of Google's bug bounty programs. As you can gather from my report, the vulnerability affects the Python module, and, assuming that setting a custom dictionary for decompression is meant as an open "API" function of Brotli that any application that utilizes Brotli may use, then this problem might affect these applications too. Whether Chrome is directly vulnerable I don't know. So to answer your question, this report primarily applies to Brotli as a library.
,
Apr 26 2016
Not sure how to reproduce. kenjibaheux@ -- do you know a good way to reproduce this and assign priority/severity?
,
Apr 26 2016
Adding folks from the Compression team.
,
Apr 26 2016
Guido, thanks for the report! Let's first confirm the issue => assigning to Eustas@ for confirmation. +Tim and Jeremy for the bug bounty program side of things.
,
Apr 26 2016
,
Apr 26 2016
Custom dictionary is an experimental feature. It is not used in chromium, so there is no way to compromise browser security. Actual problem is hidden in method BrotliSetCustomDictionary in line "s->custom_dict_size = (int)size;"
,
Apr 26 2016
,
Apr 26 2016
Hey Guido - I'm the "Tim" from security@google.com :) Based on the comments above that chromium doesn't use this feature, then this report doesn't qualify for a reward under the Chrome reward program. eustas@: would the best place to file this be Guido filing an issue on GitHub or can you pass this along to the right folks on Guido's behalf?
,
Apr 26 2016
Alright, that's too bad. I think it would be a good idea to convert most if not all of the 'int' members in struct BrotliStateStruct to size_t. I've looked into this myself but I'm not too familiar with the code yet to be convinced that nothing would break (as some functionality might depend on the members being signed and 32 bit (?) ) otherwise I'd send some patches. Guido
,
Apr 27 2016
I've already prepared fix in internal repo. Soon will be published to github. We have plans to revise usage of remaining ints in decoder (most of them were eliminated last autumn).
,
May 9 2016
,
May 17 2016
Fixed by https://codereview.chromium.org/1956893002/
,
May 18 2016
,
Aug 23 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by elawrence@chromium.org
, Apr 25 2016