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

Issue 750502 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in blink::AudioWorklet::RegisterContext

Project Member Reported by ClusterFuzz, Jul 30 2017

Issue description

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

Fuzzer: inferno_twister_c
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000040
Crash State:
  blink::AudioWorklet::RegisterContext
  blink::AudioContext::Create
  blink::V8AudioContext::constructorCallback
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=490547:490630

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


Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: msrchandra@chromium.org
Labels: M-62 Test-Predator-Wrong
Owner: hongchan@chromium.org
Status: Assigned (was: Untriaged)
Predator did not provide any possible suspects.
Assigning to the concern from the below CL --
https://chromium.googlesource.com/chromium/src/+log/5e40f323bc67b22cd168b14b019bd5dc1b48c973..63f3c7bcfc46a88f4c1b8997f2e0238816521607?pretty=fuller

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

@hongchan -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.
Cc: nhiroki@chromium.org
Components: Blink>WebAudio
It seems like the validaty/lifecycle of AudioWorklet in a detached iframe is problematic: AudioContext is trying to register itself to AudioWorklet even when it is not available.

```
var iframe = document.createElementNS("http://www.w3.org/1999/xhtml", "iframe");
document.body.appendChild(iframe);
var frameWin = iframe.contentWindow;
new frameWin.AudioContext();
document.body.removeChild(iframe);
new frameWin.AudioContext();
```

nhiroki@ What's the worklet's behavior in the detached iframe? Seems like it rejects promises from .addModule() but not sure how it behaves internally.
Status: Started (was: Assigned)
IIUC, behavior on detached frames are generally not well-defined in the HTML specs. IMO, rejecting a promise on a detached frame is a common implementation pattern (at least used in Worklet and ServiceWorker).

When a frame is detached, ContextLifecycleObserver::ContextDestroyed() is called and ContextLifecycleObserver::GetExecutionContext() starts returning nullptr. addModule() checks a returned value of GetExecutionContext() at the top to detect the frame detach.

This is the layout test for addModule():
https://chromium.googlesource.com/chromium/src/+/b74391aa92f462b32022e17d32339acd6c42e8ea/third_party/WebKit/LayoutTests/http/tests/worklet/chromium/import-on-detached-iframe.html
Thanks for the response. Yes, I saw the layout test and I thought it might be okay for use to avoid registering/unresgistering context when the Worklet is not valid somehow.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 1 2017

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

commit b300a0cc8ffeb9e6cde4fc6edaa3ab7d784d8f23
Author: Hongchan Choi <hongchan@chromium.org>
Date: Tue Aug 01 19:01:34 2017

Check if AudioWorklet exists before register AudioContext

In some cases where the worklet cannot function, registering
AudioContext is not possible. (e.g. detached iframe from the document)
So BaseAudioContext needs to check the existence of worklet before
registering itself.

The fix is locally confirmed; ASAN does not crash anymore with it.

Bug:  750502 
Change-Id: Id35358b0811c7f4c3476ab62e49611a8d28bad60
Reviewed-on: https://chromium-review.googlesource.com/594852
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491070}
[modify] https://crrev.com/b300a0cc8ffeb9e6cde4fc6edaa3ab7d784d8f23/third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp

Status: Fixed (was: Started)
Fixed and waiting for CF to confirm.
Project Member

Comment 8 by ClusterFuzz, Aug 2 2017

ClusterFuzz has detected this issue as fixed in range 491035:491089.

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

Fuzzer: inferno_twister_c
Job Type: linux_asan_content_shell_drt
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000040
Crash State:
  blink::AudioWorklet::RegisterContext
  blink::AudioContext::Create
  blink::V8AudioContext::constructorCallback
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=490547:490630
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_content_shell_drt&range=491035:491089

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


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 9 by ClusterFuzz, Aug 2 2017

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

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

Sign in to add a comment