New issue
Advanced search Search tips

Issue 797481 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security

Blocked on:
issue v8:7239



Sign in to add a comment

Crash in v8::internal::Simulator::LoadStorePairHelper

Project Member Reported by ClusterFuzz, Dec 23 2017

Issue description

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

Fuzzer: mbarbella_js_mutation
Job Type: linux_msan_d8
Platform Id: linux

Crash Type: UNKNOWN WRITE
Crash Address: 0x7fee9c875fd0
Crash State:
  v8::internal::Simulator::LoadStorePairHelper
  v8::internal::Simulator::Run
  v8::internal::Simulator::CallVoid
  
Sanitizer: memory (MSAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=linux_msan_d8&range=49558:49559

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

Issue filed automatically.

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

Comment 1 by sheriffbot@chromium.org, Dec 23 2017

Labels: Pri-1
Reduced test case:
a = /x/;
b = /y/;
b.exec = a.test;
a.test.call(b, 'foo');

Bisecting...
Cc: yangguo@chromium.org
Components: -Blink>JavaScript Blink>JavaScript>Regexp
Labels: M-62
Owner: jgruber@chromium.org
Status: Assigned (was: Untriaged)
Seems to be a really old bug. Bisects to ae3357d216ed3ce82deed2ad2c6bb526370ef6aa ("[regexp] Move RegExp.prototype.test to TF"), from October 2016.

I confirmed that it also crashes in stable (M62).
Project Member

Comment 4 by sheriffbot@chromium.org, Dec 28 2017

Labels: -M-62 Security_Impact-Stable M-63
Project Member

Comment 5 by sheriffbot@chromium.org, Jan 6 2018

jgruber: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

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
Thanks! Even smaller repro:

const a = /x/;
a.exec = RegExp.prototype.test;
RegExp.prototype.test.call(a);
Blockedon: v8:7239
This is another instance of https://crbug.com/v8/7239, in which infinite recursion ends up overflowing the stack. TFJ builtins do not have stack checks by default and thus they simply overflow and segfault instead of throwing a RangeError.

The quick fix is to add a PerformStackCheck call, but we should really add stack checks to all TFJ builtins by default.
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 11 2018

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

commit e1f676ec9937af14c120f4414158f222bb1212b9
Author: jgruber <jgruber@chromium.org>
Date: Thu Jan 11 15:39:34 2018

[regexp] Add stack check to RegExpExec

Band-aid fix for infinite recursion in RegExp TFJ builtins.

TFJ builtins don't contain stack checks in general, so any deep
recursion involving only TFJ builtins can end up overflowing the stack
and segfaulting on the red area.

RegExp builtins in particular can only build such recursions using
RegExp.p.exec, and (as far as I can tell) only by modifying the instance
or prototype, thus hitting the slow path in all builtins.

This CL adds a stack check to RegExpExec, which is the choke point for
calling exec on slow-mode RegExps.

Bug: v8:7239,  chromium:797481 

Regression test

Change-Id: I78dbb5f868a775d9697606d513623f912639d7db
Reviewed-on: https://chromium-review.googlesource.com/856777
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50511}
[modify] https://crrev.com/e1f676ec9937af14c120f4414158f222bb1212b9/src/builtins/builtins-regexp-gen.cc
[add] https://crrev.com/e1f676ec9937af14c120f4414158f222bb1212b9/test/mjsunit/regress/regress-797481.js

Status: Fixed (was: Assigned)
Project Member

Comment 10 by ClusterFuzz, Jan 12 2018

ClusterFuzz has detected this issue as fixed in range 50510:50511.

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

Fuzzer: mbarbella_js_mutation
Job Type: linux_msan_d8
Platform Id: linux

Crash Type: UNKNOWN WRITE
Crash Address: 0x7fee9c875fd0
Crash State:
  v8::internal::Simulator::LoadStorePairHelper
  v8::internal::Simulator::Run
  v8::internal::Simulator::CallVoid
  
Sanitizer: memory (MSAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=linux_msan_d8&range=49558:49559
Fixed: https://clusterfuzz.com/revisions?job=linux_msan_d8&range=50510:50511

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

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 11 by ClusterFuzz, Jan 12 2018

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

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

Comment 12 by sheriffbot@chromium.org, Jan 12 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -M-63 M-65
Project Member

Comment 14 by sheriffbot@chromium.org, Feb 8 2018

Labels: Merge-Request-65
Project Member

Comment 15 by sheriffbot@chromium.org, Feb 9 2018

Labels: -Merge-Request-65 Merge-Review-65 Hotlist-Merge-Review
This bug requires manual review: M65 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: awhalley@chromium.org
[Bulk Edit]

+awhalley@ (Security TPM) for M65 merge review
govind@ - good for 65
Labels: -Merge-Review-65 Merge-Approved-65
Approving merge to M65 branch 3325 based on comment #17. Please merge ASAP so we can pick it up for next week Beta release. Thank you.
Labels: -Merge-Approved-65
AFAIK the fix in #8 is already in 65, no backmerge needed:

$ tools/release/mergeinfo.py e1f676ec9937af14c120f4414158f222bb1212b9
Is on Canary:    3319
First V8 branch: 6.5.182 (Might not be the rolled version)
Labels: Release-0-M65
Project Member

Comment 21 by sheriffbot@chromium.org, Apr 20 2018

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