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

Issue 624300 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

peek_any_identifier() || peek() == Token::LPAREN in parser.cc

Project Member Reported by ClusterFuzz, Jun 29 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4986045389340672

Fuzzer: mbarbella_js_mutation
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  peek_any_identifier() || peek() == Token::LPAREN in parser.cc
  
Regressed: V8: r36638:36639

Minimized Testcase (0.10 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94rE2MyoPIi5VzGugXEUMp0aN5FkJ_LEZwDZpKBEJjgjLKypzDTpsA9eiSLbKP7GyWZliI-BIaucvmZqZ6nhZN-dVT1FoRe2eCzb4TIqKkwHknwcUAeO223GkpjsZFmVtc7OF7bpJhho0Ye0Qz0J9CSj-XZdQ?testcase_id=4986045389340672
;
(function __f_15() {
  try {
    __f_15();
  } catch (e) {
 (async() => await 1).length;
  }
})();


Filer: mstarzinger

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Owner: littledan@chromium.org
Status: Assigned (was: Available)
Looks related to async functions. Regression range points to: https://chromium.googlesource.com/v8/v8/+/88ab533b32c79a2bf5d82dd3de2b48c41b0fa029
Cc: caitpott...@gmail.com
The issue here is that a stack overflow is causing the LPAREN to be returned as ILLEGAL (evan though it was already scanned via PeekAhead()).

The stack overflow looks correct since the __f_15() recursion is called __many__ times, before the arrow function's AST is built.

Should ParseLazy be aware of the stack overflow issue, perhaps?
Yeah, it seems like we should bail out in stack overflow cases. I put my take on a patch at https://codereview.chromium.org/2135503002 , but I am worried that there are more places in the parser that need to be updated.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 11 2016

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

commit 77cbe27689d3c8823ea492cbc33f7718a4b5e417
Author: littledan <littledan@chromium.org>
Date: Mon Jul 11 19:28:56 2016

Narrowly address async function stack overflow parsing case

This patch just checks for a stack overflow and returns failure
from the cases which Clusterfuzz found. However, there may be
more locations in the parser which need similar treatment.

R=caitpotter88@gmail.com,neis
BUG= v8:4483 , chromium:624300 

Review-Url: https://codereview.chromium.org/2135503002
Cr-Commit-Position: refs/heads/master@{#37655}

[modify] https://crrev.com/77cbe27689d3c8823ea492cbc33f7718a4b5e417/src/parsing/parser.cc
[add] https://crrev.com/77cbe27689d3c8823ea492cbc33f7718a4b5e417/test/mjsunit/harmony/regress/regress-624300.js

This particular bug is fixed, but we need to think more generally about how to make sure that it doesn't come up in other cases.
Project Member

Comment 7 by ClusterFuzz, Jul 12 2016

ClusterFuzz has detected this issue as fixed in range 37654:37655.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4986045389340672

Fuzzer: mbarbella_js_mutation
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  peek_any_identifier() || peek() == Token::LPAREN in parser.cc
  
Regressed: V8: r36638:36639
Fixed: V8: r37654:37655

Minimized Testcase (0.10 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94rE2MyoPIi5VzGugXEUMp0aN5FkJ_LEZwDZpKBEJjgjLKypzDTpsA9eiSLbKP7GyWZliI-BIaucvmZqZ6nhZN-dVT1FoRe2eCzb4TIqKkwHknwcUAeO223GkpjsZFmVtc7OF7bpJhho0Ye0Qz0J9CSj-XZdQ?testcase_id=4986045389340672
;
(function __f_15() {
  try {
    __f_15();
  } catch (e) {
 (async() => await 1).length;
  }
})();


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs 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 8 by ClusterFuzz, Jul 12 2016

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

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

Comment 9 by sheriffbot@chromium.org, Nov 22 2016

Labels: -Restrict-View-EditIssue
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

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

Sign in to add a comment