959139 - OpenH264: heap-buffer-overflow crash in [@WelsSVCEnc::WriteBlockResidualCavlc]

Status: RESOLVED FIXED

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

Description

[Christoph Diehl [:posidron]]

Attachment: callstack.txt

codec/encoder/core/src/set_mb_syn_cavlc.cpp:166

void  WriteBlockResidualCavlc (...) {
    [...]
    /*  levels */
    [...]
    n = iLevelPrefix + 1 + iLevelSuffixSize;
    iValue = ((1 << iLevelSuffixSize) | iLevelSuffix);
*   CAVLC_BS_WRITE (n, iValue);


This is in the encoder - set only to sec-critical because it is a direct write violation.

Tested with https://github.com/cisco/openh264/commit/14f6c4fa72

Comment 1

[Christoph Diehl [:posidron]]

Attachment: welsenc_fuzz.cfg

Comment 2

[Christoph Diehl [:posidron]]

Attachment: layer1_fuzz.cfg

Comment 3

[Christoph Diehl [:posidron]]

The crash bugs inside the encoder are blocking the generation of H264 samples for fuzzing the decoder more effectively.

Comment 4

[Eric Rescorla (:ekr)]

I'm not (In reply to Christoph Diehl [:cdiehl] from comment #3)
> The crash bugs inside the encoder are blocking the generation of H264
> samples for fuzzing the decoder more effectively.

Can you explain more what the problem is here? Do you think this is a
legitimate configuration of the encoder? If so, that's a functional
bug as well as potentially a security issue. If it's not, then why
does it impede sample generation?

In any case, there are a lot of H.264-encoded videos available and
you could also use x264 as your encoder. This wouldn't be a bad idea
in any case, since it may explore encoding regions that are hard to
access via simple fuzzing but which are incompatibilities between
the expected bitstreams.

Comment 5

[Christoph Diehl [:posidron]]

(In reply to Eric Rescorla (:ekr) from comment #4)
> Can you explain more what the problem is here? Do you think this is a
> legitimate configuration of the encoder? If so, that's a functional
> bug as well as potentially a security issue. If it's not, then why
> does it impede sample generation?

I try to be conform to the parsing conditions for the configuration as defined in codec/console/enc/src/welsenc.cpp, I assume this configuration is "valid" - generating values in allowed ranges and turning flags on/off, no extraordinary large values.

(In reply to Eric Rescorla (:ekr) from comment #4)
> In any case, there are a lot of H.264-encoded videos available and
> you could also use x264 as your encoder.

I did collect some raw bytestream samples from various places but most the ones out there are too large in size. 
Yes, my brain was now so focused on the provided OpenH264 tools that I forgot about everything else.

Comment 6

[karina]

I can't reporduce this crash issue. Could you give more details on this crash? thanks a lot.

Comment 7

[Christoph Diehl [:posidron]]

Probably because you are not using an ASan enabled build.

Try something like this:

% export CXX=<path_to_llvm>bin/clang++
% export CC=<path_to_llvm>/bin/clang
% export USE_ASAN=Yes
% make

Comment 8

[Ethan Hugg [:ehugg]]

Karina,  The Mozilla build documentation has the info on how to build llvm here:

https://developer.mozilla.org/en-US/docs/Building_Firefox_with_Address_Sanitizer#Manual_Build

Comment 9

[karina]

Hi Christoph/Ethan:
   I used the method you mentioned above, but I still can't reproduce this issue. 
   Build info on my side. I run it on MacOS

/Volumes/Work/karina/openh264/llmv1/build/bin/clang++ -O3 -fsanitize=address -m64 -Wno-deprecated-declarations -Werror -fPIC  -DX86_ASM -DNO_DYNAMIC_VP  -Icodec/api/svc -Icodec/common -Igtest/include  -Icodec/decoder/core/inc -Icodec/decoder/plus/inc -Icodec/console/dec/inc -c -o codec/console/dec/./src/d3d9_utils.o codec/console/dec/./src/d3d9_utils.cpp

Comment 10

[Christoph Diehl [:posidron]]

Yes, this bug seems indeed fixed.

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

Comment 11

[karina]

ok, then can we close this bug?