Security: Crash with JavaScript RegExp subclassing
Reported by
peter.wm...@gmail.com,
Apr 12 2018
|
|||||||||||||||||||||||||||
Issue description
VULNERABILITY DETAILS
Executing the following JavaScript will cause a browser tab to crash.
```
class MyRegExp extends RegExp {
exec(str) {
const r = super.exec.call(this, str);
r[0] = 0; // Value could be changed to something arbitrary
return r;
}
}
'a'.match(new MyRegExp('.', 'g'));
```
I believe the issue is because a code assumption is being broken:
https://cs.chromium.org/chromium/src/v8/src/builtins/builtins-regexp-gen.cc?l=1878-1879&rcl=fc27a8d63c94c91ede18fce3246fbba7aec54262
`r[0]` is assumed to be string object because it passed the "BranchIfFastRegExpResult" verification. Unfortunately it isn't and I believe this ends up with an arbitrary load:
https://cs.chromium.org/chromium/src/v8/src/builtins/builtins-regexp-gen.cc?l=1879-1881&rcl=fc27a8d63c94c91ede18fce3246fbba7aec54262
https://cs.chromium.org/chromium/src/v8/src/builtins/builtins-regexp-gen.cc?l=1904&rcl=fc27a8d63c94c91ede18fce3246fbba7aec54262
https://cs.chromium.org/chromium/src/v8/src/builtins/builtins-regexp-gen.cc?l=1912&rcl=fc27a8d63c94c91ede18fce3246fbba7aec54262
https://cs.chromium.org/chromium/src/v8/src/code-stub-assembler.cc?l=1642-1647&rcl=fc27a8d63c94c91ede18fce3246fbba7aec54262
Although I don't think it's possible to get actual value of the load, you could detect whether it is a particular value or not (smi zero) depending on down stream side effects (regexp.lastIndex was incremented or not).
VERSION
Chrome Version: stable (65.0.3325.181), also verified on dev (67.0.3394.0) and latest version of Node.js (9.11.1)
Operating System: Mac OS X High Sierra 10.13.3
REPRODUCTION CASE
```
class MyRegExp extends RegExp {
exec(str) {
const r = super.exec.call(this, str);
r[0] = 0; // Value could be changed to something arbitrary
return r;
}
}
'a'.match(new MyRegExp('.', 'g'));
```
FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: tab
Crash ID: crash/9beff3f332dcbf6a
,
Apr 12 2018
Detailed report: https://clusterfuzz.com/testcase?key=4949311713181696 Job Type: linux_asan_chrome_mp Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x00000000000f Crash State: v8::internal::Invoke v8::internal::Execution::Call v8::Script::Run Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=523898:523900 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4949311713181696 See https://github.com/google/clusterfuzz-tools for more information.
,
Apr 12 2018
Automatically applying components based on crash stacktrace and information from OWNERS files. If this is incorrect, please apply the Test-Predator-Wrong-Components label.
,
Apr 12 2018
,
Apr 12 2018
Thanks, Peter already has a fix in flight here: https://crrev.com/c/1009325.
,
Apr 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/7bdbe77a3fa38ed8df0813323f116f0dff86c4f9 commit 7bdbe77a3fa38ed8df0813323f116f0dff86c4f9 Author: peterwmwong <peter.wm.wong@gmail.com> Date: Thu Apr 12 14:54:54 2018 [builtins] Fix missing ToString in RegExp.p.match It is not safe to assume the first match is a string just because the RegExp result is fast. Bug: chromium:831943 Change-Id: Idd40f8b75312f0be54f45f626dc017339033abc6 Reviewed-on: https://chromium-review.googlesource.com/1009325 Reviewed-by: Jakob Gruber <jgruber@chromium.org> Commit-Queue: Peter Wong <peter.wm.wong@gmail.com> Cr-Commit-Position: refs/heads/master@{#52578} [modify] https://crrev.com/7bdbe77a3fa38ed8df0813323f116f0dff86c4f9/src/builtins/builtins-regexp-gen.cc [add] https://crrev.com/7bdbe77a3fa38ed8df0813323f116f0dff86c4f9/test/mjsunit/regress/regress-crbug-831943.js
,
Apr 12 2018
,
Apr 12 2018
This bug requires manual review: We don't branch M67 until 2018-04-12. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 12 2018
This bug requires manual review: We are only 4 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 12 2018
#6 fixes a possible OOB read present since 57, originally introduced in: =====================ORIGINAL COMMIT START=================== commit 4e7571a5a9bb8563ae93e0aff48c08bead765b14 Author: jgruber <jgruber@chromium.org> Date: Tue Nov 29 01:02:48 2016 -0800 [regexp] Migrate @@match to TurboFan Microbenchmarks show a 4x improvement on the fast path and 2.5x improvement on the slow path when compared to the CPP builtin implementation. Compared to the old JS implementation, the fast path is 20% faster and the slow path 35% slower. BUG= v8:5339 , v8:5562 Review-Url: https://codereview.chromium.org/2527963002 Cr-Commit-Position: refs/heads/master@{#41338} =====================ORIGINAL COMMIT END===================== 2.) General information: Is LKGR: True Is on Canary: 2937 First V8 branch: 5.7.100 (Might not be the rolled version)
,
Apr 12 2018
which OS is this impacting?
,
Apr 12 2018
abdulsyed@, I've verified this crashes on Windows 10, MacOS X High Sierra and Android 8.1.0. As far as I can tell by the code, all operating systems would be impacted.
,
Apr 12 2018
Apply all OSs per comment #12.
,
Apr 13 2018
,
Apr 13 2018
+ awhalley@ - thoughts on taking this for M66 vs waiting until 67 from a security perspective?
,
Apr 13 2018
I'm OK waiting to 67
,
Apr 13 2018
,
Apr 13 2018
awhalley@, pls do M67 merge review. Thank you.
,
Apr 13 2018
good for 67
,
Apr 13 2018
Approving merge to M67 branch 3396 based on comment #19. Please merge ASAP. Thank you.
,
Apr 15 2018
Pls merge your change to M67 branch 3396 by 1:00 PM PT Monday (04/16/18) so we can pick it up for next M67 dev release. Thank you.
,
Apr 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/9a443d2aea5764e41fbf8417ad3885da923f38ae commit 9a443d2aea5764e41fbf8417ad3885da923f38ae Author: peterwmwong <peter.wm.wong@gmail.com> Date: Mon Apr 16 06:52:05 2018 Merged: [builtins] Fix missing ToString in RegExp.p.match It is not safe to assume the first match is a string just because the RegExp result is fast. NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true Bug: chromium:831943 Change-Id: Idd40f8b75312f0be54f45f626dc017339033abc6 Reviewed-on: https://chromium-review.googlesource.com/1009325 Reviewed-by: Jakob Gruber <jgruber@chromium.org> Commit-Queue: Peter Wong <peter.wm.wong@gmail.com> Cr-Original-Commit-Position: refs/heads/master@{#52578}(cherry picked from commit 7bdbe77a3fa38ed8df0813323f116f0dff86c4f9) Reviewed-on: https://chromium-review.googlesource.com/1013578 Commit-Queue: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/branch-heads/6.7@{#12} Cr-Branched-From: 8457e810efd34381448d51d93f50079cf1f6a812-refs/heads/6.7.288@{#2} Cr-Branched-From: e921be5c4f2c6407936bde750992dedbf47c1016-refs/heads/master@{#52547} [modify] https://crrev.com/9a443d2aea5764e41fbf8417ad3885da923f38ae/src/builtins/builtins-regexp-gen.cc [add] https://crrev.com/9a443d2aea5764e41fbf8417ad3885da923f38ae/test/mjsunit/regress/regress-crbug-831943.js
,
Apr 16 2018
Removing Merge-Review-66 as per #16, thanks everyone.
,
Apr 16 2018
Per comment #22, this is already merged to M67. If nothing is pending, pls remove "Merge-Approved-67" label. Thank you.
,
Apr 16 2018
,
Apr 16 2018
,
Apr 18 2018
,
Apr 18 2018
ClusterFuzz has detected this issue as fixed in range 550886:550887. Detailed report: https://clusterfuzz.com/testcase?key=4949311713181696 Job Type: linux_asan_chrome_mp Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x00000000000f Crash State: v8::internal::Invoke v8::internal::Execution::Call v8::Script::Run Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=523898:523900 Fixed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_mp&range=550886:550887 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4949311713181696 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.
,
Apr 18 2018
ClusterFuzz testcase 4949311713181696 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Apr 20 2018
*** Boilerplate reminders! *** Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing. *********************************
,
Apr 20 2018
Many thanks, peter.wm.wong@! The Chrome VRP panel decided to award $1,000 for the bug report, and a $500 bonus because you supplied the fix. A member of our finance team will be in touch to arrange payment. Also, how would you like to be credited in Chrome release notes? Cheers!
,
Apr 20 2018
,
Apr 21 2018
awhalley@ Wow! You can credit me as "Peter Wong". Thanks!
,
May 18 2018
CC'ing Zuojian from UC Browser
,
May 29 2018
,
May 29 2018
,
Jul 20
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
,
Jul 28
|
|||||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||||
Comment 1 by ClusterFuzz
, Apr 12 2018