1462189 - nestegg: unsigned integer overflow in [@ ne_read_svint]

Status: NEW

(Core :: Audio/Video: Playback, defect, P3)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.webm

nestegg (test) was build unsing the following config command:
CFLAGS="-fsanitize=integer" CC=clang ./configure --enable-static --disable-shared

src/nestegg.c:711:19: runtime error: unsigned integer overflow: 3 - 63 cannot be represented in type 'unsigned long'
    #0 0x504c5a in ne_read_svint nestegg/src/nestegg.c:711:19
    #1 0x504c5a in ne_read_ebml_lacing nestegg/src/nestegg.c:1292
    #2 0x504c5a in ne_read_block nestegg/src/nestegg.c:1509
    #3 0x4fdf38 in nestegg_read_packet nestegg/src/nestegg.c:2802:15
    #4 0x4ea703 in main nestegg/test/test.c:166:9
    #5 0x7ff0728bb82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #6 0x418928 in _start (nestegg/test_ubsan+0x418928)

Comment 1

[Matthew Gregan [:kinetik]]

I think this one is fine as we're relying on defined modulo behaviour of unsigned types.  It'd be nice to mark it explicitly so it doesn't trigger the sanitizer, but other than applying "__attribute__((no_sanitize("integer")))" to the entire function (which could hide some other bug), I'm not sure how to do so.

Comment 2

[Matthew Gregan [:kinetik]]

Even if we computed bad results, we check that they're <= LIMIT_FRAME (256MB) in ne_read_block before passing them to malloc.  Based on this and my above comment, I'll clear s-s on this one.

Comment 3

[Matthew Gregan [:kinetik]]

We also sanity check the sum of frame_sizes against block_size, which is read from the media and verified to be <= 1GB.  If the frame_sizes pass these sanity checks, we read the frame into the allocated buffer which will eventually be passed as an opaque data block to the codec without further processing within nestegg or the Gecko media stack.