Closed Bug 959139 Opened 8 years ago Closed 8 years ago

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

Categories

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

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
firefox27 --- unaffected
firefox28 --- unaffected
firefox29 --- unaffected
firefox-esr24 --- unaffected
b2g18 --- unaffected
b2g-v1.2 --- unaffected
b2g-v1.3 --- unaffected
b2g-v1.4 --- unaffected

People

(Reporter: posidron, Unassigned)

References

(Blocks 1 open bug)

Details

(4 keywords)

Attachments

(3 files)

Attached file 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
Attached file welsenc_fuzz.cfg
Attached file layer1_fuzz.cfg
The crash bugs inside the encoder are blocking the generation of H264 samples for fuzzing the decoder more effectively.
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.
(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.
Component: Video/Audio → WebRTC: Audio/Video
I can't reporduce this crash issue. Could you give more details on this crash? thanks a lot.
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
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
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
Yes, this bug seems indeed fixed.

Tested with https://github.com/cisco/openh264/commit/b1fc94e314
ok, then can we close this bug?
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Group: core-security
You need to log in before you can comment on or make changes to this bug.