New issue
Advanced search Search tips

Issue 727218 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security



Sign in to add a comment

CHECK failure: is_resolved() in ast.h

Project Member Reported by ClusterFuzz, May 29 2017

Issue description

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

Fuzzer: inferno_js_fuzzer
Job Type: linux_asan_d8_v8_arm64_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  is_resolved() in ast.h
  var
  VisitVariableProxyReference
  
Sanitizer: address (ASAN)

Regressed: V8: 44392:44393

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


Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 

Comment 1 by ishell@chromium.org, May 29 2017

Cc: vogelheim@chromium.org adamk@chromium.org marja@chromium.org
Owner: caitp@chromium.org
Status: Assigned (was: Untriaged)
CF points to 5f782db9541eca72dc6cbe3c6f5e8dc52ff7c16f.


Comment 2 by ishell@chromium.org, May 29 2017

 Issue 727221  has been merged into this issue.

Comment 3 by ishell@chromium.org, May 29 2017

Repro: 

out.gn/x64.debug/d8 --predictable --no-turbo test.js

===== test.js =====

var __v_5 = ({ x } = { x: __v_3 }) => {
  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
  x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;
};

Comment 4 by marja@chromium.org, May 29 2017

Maybe related to aborting the preparsing of arrow funcs. Maybe we should just get rid of that feature... :/ it's really tricky to get right.

Comment 5 by marja@chromium.org, May 29 2017

It is. Here's a fix proposal: https://chromium-review.googlesource.com/c/518145/

Wdyt?
Project Member

Comment 6 by sheriffbot@chromium.org, May 29 2017

Labels: M-60
Project Member

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

Labels: ReleaseBlock-Beta
This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

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

Comment 8 by sheriffbot@chromium.org, May 29 2017

Labels: Pri-1

Comment 9 by marja@chromium.org, May 29 2017

Can't revert the change that introduced it, since that was a bugfix to another security bug :(

So I'm now even more strongly suggesting the simple fix above.
Project Member

Comment 10 by bugdroid1@chromium.org, May 30 2017

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

commit 36de9199f4093585133cf817405c6bfc7dcfb730
Author: Marja Hölttä <marja@chromium.org>
Date: Tue May 30 14:00:54 2017

[parser] Disable aborting preparsing for arrow functions.

It's extremely difficult to get right: there have been several bugs
related to this feature, especially when combined with
non-simple parameter lists in arrow functions.

BUG= chromium:727218 

Change-Id: I97dfbc57a7650199964c5fe99de69143c8e537c2
Reviewed-on: https://chromium-review.googlesource.com/518145
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45603}
[modify] https://crrev.com/36de9199f4093585133cf817405c6bfc7dcfb730/src/parsing/parser-base.h
[add] https://crrev.com/36de9199f4093585133cf817405c6bfc7dcfb730/test/mjsunit/regress/regress-727218.js

Comment 11 by marja@chromium.org, May 31 2017

Cc: caitp@chromium.org
Owner: marja@chromium.org
Status: Fixed (was: Assigned)
The commit that broke this is from Apr 4, shouldn't this be merged to M59 too? The labels only say M60... Hmm...
Project Member

Comment 12 by sheriffbot@chromium.org, May 31 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: Merge-Request-60
Project Member

Comment 14 by sheriffbot@chromium.org, Jun 6 2017

Labels: -Merge-Request-60 Hotlist-Merge-Approved Merge-Approved-60
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

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

Comment 17 by sheriffbot@chromium.org, Jun 12 2017

Cc: awhalley@chromium.org
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 18 by adamk@chromium.org, Jun 12 2017

Labels: -Merge-Approved-60 Merge-Merged-60
Already merged as noted in #16
Project Member

Comment 19 by sheriffbot@chromium.org, Sep 6 2017

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