New issue
Advanced search Search tips

Issue 788508 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Security



Sign in to add a comment

Heap-use-after-free in media::PipelineImpl::RendererWrapper::Stop

Project Member Reported by ClusterFuzz, Nov 25 2017

Issue description

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

Fuzzer: libFuzzer_media_pipeline_integration_fuzzer
Job Type: mac_libfuzzer_chrome_asan
Platform Id: mac

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x611000038080
Crash State:
  media::PipelineImpl::RendererWrapper::Stop
  void base::internal::Invoker<base::internal::BindState<void
  base::OnceCallback<void
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=mac_libfuzzer_chrome_asan&range=511295:511333

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Nov 25 2017

Components: Internals>Core Internals>Media
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, Nov 25 2017

Labels: Test-Predator-Auto-Owner
Owner: sande...@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/91965df34bca6d9236e62c7a228354d17843dcfa (Limit TrackRunInfo.samples allocation size to 150MB.).

If this is incorrect, please remove the owner and apply the Test-Predator-Wrong-CLs label.
Labels: M-63
Project Member

Comment 4 by sheriffbot@chromium.org, Nov 26 2017

Labels: -Security_Impact-Head Security_Impact-Beta
Project Member

Comment 5 by sheriffbot@chromium.org, Nov 26 2017

Labels: ReleaseBlock-Stable
This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by sheriffbot@chromium.org, Nov 26 2017

Labels: Pri-1

Comment 7 by gov...@chromium.org, Nov 27 2017

Cc: awhalley@chromium.org
+awhalley@

Comment 8 by awhalley@google.com, Nov 27 2017

Labels: -Security_Impact-Beta -M-63 Security_Impact-Head M-64
Note: The suspected CL is almost certainly incorrect, but I'm still probably the right person to take this bug.
Cc: xhw...@chromium.org
I am unable to reproduce this issue (even with the provided build), but strongly suspect that it is test-only. I have created a CL to bring the test path more in line with the production path, hopefully the bots can confirm whether it fixes the issue.
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 28 2017

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

commit dbce4c1629ddc42427c466f23073fd85a053b291
Author: Dan Sanders <sandersd@chromium.org>
Date: Tue Nov 28 21:59:50 2017

[media] When testing, call RendererWrapper::Stop() synchronously.

Before this change, the testing path invoked RenderWrapper::Stop() by
posting a task, which would execute after PipelineImpl::Stop().

The production path also posts a task, but to a different thread, and
PipelineImpl::Stop() waits for that task to execute before returning.

This change makes the testing path more similar to the production path,
by ensuring that RendererWrapper::Stop() executes before
PipelineImpl::Stop() returns.

The PipelineTeardownTest is also updated so that expectations on the
Demuxer that Stop() the Pipeline are posted rather than directly
called. This better matches the production behavior where they would
be on separate threads and therefore cannot be re-entrant.

Bug:  788508 
Change-Id: I894ee6c2febb3b739aaa809c6aa0d9388b76b64a
Reviewed-on: https://chromium-review.googlesource.com/791860
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519862}
[modify] https://crrev.com/dbce4c1629ddc42427c466f23073fd85a053b291/media/base/pipeline_impl.cc
[modify] https://crrev.com/dbce4c1629ddc42427c466f23073fd85a053b291/media/base/pipeline_impl_unittest.cc

Project Member

Comment 12 by ClusterFuzz, Nov 29 2017

ClusterFuzz has detected this issue as fixed in range 519701:519865.

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

Fuzzer: libFuzzer_media_pipeline_integration_fuzzer
Job Type: mac_libfuzzer_chrome_asan
Platform Id: mac

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x611000038080
Crash State:
  media::PipelineImpl::RendererWrapper::Stop
  void base::internal::Invoker<base::internal::BindState<void
  base::OnceCallback<void
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=mac_libfuzzer_chrome_asan&range=511295:511333
Fixed: https://clusterfuzz.com/revisions?job=mac_libfuzzer_chrome_asan&range=519701:519865

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

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.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 13 by ClusterFuzz, Nov 29 2017

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

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

Comment 14 by sheriffbot@chromium.org, Nov 29 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 15 by sheriffbot@chromium.org, Mar 7 2018

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 16 by sheriffbot@chromium.org, Mar 27 2018

Labels: -Security_Impact-Head -M-64 M-65 Security_Impact-Stable
Labels: -ReleaseBlock-Stable

Sign in to add a comment