New issue
Advanced search Search tips

Issue 920895 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jan 15
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

CHECK failure: false in webm_content_encodings_client.cc

Project Member Reported by ClusterFuzz, Jan 11

Issue description

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

Fuzzer: libFuzzer_mediasource_WEBM_OPUS_pipeline_integration_fuzzer
Fuzz target binary: mediasource_WEBM_OPUS_pipeline_integration_fuzzer
Job Type: x86_libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  false in webm_content_encodings_client.cc
  media::WebMContentEncodingsClient::OnBinary
  media::ParseBinary
  
Sanitizer: address (ASAN)

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

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for instructions to reproduce this bug locally.
 
Project Member

Comment 1 by ClusterFuzz, Jan 11

Cc: xhw...@chromium.org
Labels: ClusterFuzz-Auto-CC
Automatically adding ccs based on OWNERS file / target commit history.

If this is incorrect, please add ClusterFuzz-Wrong label.
Cc: -xhw...@chromium.org
Components: Internals>Media
Owner: xhw...@chromium.org
Status: Assigned (was: Untriaged)
The code only expects kWebMIdContentSigKeyID, but kWebMIdContentSignature is passed in, causing a DCHECK error. I'll work on a fix.
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 15

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

commit 5b21e46ee10660ecce5a4be7c03135c88681da23
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Tue Jan 15 03:11:49 2019

media: WebMContentEncodingsClient handles invalid WebM elements

Currently WebMContentEncodingsClient expects only valid WebM elements
are parsed and passed in. DCHECKs are added to check that.

However, in webm_parser.cc, we list all Matroska elements as valid,
including those not supported by the WebM spec. These elements will
be accepted by the parser, and passed to clients (e.g.
WebMContentEncodingsClient) for processing. Examples of such elements
are:
- ContentSignature
- ContentSigKeyID
- ContentCompression
- ContentSigAlgo
- ContentSigHashAlgo

This CL fixes WebMContentEncodingsClient to replace those DCHECKs with
error logs.

The alternative fix would be to remove all non-WebM elements from
webm_parser.cc, so that when encountered the parser will fail
immediately, and the clients should never see them.

Bug:  920895 
Test: The repro case doesn't fail anymore.
Change-Id: I222ec69b2322fe9b2effaf6c33489e34def032bc
Reviewed-on: https://chromium-review.googlesource.com/c/1407960
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622705}
[modify] https://crrev.com/5b21e46ee10660ecce5a4be7c03135c88681da23/media/formats/webm/webm_content_encodings_client.cc
[modify] https://crrev.com/5b21e46ee10660ecce5a4be7c03135c88681da23/media/formats/webm/webm_parser.cc

Project Member

Comment 5 by ClusterFuzz, Jan 15

ClusterFuzz has detected this issue as fixed in range 622689:622709.

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

Fuzzer: libFuzzer_mediasource_WEBM_OPUS_pipeline_integration_fuzzer
Fuzz target binary: mediasource_WEBM_OPUS_pipeline_integration_fuzzer
Job Type: x86_libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  false in webm_content_encodings_client.cc
  media::WebMContentEncodingsClient::OnBinary
  media::ParseBinary
  
Sanitizer: address (ASAN)

Fixed: https://clusterfuzz.com/revisions?job=x86_libfuzzer_chrome_asan_debug&range=622689:622709

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

See https://github.com/google/clusterfuzz-tools for instructions to reproduce this bug locally.

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

Comment 6 by ClusterFuzz, Jan 15

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Sign in to add a comment