New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment
link

Issue 883643: Null-dereference READ in blink::CallbackInterfaceBase::CallbackInterfaceBase

Reported by ClusterFuzz, Sep 13 Project Member

Issue description

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

Fuzzer: lcamtuf_cross_fuzz
Job Type: linux_ubsan_vptr_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  blink::CallbackInterfaceBase::CallbackInterfaceBase
  blink::V8EventListenerImpl::V8EventListenerImpl
  blink::V8EventListenerHelper::GetEventListener
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=588747:588748

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

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Sep 13

Project Member
Components: Blink>Bindings
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.

Comment 2 by ClusterFuzz, Sep 13

Project Member
Labels: Test-Predator-Auto-Owner
Owner: yukiy@google.com
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/da27378f03ea3dc4bdba82e3cb1d13ad2fef7a7c (Split implementation of EventListener and EventHandler).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.

Comment 3 by yukiy@google.com, Sep 13

Cc: haraken@chromium.org peria@chromium.org yukishiino@chromium.org

Comment 4 by yukishiino@chromium.org, Sep 13

Status: Started (was: Assigned)

Comment 5 by yukiy@google.com, Sep 14

Owner: yukishiino@chromium.org
I've investigated this crush and found that v8::JSReceiver::GetCreationContext() can return empty handle. This could cause crashes in not only blink::V8EventListenerImpl but also other callback classes.
In such a case, it is better to throw TypeError before calling blink::V8EventListenerHelper::GetEventListener().
This will be addressed by yukishiino@.

Comment 6 by bugdroid1@chromium.org, Sep 19

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

commit e58d4344005a556409d314662403e9df6fc2c060
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Wed Sep 19 12:09:29 2018

v8binding: Handles a cross origin object as IDL callback interface.

Since any object can be an IDL callback interface, web author can
pass a cross origin object as an IDL callback interface. However,
in case of a remote context (e.g. oopif/site-per-process), the
cross origin object does not have a creation context of
v8::Context, and it causes crash.

This patch fixes the issue by checking whether an object has a
non-empty creation context.

http/tests/dom/eventlistener-with-remote-context.html
demonstrates the issue and its fix.

Change-Id: I3251f1caf8df3add3505afa75a1a056786c87c7c
Bug:  883643 ,  886588 
Reviewed-on: https://chromium-review.googlesource.com/1226893
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592369}
[add] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/WebKit/LayoutTests/http/tests/dom/eventlistener-with-remote-context.html
[modify] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/blink/renderer/bindings/core/v8/v8_event_listener_impl.cc
[modify] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/blink/renderer/bindings/templates/callback_interface.cpp.tmpl
[modify] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/blink/renderer/bindings/templates/callback_interface.h.tmpl
[modify] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/blink/renderer/bindings/templates/methods.cpp.tmpl
[modify] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/blink/renderer/bindings/tests/results/core/v8_test_callback_interface.cc
[modify] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/blink/renderer/bindings/tests/results/core/v8_test_callback_interface.h
[modify] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/blink/renderer/bindings/tests/results/core/v8_test_legacy_callback_interface.cc
[modify] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/blink/renderer/bindings/tests/results/core/v8_test_legacy_callback_interface.h
[modify] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/blink/renderer/bindings/tests/results/core/v8_test_object.cc
[modify] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/blink/renderer/bindings/tests/results/core/v8_test_typedefs.cc
[modify] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/blink/renderer/platform/bindings/callback_interface_base.cc
[modify] https://crrev.com/e58d4344005a556409d314662403e9df6fc2c060/third_party/blink/renderer/platform/bindings/callback_interface_base.h

Comment 7 by yukishiino@chromium.org, Sep 19

Status: Fixed (was: Started)

Comment 8 by ClusterFuzz, Sep 20

Project Member
ClusterFuzz has detected this issue as fixed in range 592368:592369.

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

Fuzzer: lcamtuf_cross_fuzz
Job Type: linux_ubsan_vptr_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  blink::CallbackInterfaceBase::CallbackInterfaceBase
  blink::V8EventListenerImpl::V8EventListenerImpl
  blink::V8EventListenerHelper::GetEventListener
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=588747:588748
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=592368:592369

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

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.

Comment 9 by ClusterFuzz, Sep 20

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

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

Comment 11 by yukishiino@chromium.org, Dec 5

Status: Started (was: Verified)

Comment 12 by yukishiino@chromium.org, Dec 5

 Issue 911646  has been merged into this issue.

Comment 13 by bugdroid1@chromium.org, Dec 11

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0d557dedf31c8cf30bee794ea983b5a16f899b0e

commit 0d557dedf31c8cf30bee794ea983b5a16f899b0e
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Tue Dec 11 10:27:02 2018

v8binding: Do not hold a cross origin ScriptState in IDL callback interface

Make IDL callback interface not hold a ScriptState of its
creation context when it's cross origin from the incumbent
realm.

This is the same fix as https://crrev.com/c/1314023 for
IDL callback interface.

Bug:  886588 ,  883643 
Change-Id: I38887c8d460d2b6879818bb31427f04a15dcf815
Reviewed-on: https://chromium-review.googlesource.com/c/1343816
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615480}
[modify] https://crrev.com/0d557dedf31c8cf30bee794ea983b5a16f899b0e/third_party/blink/renderer/bindings/core/v8/js_event_listener.h
[modify] https://crrev.com/0d557dedf31c8cf30bee794ea983b5a16f899b0e/third_party/blink/renderer/bindings/templates/callback_interface.cc.tmpl
[modify] https://crrev.com/0d557dedf31c8cf30bee794ea983b5a16f899b0e/third_party/blink/renderer/bindings/templates/callback_interface.h.tmpl
[modify] https://crrev.com/0d557dedf31c8cf30bee794ea983b5a16f899b0e/third_party/blink/renderer/bindings/templates/methods.cc.tmpl
[modify] https://crrev.com/0d557dedf31c8cf30bee794ea983b5a16f899b0e/third_party/blink/renderer/bindings/tests/results/core/v8_test_callback_interface.cc
[modify] https://crrev.com/0d557dedf31c8cf30bee794ea983b5a16f899b0e/third_party/blink/renderer/bindings/tests/results/core/v8_test_callback_interface.h
[modify] https://crrev.com/0d557dedf31c8cf30bee794ea983b5a16f899b0e/third_party/blink/renderer/bindings/tests/results/core/v8_test_legacy_callback_interface.cc
[modify] https://crrev.com/0d557dedf31c8cf30bee794ea983b5a16f899b0e/third_party/blink/renderer/bindings/tests/results/core/v8_test_legacy_callback_interface.h
[modify] https://crrev.com/0d557dedf31c8cf30bee794ea983b5a16f899b0e/third_party/blink/renderer/bindings/tests/results/core/v8_test_object.cc
[modify] https://crrev.com/0d557dedf31c8cf30bee794ea983b5a16f899b0e/third_party/blink/renderer/bindings/tests/results/core/v8_test_typedefs.cc
[modify] https://crrev.com/0d557dedf31c8cf30bee794ea983b5a16f899b0e/third_party/blink/renderer/platform/bindings/callback_interface_base.cc
[modify] https://crrev.com/0d557dedf31c8cf30bee794ea983b5a16f899b0e/third_party/blink/renderer/platform/bindings/callback_interface_base.h

Comment 14 by yukishiino@chromium.org, Dec 12

Status: Fixed (was: Started)

Sign in to add a comment