Created attachment 8797881 [details] id:000698,src:000108+000246,op:splice,rep:2,+cov User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.143 Safari/537.36 Steps to reproduce: The issue reported here was found by creating a corpus of malformed input with AFL and then running the input through a libvorbis example app with UBSAN sanitization enabled. Here are steps to reproduce the test harness: 1) Download libvorbis-1.3.5.tar.gz from http://downloads.xiph.org/releases/vorbis/libvorbis-1.3.5.tar.gz 2) Extract and build the code using clang with UBSAN support (i.e. CFLAGS="-fsanitize=undefined -fno-omit-frame-pointer -g") 3) Make the examples (i.e. cd examples && make) 4) Run the attached AFL produced test case through the decoder sample (i.e. cat $FILE | decoder_example) To be clear, I have not verified this problem with a proper Firefox build, but I believe the affected code is similar enough that FF's custom version of libVorbis could be affected as well. The vulnerable line appears in the gecko-dev git here: https://github.com/mozilla/gecko-dev/blob/233f30f2377f3df0f3388721901681f432b813fb/media/libtremor/lib/tremor_block.c#L442 Actual results: UBSAN produces some runtime errors including the following signed integer overflow: block.c:902:33: runtime error: signed integer overflow: 32896 - -9223372036854743040 cannot be represented in type 'long' (NOTE: block.c:902 in my source lines up with tremor_block.c:442 as shown in the gecko-dev git here: https://github.com/mozilla/gecko-dev/blob/233f30f2377f3df0f3388721901681f432b813fb/media/libtremor/lib/tremor_block.c#L442) Note that this happens after several errors about left shift operations which cannot be represented as int values. Expected results: Values should not overflow their data type permissible ranges.
:jya, can you take a look?
Isn't that the same as bug 1306476?
No, it is not the same bug. 1306476 is an integer overflow in sharedbook.c -- this one is in block.c
This is an overflow in comparing the difference between two timestamps. If the concern is that this is technically undefined behavior, okay, but I don't think this is a security bug. I doubt any compiler would compile this in a way to produce anything other than the obvious behavior. The result is carefully checked against crazy values. It *is* possible to sanitize timestamp handling to avoid any undefined behavior (see op_granpos_diff() at <https://git.xiph.org/?p=opusfile.git;a=blob;f=src/opusfile.c;h=7a697be32556#l665> for an example). Do people think that is worth doing here?
At least it would prevent future reports like this. I too do think that neither this bug nor 1306476 are security bugs
(In reply to Jean-Yves Avenard [:jya] from comment #5) > At least it would prevent future reports like this. Okay. If you want to have a go at adapting the code linked above, I'm happy to review.
Signed integer overflow is well defined behaviour so I call this a false positive.