New issue
Advanced search Search tips

Issue 836141 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 0
Type: Bug-Security

Blocking:
issue 835887



Sign in to add a comment

Null-dereference READ in v8::internal::wasm::InstantiateToInstanceObject

Project Member Reported by ClusterFuzz, Apr 24

Issue description

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

Job Type: linux_asan_chrome_mp
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  v8::internal::wasm::InstantiateToInstanceObject
  v8::WebAssemblyInstantiateImpl
  v8::WebAssemblyInstantiateImplCallback
  
Sanitizer: address (ASAN)

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

Issue manually filed by: clemensh

See https://github.com/google/clusterfuzz-tools for more information.
 
Components: Blink>JavaScript>WebAssembly
Labels: -Type-Bug Restrict-View-SecurityTeam Type-Bug-Security
Owner: ahaas@chromium.org
Status: Assigned (was: Untriaged)
Labels: Security_Severity-Critical Security_Impact-Stable
Blocking: 835887
Cc: mstarzinger@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 5 by ClusterFuzz, Apr 24

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=6290771918192640.
Project Member

Comment 6 by ClusterFuzz, Apr 24

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

Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: Ill
Crash Address: 0x7fb213146f18
Crash State:
  v8::Utils::ReportApiFailure
  ApiCheck
  v8::Object::CheckCast
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=45141:45142

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

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

Comment 8 by bugdroid1@chromium.org, Apr 24

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/49712d8acf1b16d8d36a6f95ae7e97210a559a45

commit 49712d8acf1b16d8d36a6f95ae7e97210a559a45
Author: Andreas Haas <ahaas@chromium.org>
Date: Tue Apr 24 13:01:18 2018

[wasm] Call AsyncInstantiate directly when instantiating a module object

WebAssembly.instantiate is polymorphic, it can either take a module
object as parameter, or a buffer source which should be compiled first.
To share code between the two implementations, the module object was
first passed to a promise (i.e. which is the result of compilation).
However, passing the module object to a promise has a side effect if
the module object has a then function. To avoid this side effect I
remove this code sharing and call AsyncInstantiate directly in case
the parameter is a module object.

R=mstarzinger@chromium.org

Bug:  chromium:836141 
Change-Id: I67b76d0d7761c5aeb2cf1deda45b6842e494eed4
Reviewed-on: https://chromium-review.googlesource.com/1025774
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52755}
[modify] https://crrev.com/49712d8acf1b16d8d36a6f95ae7e97210a559a45/src/wasm/wasm-js.cc
[add] https://crrev.com/49712d8acf1b16d8d36a6f95ae7e97210a559a45/test/mjsunit/regress/wasm/regress-836141.js

Project Member

Comment 9 by sheriffbot@chromium.org, Apr 24

Labels: ReleaseBlock-Beta
This is a critical security issue. 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 10 by sheriffbot@chromium.org, Apr 24

Labels: -Pri-1 Pri-0
Cc: awhalley@chromium.org
Labels: M-68 M-67
We're planning to promote current M67 Dev build #67.0.3396.18 to M67 Beta on Thursday, 04/26. Will this be a M67 Beta blocker for first promotion?
Is this only applicable to Linux? If not, pls add all applicable OSs.
Labels: -Security_Severity-Critical Security_Severity-High
Labels: OS-Android OS-Chrome OS-Fuchsia OS-Mac OS-Windows
 Issue 836421  has been merged into this issue.
Status: Fixed (was: Started)
Project Member

Comment 18 by ClusterFuzz, Apr 25

ClusterFuzz has detected this issue as fixed in range 52754:52755.

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

Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: Ill
Crash Address: 0x7fb213146f18
Crash State:
  v8::Utils::ReportApiFailure
  ApiCheck
  v8::Object::CheckCast
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=45141:45142
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=52754:52755

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

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 19 by ClusterFuzz, Apr 25

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: Merge-Request-66 Merge-Request-67
The fix is in today's canary (68.0.3406.0) and as far as I can tell there are no related crashes.
Labels: -Merge-Request-67 Merge-Approved-67
Labels: -Merge-Request-66 Merge-Approved-66
Project Member

Comment 23 by bugdroid1@chromium.org, Apr 25

Labels: merge-merged-6.6
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/6324c5a8d5d1633d0171e504aaa200fc13cdb46d

commit 6324c5a8d5d1633d0171e504aaa200fc13cdb46d
Author: Andreas Haas <ahaas@chromium.org>
Date: Wed Apr 25 14:19:43 2018

Merged: [wasm] Call AsyncInstantiate directly when instantiating a module object

WebAssembly.instantiate is polymorphic, it can either take a module
object as parameter, or a buffer source which should be compiled first.
To share code between the two implementations, the module object was
first passed to a promise (i.e. which is the result of compilation).
However, passing the module object to a promise has a side effect if
the module object has a then function. To avoid this side effect I
remove this code sharing and call AsyncInstantiate directly in case
the parameter is a module object.

NOTRY=true
NOPRESUBMIT=true
NOTREECHECK=true

R=​mstarzinger@chromium.org

Bug:  chromium:836141 
Change-Id: I67b76d0d7761c5aeb2cf1deda45b6842e494eed4
Reviewed-on: https://chromium-review.googlesource.com/1025774
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#52755}(cherry picked from commit 49712d8acf1b16d8d36a6f95ae7e97210a559a45)
Reviewed-on: https://chromium-review.googlesource.com/1027875
Cr-Commit-Position: refs/branch-heads/6.6@{#49}
Cr-Branched-From: d500271571b92cb18dcd7b15885b51e8f437d640-refs/heads/6.6.346@{#1}
Cr-Branched-From: 265ef0b635f8761df7c89eb4e8ec9c1a6ebee184-refs/heads/master@{#51624}
[modify] https://crrev.com/6324c5a8d5d1633d0171e504aaa200fc13cdb46d/src/wasm/wasm-js.cc
[add] https://crrev.com/6324c5a8d5d1633d0171e504aaa200fc13cdb46d/test/mjsunit/regress/wasm/regress-836141.js

Labels: -Merge-Approved-66
Project Member

Comment 25 by bugdroid1@chromium.org, Apr 25

Labels: merge-merged-6.7
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/f45bbb86ef56073b25e500f21030ebc9f12f96ad

commit f45bbb86ef56073b25e500f21030ebc9f12f96ad
Author: Andreas Haas <ahaas@chromium.org>
Date: Wed Apr 25 14:23:53 2018

Merged: [wasm] Call AsyncInstantiate directly when instantiating a module object

WebAssembly.instantiate is polymorphic, it can either take a module
object as parameter, or a buffer source which should be compiled first.
To share code between the two implementations, the module object was
first passed to a promise (i.e. which is the result of compilation).
However, passing the module object to a promise has a side effect if
the module object has a then function. To avoid this side effect I
remove this code sharing and call AsyncInstantiate directly in case
the parameter is a module object.

R=​mstarzinger@chromium.org

NOTRY=true
NOPRESUBMIT=true
NOTREECHECK=true

Bug:  chromium:836141 
Change-Id: I67b76d0d7761c5aeb2cf1deda45b6842e494eed4
Reviewed-on: https://chromium-review.googlesource.com/1025774
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#52755}(cherry picked from commit 49712d8acf1b16d8d36a6f95ae7e97210a559a45)
Reviewed-on: https://chromium-review.googlesource.com/1027876
Cr-Commit-Position: refs/branch-heads/6.7@{#34}
Cr-Branched-From: 8457e810efd34381448d51d93f50079cf1f6a812-refs/heads/6.7.288@{#2}
Cr-Branched-From: e921be5c4f2c6407936bde750992dedbf47c1016-refs/heads/master@{#52547}
[modify] https://crrev.com/f45bbb86ef56073b25e500f21030ebc9f12f96ad/src/wasm/wasm-js.cc
[add] https://crrev.com/f45bbb86ef56073b25e500f21030ebc9f12f96ad/test/mjsunit/regress/wasm/regress-836141.js

Project Member

Comment 26 by sheriffbot@chromium.org, Apr 25

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -Merge-Approved-67
Labels: -ReleaseBlock-Beta
ahaas@: Note that the reporter might have spotted another case of this bug in https://bugs.chromium.org/p/chromium/issues/detail?id=835887#c21.  Do you want to track that here or in another bug?
I'd prefer another bug to make tracking merges easier
Ack. Clusterfuzz should open a new bug once it finishes analysis here: https://clusterfuzz.com/v2/testcase-detail/5305166916747264
I saw the comment and will open a new issue.
Labels: Release-1-M66
Labels: CVE-2018-6122 CVE_description-missing
Labels: NodeJS-Backport-Approved
Affects Node 8, so this will need to be floated on the Node.js side.
Labels: -NodeJS-Backport-Approved NodeJS-Backport-Done
Node.js 8.x PR: https://github.com/nodejs/node/pull/21334
Labels: reward-topanel
(reward-topanel as part of considering issue 835887)
Labels: -reward-topanel
Project Member

Comment 39 by sheriffbot@chromium.org, Aug 1

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