New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 763023 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

CHECK failure: video_decoder_config.IsValidConfig() in mp2t_stream_parser.cc

Project Member Reported by ClusterFuzz, Sep 7 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6021584140894208

Fuzzer: libFuzzer_mediasource_MP2T_AACSBR_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  video_decoder_config.IsValidConfig() in mp2t_stream_parser.cc
  base::debug::DebugBreak
  media::mp2t::Mp2tStreamParser::OnVideoConfigChanged
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=499783:499873

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6021584140894208

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Cc: msrchandra@chromium.org wolenetz@chromium.org
Labels: Test-Predator-Wrong-CLs M-63
Owner: dalecur...@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.
Using Code Search for the file, "mp2t_stream_parser.cc" assigning to the concern owner.

@dalecurtis -- Could you please look into it issue, kindly re-assign if this not related your changes.
Thank You.
Cc: dalecur...@chromium.org
Components: Internals>Media>Source
Owner: servolk@chromium.org
=>servolk@

Sergey, our new MSE fuzzers are triggering this DCHECK that damienv@ touched last in https://chromium.googlesource.com/chromium/src/+/990e3f9e6b878a9a0f1c84626090180390972fd0

I'm uncertain what would happen in release builds (of this Chromecast-specific code path) if such an invalid config were undetected, hence I'm leaving it P1. Perhaps parse error is best in this case.

Can you please make a fix; assign it back to either Dale or me if you don't have bandwidth for this. Thanks!
It's also interesting that there's a video config involved at all in an audio-only mediasource (just AACSBR is expected per mime-type in addSourceBuffer()). Please investigate that, too.
Project Member

Comment 4 by ClusterFuzz, Oct 1 2017

Components: Internals>Core Internals>Media
Labels: Test-Predator-AutoComponents
Automatically applying components based on information from OWNERS files. If this seems incorrect, please apply the Test-Predator-Wrong-Components label.
Cc: tguilbert@chromium.org
Ping servolk.
Status: Started (was: Assigned)
Re #2, good question, I'm not entirely sure, I suspect this could lead to a crash somewhere down the road, because we would ignore the DCHECK in release builds and would continue with an invalid video config.
Re #3, I think that's expected, because the crash happens in mp2t parser before it reports configs to SourceBufferState. If there was no crash due to the DCHECK, SourceBufferState::OnNewConfigs would try to check received configs against the expected codecs from the mime type and would reject this stream.
I believe the best course of action here is to check that the video config is actually valid in EsParserH264::UpdateVideoDecoderConfig, before we get to this DCHECK. I'll prepare a CL.
@#6 SGTM Thanks for looking into this, servolK@. I've provided initial comment on your CL.
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b00c058d243fe00970562f1d6a369617e5ecac7

commit 1b00c058d243fe00970562f1d6a369617e5ecac7
Author: Sergey Volk <servolk@google.com>
Date: Mon Oct 02 22:45:38 2017

Validate video config in EsParserH264 before emitting it

This issue was uncovered by a libFuzzer test, apparently the video
config read from the input stream might be invalid.

Bug:  763023 
Change-Id: I83b181ad2ce0a9f0e1cbae27497252b339381f51
Reviewed-on: https://chromium-review.googlesource.com/695386
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Sergey Volk <servolk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505826}
[modify] https://crrev.com/1b00c058d243fe00970562f1d6a369617e5ecac7/media/formats/mp2t/es_parser_adts.cc
[modify] https://crrev.com/1b00c058d243fe00970562f1d6a369617e5ecac7/media/formats/mp2t/es_parser_h264.cc
[modify] https://crrev.com/1b00c058d243fe00970562f1d6a369617e5ecac7/media/formats/mp2t/es_parser_mpeg1audio.cc

Status: Fixed (was: Started)
Project Member

Comment 10 by ClusterFuzz, Oct 3 2017

ClusterFuzz has detected this issue as fixed in range 505783:505840.

Detailed report: https://clusterfuzz.com/testcase?key=6021584140894208

Fuzzer: libFuzzer_mediasource_MP2T_AACSBR_pipeline_integration_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  video_decoder_config.IsValidConfig() in mp2t_stream_parser.cc
  media::mp2t::Mp2tStreamParser::OnVideoConfigChanged
  void base::internal::FunctorTraits<void
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=499783:499873
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=505783:505840

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6021584140894208

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 11 by ClusterFuzz, Oct 3 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 6021584140894208 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: -Test-Predator-AutoComponents Test-Predator-Auto-Components

Sign in to add a comment