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

Issue 612049 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Mac
Pri: 2
Type: Bug-Security



Sign in to add a comment

Heap-use-after-free in content::MediaStreamVideoSource::RemoveTrack

Project Member Reported by ClusterFuzz, May 15 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5472302287814656

Fuzzer: inferno_twister_custom_bundle
Job Type: mac_asan_chrome
Platform Id: mac

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x6140002e6168
Crash State:
  content::MediaStreamVideoSource::RemoveTrack
  content::MediaStreamVideoTrack::~MediaStreamVideoTrack
  content::MediaStreamVideoTrack::~MediaStreamVideoTrack
  
Recommended Security Severity: High

Regressed: https://cluster-fuzz.appspot.com/revisions?job=mac_asan_chrome&range=392978:393031

Minimized Testcase (2.19 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97fKxUC4TKp8L0cniDt5A5Z5sC_skshCYlCY2Ju_1gHCwyb2XuQejB8nL8LO-Wj6ZEE0OLWIRkXNh3XLlVO-5OrhcYa1lwwGjg_Bt3Jgtc_HEVKffMDSJ956ij3ov7y8iEbNwGmSe-QB2UQ5AJxsRPOB_DHCg

Additional requirements: Requires HTTP

Filer: mmoroz

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 

Comment 1 by mmoroz@chromium.org, May 15 2016

Cc: m...@chromium.org mikhail....@intel.com
Labels: Pri-1
Owner: x...@chromium.org
Looks similar to bug 611299.
Project Member

Comment 2 by ClusterFuzz, May 16 2016

Status: Assigned (was: Available)
Project Member

Comment 3 by sheriffbot@chromium.org, May 16 2016

Labels: M-52
Project Member

Comment 4 by sheriffbot@chromium.org, May 16 2016

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

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Components: Internals>Media

Comment 6 by m...@chromium.org, May 16 2016

Labels: -Pri-1 Pri-2
This issue does not seem related to any recent CLs. Looking at source code history, it's likely been a problem since Apr 2014.

Seems that the assumption that the MediaStreamVideoSource outlives the track is false. We can fix this quickly by having the track store a base::WeakPtr<source>.
Project Member

Comment 7 by sheriffbot@chromium.org, May 16 2016

Labels: OS-Windows Fracas OS-Android
Users experienced this crash on the following builds:

Win Canary 52.0.2738.0 -  2.16 CPM, 18 reports, 6 clients (signature content::MediaStreamVideoSource::RemoveTrack)
Mac Canary 52.0.2738.0 -  1.20 CPM, 1 reports, 1 clients (signature content::MediaStreamVideoSource::RemoveTrack)
Android Dev 52.0.2735.0 -  1.41 CPM, 21 reports, 3 clients (signature content::MediaStreamVideoSource::RemoveTrack)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas
Project Member

Comment 8 by bugdroid1@chromium.org, May 18 2016

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

commit 339f60de6b251ae9872fa2c690439aee309b7ea2
Author: xjz <xjz@chromium.org>
Date: Wed May 18 03:17:42 2016

MediaVideoStreamTrack should store weak reference to the media source.
Since MediaVideoStreamSource cann't guarantee to outlive
MediaVideoStreamTrack, directly using its raw pointer might cause the
incorrect use of MediaVideoStreamSource after it is destructed.

BUG= 612049 

Review-Url: https://codereview.chromium.org/1981113002
Cr-Commit-Position: refs/heads/master@{#394319}

[modify] https://crrev.com/339f60de6b251ae9872fa2c690439aee309b7ea2/content/renderer/media/media_stream_video_source.h
[modify] https://crrev.com/339f60de6b251ae9872fa2c690439aee309b7ea2/content/renderer/media/media_stream_video_track.cc
[modify] https://crrev.com/339f60de6b251ae9872fa2c690439aee309b7ea2/content/renderer/media/media_stream_video_track.h

Project Member

Comment 9 by sheriffbot@chromium.org, May 19 2016

Labels: OS-Linux
Users experienced this crash on the following builds:

Win Dev 52.0.2739.0 -  1.51 CPM, 151 reports, 140 clients (signature content::MediaStreamVideoSource::RemoveTrack)
Mac Canary 52.0.2740.0 -  1.18 CPM, 3 reports, 3 clients (signature content::MediaStreamVideoSource::RemoveTrack)
Linux Dev 52.0.2739.0 -  4.06 CPM, 1 reports, 1 clients (signature content::MediaStreamVideoSource::RemoveTrack)
Android Dev 52.0.2739.3 -  1.69 CPM, 2 reports, 2 clients (signature content::MediaStreamVideoSource::RemoveTrack)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

Comment 10 by x...@chromium.org, May 23 2016

Status: Fixed (was: Assigned)
Project Member

Comment 11 by ClusterFuzz, May 23 2016

Labels: Merge-Triage
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Request-XX label, where XX is the Chrome milestone.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz
Project Member

Comment 12 by sheriffbot@chromium.org, May 24 2016

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify

Comment 13 by m...@chromium.org, May 24 2016

Labels: Merge-Request-51
Requesting M51 merge for the fix.

Comment 14 by tin...@google.com, May 24 2016

Labels: -Merge-Request-51 Merge-Review-51 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before stable on M51, manual review required.
Before we approve merge to M51, Could you please confirm whether this change is baked/verified in Canary and safe to merge?

Comment 16 by x...@chromium.org, May 25 2016

The change should be safe to merge. It was in M52 for one week.
Project Member

Comment 17 by ClusterFuzz, May 27 2016

ClusterFuzz has detected this issue as fixed in range 396024:396083.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5472302287814656

Fuzzer: inferno_twister_custom_bundle
Job Type: mac_asan_chrome
Platform Id: mac

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x6140002e6168
Crash State:
  content::MediaStreamVideoSource::RemoveTrack
  content::MediaStreamVideoTrack::~MediaStreamVideoTrack
  content::MediaStreamVideoTrack::~MediaStreamVideoTrack
  
Recommended Security Severity: High

Regressed: https://cluster-fuzz.appspot.com/revisions?job=mac_asan_chrome&range=392978:393031
Fixed: https://cluster-fuzz.appspot.com/revisions?job=mac_asan_chrome&range=396024:396083

Minimized Testcase (2.19 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97fKxUC4TKp8L0cniDt5A5Z5sC_skshCYlCY2Ju_1gHCwyb2XuQejB8nL8LO-Wj6ZEE0OLWIRkXNh3XLlVO-5OrhcYa1lwwGjg_Bt3Jgtc_HEVKffMDSJ956ij3ov7y8iEbNwGmSe-QB2UQ5AJxsRPOB_DHCg

Additional requirements: Requires HTTP

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Cc: timwillis@chromium.org
Labels: -Merge-Review-51 Merge-Approved-51
Approving merge to M51 branch 2704 based comment #16 and comment #17.  Please merge ASAP so we can take it for next week Stable release. Thank you.

Comment 19 by x...@chromium.org, May 27 2016

Yuri: Could you please do this merge for me? I am still not a full committer yet. Thanks!

Comment 20 by m...@chromium.org, May 27 2016

Doing it now...
Project Member

Comment 21 by bugdroid1@chromium.org, May 27 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5cfb468a11a4eb74cdab476b9c5b13109117a1a9

commit 5cfb468a11a4eb74cdab476b9c5b13109117a1a9
Author: Yuri Wiitala <miu@chromium.org>
Date: Fri May 27 21:08:19 2016

MediaVideoStreamTrack should store weak reference to the media source. Since MediaVideoStreamSource cann't guarantee to outlive MediaVideoStreamTrack, directly using its raw pointer might cause the incorrect use of MediaVideoStreamSource after it is destructed.

BUG= 612049 

Review-Url: https://codereview.chromium.org/1981113002
Cr-Commit-Position: refs/heads/master@{#394319}
(cherry picked from commit 339f60de6b251ae9872fa2c690439aee309b7ea2)

Review URL: https://codereview.chromium.org/2011163004 .

Cr-Commit-Position: refs/branch-heads/2704@{#670}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/5cfb468a11a4eb74cdab476b9c5b13109117a1a9/content/renderer/media/media_stream_video_source.h
[modify] https://crrev.com/5cfb468a11a4eb74cdab476b9c5b13109117a1a9/content/renderer/media/media_stream_video_track.cc
[modify] https://crrev.com/5cfb468a11a4eb74cdab476b9c5b13109117a1a9/content/renderer/media/media_stream_video_track.h

Labels: -Merge-Triage -Security_Impact-Head Security_Impact-Stable M-51 Release-1-M51
Updating impact based on merge labels.
Project Member

Comment 23 by sheriffbot@chromium.org, Aug 30 2016

Labels: -Restrict-View-SecurityNotify
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 24 by sheriffbot@chromium.org, Oct 1 2016

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 25 by sheriffbot@chromium.org, Oct 2 2016

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
Labels: allpublic

Sign in to add a comment