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

Issue 765625 link

Starred by 2 users

Issue metadata

Status: Fixed
Merged: issue 759325
Owner:
Closed: Sep 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference WRITE in blink::MediaStream::MediaStream

Project Member Reported by ClusterFuzz, Sep 15 2017

Issue description

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

Fuzzer: inferno_twister
Job Type: windows_asan_chrome
Platform Id: windows

Crash Type: Null-dereference WRITE
Crash Address: 0x00000000
Crash State:
  blink::MediaStream::MediaStream
  blink::MediaStream::Create
  blink::HTMLMediaElementCapture::captureStream
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome&range=497280:497332

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

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: msrchandra@chromium.org pnangunoori@chromium.org
Labels: Test-Predator-Wrong-CLs M-62
Owner: emir...@chromium.org
Status: Assigned (was: Untriaged)
Using the provided regression range assigning to the possible suspect as per the change made for the file, “HTMLMediaElementCapture.cpp”

Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/0c6478f24f6bd0843e166aed9da19481c24d5af8

@emircan -- Could you please look into this issue, kindly reassign if it has nothing to do with your changes.
Mergedinto: 759325
Status: Duplicate (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 28 2017

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

commit dfbd5e4a13e399494354a4a51e4fb0a809090b50
Author: Emircan Uysaler <emircan@chromium.org>
Date: Thu Sep 28 07:05:16 2017

Handle empty currentSrc() in HTMLMediaElementCapture

This CL addresses issues discovered by fuzz tests:
- currentSrc() can be null for a MediaStream. We can get descriptor via
GetSrcObject() then.
- VideoFramePool::CreateFrame() can fail then as resolution hasn't been
set. We skip capture for those frames then.

Bug:  765625 
Change-Id: Ifc4f9c12d25511dd12f94cf01eb383078360dbfd
Reviewed-on: https://chromium-review.googlesource.com/687962
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Commit-Queue: Emircan Uysaler <emircan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504929}
[modify] https://crrev.com/dfbd5e4a13e399494354a4a51e4fb0a809090b50/content/renderer/media_capture_from_element/html_video_element_capturer_source.cc
[modify] https://crrev.com/dfbd5e4a13e399494354a4a51e4fb0a809090b50/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp

Comment 4 by guidou@chromium.org, Sep 28 2017

Cc: guidou@chromium.org emir...@chromium.org
 Issue 769649  has been merged into this issue.
Labels: Merge-Request-62
Status: Fixed (was: Duplicate)
Project Member

Comment 7 by sheriffbot@chromium.org, Sep 29 2017

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

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

Comment 8 by ClusterFuzz, Sep 29 2017

ClusterFuzz has detected this issue as fixed in range 504875:505014.

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

Fuzzer: inferno_twister
Job Type: windows_asan_chrome
Platform Id: windows

Crash Type: Null-dereference WRITE
Crash Address: 0x00000000
Crash State:
  blink::MediaStream::MediaStream
  blink::MediaStream::Create
  blink::HTMLMediaElementCapture::captureStream
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome&range=497280:497332
Fixed: https://clusterfuzz.com/revisions?job=windows_asan_chrome&range=504875:505014

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

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.
Cc: niklase@chromium.org jbroman@chromium.org
 Issue 759325  has been merged into this issue.
Thanks for the fix - do you think this is really needed for M62, or can we wait until M63?
It is necessary:
- Without this patch, we get a crash in capture from DOM elements with src null.
- This feature is advertised and it would be nice to eliminate the crash: https://blog.chromium.org/2017/09/chrome-62-beta-network-quality.html 
Labels: -Merge-Review-62 Merge-Approved-62
Thanks for the feedback. Approving merge to M62. branch:3202
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 3 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3460c77c40fa862ffa1725cb7d8117a013b74b25

commit 3460c77c40fa862ffa1725cb7d8117a013b74b25
Author: Emircan Uysaler <emircan@chromium.org>
Date: Tue Oct 03 21:56:59 2017

Handle empty currentSrc() in HTMLMediaElementCapture

This CL addresses issues discovered by fuzz tests:
- currentSrc() can be null for a MediaStream. We can get descriptor via
GetSrcObject() then.
- VideoFramePool::CreateFrame() can fail then as resolution hasn't been
set. We skip capture for those frames then.

TBR=emircan@chromium.org

(cherry picked from commit dfbd5e4a13e399494354a4a51e4fb0a809090b50)

Bug:  765625 
Change-Id: Ifc4f9c12d25511dd12f94cf01eb383078360dbfd
Reviewed-on: https://chromium-review.googlesource.com/687962
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Commit-Queue: Emircan Uysaler <emircan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#504929}
Reviewed-on: https://chromium-review.googlesource.com/699292
Cr-Commit-Position: refs/branch-heads/3202@{#563}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/3460c77c40fa862ffa1725cb7d8117a013b74b25/content/renderer/media_capture_from_element/html_video_element_capturer_source.cc
[modify] https://crrev.com/3460c77c40fa862ffa1725cb7d8117a013b74b25/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp

Sign in to add a comment