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

Issue 829280 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Mac
Pri: 1
Type: Bug-Security



Sign in to add a comment

Heap-use-after-free in cc::VideoResourceUpdater::AllocateResource

Project Member Reported by ClusterFuzz, Apr 5 2018

Issue description

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

Fuzzer: inferno_webbot
Job Type: mac_asan_chrome
Platform Id: mac

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x6150001ea080
Crash State:
  cc::VideoResourceUpdater::AllocateResource
  cc::VideoResourceUpdater::CreateForSoftwarePlanes
  cc::VideoResourceUpdater::CreateExternalResourcesFromVideoFrame
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=mac_asan_chrome&range=548267:548286

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

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, Apr 5 2018

Components: Internals>Compositing
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 sheriffbot@chromium.org, Apr 5 2018

Labels: M-67
Project Member

Comment 3 by sheriffbot@chromium.org, Apr 5 2018

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 4 by sheriffbot@chromium.org, Apr 5 2018

Labels: Pri-1
Owner: kylec...@chromium.org
Status: Assigned (was: Untriaged)
kylechar@chromium.org, could you take a look at this issue?  The crashing point is in the code you changed lately. 
https://chromium-review.googlesource.com/c/chromium/src/+/956968

Cc: kylec...@chromium.org hbengali@chromium.org
Owner: mlamouri@chromium.org
VideoResourceUpdater is supposed to be destroyed before the ContextProvider is destroyed, so this shouldn't be possible. There is a new UseSurfaceLayerForVideo feature that changes how video works though. As far as I can tell, UseSurfaceLayerForVideo doesn't handle context loss and I see a CL in the regression range to enable the feature.

https://chromium.googlesource.com/chromium/src/+/2725fa428722a5c26d55d78cfc299069521ce78d

I don't see any way for VideoFrameResourceProvider to find out about lost context so it can destroy the VideoResourceUpdater first. I'm not familiar with the code so I'm not positive if it's handled elsewhere. Either way, this seems like the culprit.

+mlamouri to find an owner.
Project Member

Comment 7 by ClusterFuzz, Apr 5 2018

Labels: OS-Linux
Cc: mlamouri@chromium.org
Owner: lethalantidote@chromium.org
Project Member

Comment 9 by ClusterFuzz, Apr 9 2018

Labels: Fuzz-Blocker ReleaseBlock-Beta
This crash occurs very frequently on mac platform and is likely preventing the fuzzer inferno_webbot from making much progress. Fixing this will allow more bugs to be found.

Marking this bug as a blocker for next Beta release.

If this is incorrect, please add ClusterFuzz-Wrong label and remove the ReleaseBlock-Beta label.
A friendly reminder that M67 branch is tomorrow, Thursday 04/12! Your bug is labelled as Beta ReleaseBlock, pls make sure to land the fix ASAP to trunk. This way we branch M67 from a high quality trunk. Thank you.
Any update on this bug as M67 is branching today (04/12) and this bug is marked as M67 Beta Blocker?
This was also another bug that was found through the finch experiment. Since the finch experiment is now disabled, it should not be an issue. 
Labels: -ReleaseBlock-Beta
Labels: -ReleaseBlock-Stable
Project Member

Comment 15 by sheriffbot@chromium.org, Apr 13 2018

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
Status: Started (was: Assigned)
Probably not worth fighting with sheriffbot@, we know machines always win at the end :) Though, this is not RBS because it's not actually launching. ClusterFuzz is using the flag to turn the feature on.

This said, lethalantidote@ has CL in review.
Project Member

Comment 17 by sheriffbot@chromium.org, Apr 18 2018

Labels: -Security_Impact-Head Security_Impact-Beta

Comment 18 by vakh@chromium.org, Apr 19 2018

Labels: -ReleaseBlock-Stable ReleaseBlock-NA
Project Member

Comment 19 by ClusterFuzz, Apr 22 2018

ClusterFuzz has detected this issue as fixed in range 552588:552589.

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

Fuzzer: inferno_webbot
Job Type: mac_asan_chrome
Platform Id: mac

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x6150001ea080
Crash State:
  cc::VideoResourceUpdater::AllocateResource
  cc::VideoResourceUpdater::CreateForSoftwarePlanes
  cc::VideoResourceUpdater::CreateExternalResourcesFromVideoFrame
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=mac_asan_chrome&range=548267:548286
Fixed: https://clusterfuzz.com/revisions?job=mac_asan_chrome&range=552588:552589

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

See https://github.com/google/clusterfuzz-tools 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 20 by ClusterFuzz, Apr 22 2018

Labels: ClusterFuzz-Verified
Status: Verified (was: Started)
ClusterFuzz testcase 4900207016542208 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 21 by sheriffbot@chromium.org, Apr 22 2018

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

Comment 22 by sheriffbot@chromium.org, Apr 27 2018

Labels: Merge-Request-67
Project Member

Comment 23 by sheriffbot@chromium.org, Apr 27 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: awhalley@chromium.org
+awhalley@ for M67 merge review. 
mlamouri@, could you confirm that this feature won't be enabled (by default or via finch) in M67?  If so, no need to merge.
Yes, that's correct. This feature is not enabled by default on client neither is it via Finch. It's only a perf test config.
In this case no merge is needed to M67, correct?
Labels: -Hotlist-Merge-Review -Merge-Review-67 Merge-Rejected-67
Correct.
Project Member

Comment 29 by sheriffbot@chromium.org, Jul 29

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

Sign in to add a comment