Automatically applying components based on crash stacktrace and information from OWNERS files. If this is incorrect, please apply the Test-Predator-Wrong-Components label.

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

Regression range points to March, so I think this is Impact-Stable.

mcasas, could you please take a look at this, or help with assigning it to the right person?

I think this already showed up once in  https://crbug.com/749851 ? 
emircan@ can you take a look please? Thanks

This issue found by fuzz tests existed since March.
- For captured Mediastream, we already created a clone so loadedmetadata should do nothing.
- For captured normal element, which are assigned as MediaStream after loadedmetadata, I am not sure what to do. We cannot use CreateHTML*ElementCapturer() calls because they are WebMediaPlayerImpl specific. We can clone MediaStream tracks but then we need it to keep tracking, i.e. clone the descriptor like early clone. AFAICT there isn't functionality for that. guidou@ WDYT? Would below patch where we do nothing at all be enough? 
https://chromium-review.googlesource.com/c/chromium/src/+/699747

When a MediaStream is assigned after loadedmetadata, we have to remove the tracks from the old returned MediaStream and replace its tracks with clones of the tracks of the element's MediaStream.
This has to be done before creating the capturerSources in order to avoid the incorrect cast.

I found one reason why we cannot change the cast to cover WebMediaPlayerMS in the current design. Audio capture is based on using a media::WebAudioSourceProviderImpl which is not a member of WebMediaPlayerMS[0].

I looked at the possibility of adding a "live clone" that keeps track of changes in captured MediaStream tracks. However, I am not sure if it is needed based on the spec[1]: "The set of captured MediaStreamTracks change if the source of the media element changes. If the source for the media element ends, a different source is selected." This sounds like we should only track changes only for loadedmetadata event. Then, I will just take the suggestion above, see PS#2[2]. WDYT?

[0] https://cs.chromium.org/chromium/src/content/renderer/media_capture_from_element/html_audio_element_capturer_source.cc?rcl=2ea3968e0c87432c0d759026fa4be13c6d54c82e&l=23
[1] https://w3c.github.io/mediacapture-fromelement/#html-media-element-media-capture-extensions
[2] https://chromium-review.googlesource.com/c/chromium/src/+/699747

I think [2] is a good improvement over the current situation and IMO landing it is better than not landing it.

However, as a user, the live clone is the one that would make most sense.
With [2], if the set of tracks in the stream changes, the element will display the new tracks, but the capturer will stay capturing the old tracks, which might no longer be associated with the element at all. Thus, it will not look like a true element capture.

> However, as a user, the live clone is the one that would make most sense.
I agree. I think we should land [2] first as fix for security and fuzz issues. I will send it for review. Then, we can spell out "live clone" in the spec and work on it in a follow-up. 

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

commit fe8edc0fafdd3c94b1f8a944a87421baa125783d
Author: Emircan Uysaler <emircan@chromium.org>
Date: Fri Oct 06 23:54:31 2017

Fix loadedmetadata checks for MediaStreams in HTMLMediaElementCapture

This CL handles the case where a MediaStream is assigned after loadedmetadata.
We should clone the newly generated tracks to the returned media stream.

Bug:  771117 
Change-Id: I7d7625133fcff6fc91c5473aafa4d7895c81c311
Reviewed-on: https://chromium-review.googlesource.com/699747
Commit-Queue: Emircan Uysaler <emircan@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507233}
[modify] https://crrev.com/fe8edc0fafdd3c94b1f8a944a87421baa125783d/third_party/WebKit/Source/modules/mediacapturefromelement/HTMLMediaElementCapture.cpp

ClusterFuzz has detected this issue as fixed in range 507223:507264.

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

Fuzzer: inferno_twister
Job Type: linux_ubsan_vptr_content_shell_drt
Platform Id: linux

Crash Type: Bad-cast
Crash Address: 0x0409e6e92908
Crash State:
  Bad-cast to media::WebMediaPlayerImpl from base class subobject at offset 8
  content::HtmlVideoElementCapturerSource::CreateFromWebMediaPlayerImpl
  content::RendererBlinkPlatformImpl::CreateHTMLVideoElementCapturer
  
Sanitizer: undefined (UBSAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_content_shell_drt&range=455700:456019
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_content_shell_drt&range=507223:507264

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

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.

ClusterFuzz testcase 6668325714395136 is verified as fixed, so closing issue as verified.

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

This bug requires manual review: M63 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), gkihumba@(ChromeOS), govind@(Desktop)

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

+awhalley@ (Security TPM) for M63 merge review

Note that the fix for this landed originally in 63, so no merge necessary.
I guess the merge request to 63 is a mistake by sheriffbot.

Yes, see https://chromiumdash.appspot.com/commit/fe8edc0fafdd3c94b1f8a944a87421baa125783d.

Removing "Merge-Review-63" label per comment #20 and #21.

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