963828 - OpenH264: "runtime error: left shift of negative value" to sReadBitsCache.uiCache32Bit, pBs->uiCurBits

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: testcase.264

In order to reproduce you need to compile OpenH264 with UBSan support.

export CC="$LLVM_HOME/build/bin/clang -fsanitize=undefined -fno-sanitize=vptr"
export CXX="$LLVM_HOME/build/bin/clang++ -fsanitize=undefined -fno-sanitize=vptr"
export LD="$LLVM_HOME/build/bin/clang++"
export LDFLAGS="-fsanitize=undefined"
make


codec/decoder/./core/src/parse_mb_syn_cavlc.cpp:698:97: runtime error: left shift of negative value -1543492599

sReadBitsCache.uiCache32Bit = ((((pBuf[0] << 8) | pBuf[1]) << 16) | (pBuf[2] << 8) | pBuf[3]) << (iCurIdx & 0x07);


codec/decoder/./core/src/parse_mb_syn_cavlc.cpp:487:116: runtime error: left shift of negative value -239537263

pBs->uiCurBits = ((((pBs->pCurBuf[0] << 8) | pBs->pCurBuf[1]) << 16) | (pBs->pCurBuf[2] << 8) | pBs->pCurBuf[3]) << (pBs->iIndex & 0x07);


I will hide this bug first because I am not sure how you would like me to treat this class of bugs.


Tested with https://github.com/cisco/openh264/commit/58c33b8ee8

Comment 1

[wayne]

Hi Christoph, semms it is not a bug, but a design to read 4 bytes. I'll need some time to double check this.

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Casting to uint32_t should resolve this (and it might currently be relying on undefined compiler behavior)

Comment 3

[wayne]

so the following may be more safe:

sReadBitsCache.uiCache32Bit = (uint32_t)(((((pBuf[0] << 8) | pBuf[1]) << 16) | (pBuf[2] << 8) | pBuf[3])) << (iCurIdx & 0x07);

Comment 4

[wayne]

Hi Christoph, the bug has been fixed in cisco master branch.
Could u please check that? Thanks.

Comment 5

[Christoph Diehl [:posidron]]

Fixed.

Tested with https://github.com/cisco/openh264/commit/d468404822