Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/04f7d484db22b1526afa5414c06eda443c5b4fad (RegExp: Add the ability to switch flags on and off within the regexp.).

If this is incorrect, please remove the owner and apply the Test-Predator-Wrong-CLs label.

This is a serious security regression. 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

The bot hasn't identified the correct CL that introduces this bug.  It appears with the change that activates unicode regexp properties, 2feb99dc8ac75f20d2e5c9c1b343e923476851ea from November 9.

My change is 04f7d484db22b1526afa5414c06eda443c5b4fad from November 8, but even before my change it can be triggered if you use --harmony-regexp-property.

Repro:

var re = new RegExp("\0\\1(\\P{P\0[}()/", "u");
re.test("?x>>>>");

I suspect the problem is that the regexp-preparser gets confused about the \P properties.

This one is quite interesting as there's interaction between the general (non-regexp) parser, the new regexp unicode properties feature, and the capture group mini-parser in RegExpParser::ScanForCaptures.

Initial investigation of the repro in #6:

//          a  bc   d    e
new RegExp("\\1(\\P{P\0[}()/", "u");

1. At position a, the '\1' backreference triggers the RegExpParser::ScanForCaptures.
2. b opens a new capture group.
3. c starts the '\P{property}' syntax for unicode property escapes.
4. d begins the property name, which terminates with the closing bracket '}'.
5. e opens and closes the second capture group.

Parsing the property name (steps 3 and 4) accept 'P\0['. We pass a null-terminated string to ICU during name lookup, so ICU sees 'P' which is a valid abbreviation for 'Punctuation'. No validation happens on anything after the '\0', in particular the '[' char.

But RegExpParser::ScanForCaptures (step 1) sees '[' as starting a character class, and basically ignores everything that follows since the supposed class is never closed. It therefore sets `capture_count_` to 1.

In step e, we finally fail during `RegExpParser::GetCapture(2)` since we believe there's only one capture.

A final note is that this fails differently when the regexp is created as a literal: 

/\1(\P{P\0[}()/u.test("")  // SyntaxError: Invalid regular expression: missing /


Restricting property names (as per the grammar in [0]) should fix this.

[0] https://tc39.github.io/proposal-regexp-unicode-property-escapes/.

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

commit 0da56e74cf8ab9317b4e6e0a0abe0593de5bc57c
Author: jgruber <jgruber@chromium.org>
Date: Fri Dec 15 14:17:34 2017

[regexp] Restrict unicode property value expressions

The unicode property escape syntax restricts unicode property names and
unicode property values to consist only of characters taken from the
[a-zA-Z0-9_] character class. See the spec at:

https://tc39.github.io/proposal-regexp-unicode-property-escapes/

In most cases, we do not actually need to validate that this is the
case, since subsequent property lookup in ICU will fail (and throw a
SyntaxError) if the given property does not exist.

However, there one special case. The ICU lookup takes the property name
as a null-terminated string, so it will accept carefully malformed
property names (e.g. '\p{Number\0[}'). This can end up confusing the
regexp parser.

With this CL, we explicitly restrict potential property names / values
to the character set as specified.

Bug:  v8:4743 ,  chromium:793793 
Change-Id: Ic97deea8602571ec6793b79c4bb858e1c7597405
Reviewed-on: https://chromium-review.googlesource.com/824272
Reviewed-by: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50130}
[modify] https://crrev.com/0da56e74cf8ab9317b4e6e0a0abe0593de5bc57c/src/regexp/regexp-parser.cc
[add] https://crrev.com/0da56e74cf8ab9317b4e6e0a0abe0593de5bc57c/test/mjsunit/regress/regress-793793.js

ClusterFuzz has detected this issue as fixed in range 524430:524448.

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

Fuzzer: libFuzzer_v8_regexp_parser_fuzzer
Job Type: libfuzzer_chrome_asan
Platform Id: linux

Crash Type: Use-after-poison READ 8
Crash Address: 0x6250000169f8
Crash State:
  v8::internal::RegExpParser::GetCapture
  v8::internal::RegExpParser::ParseDisjunction
  v8::internal::RegExpParser::ParsePattern
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan&range=515946:515969
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan&range=524430:524448

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

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 4745972801601536 is verified as fixed, so closing issue as verified.

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

This bug requires manual review: M64 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), kbleicher@(ChromeOS), abdulsyed@(Desktop)

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

+hablich for V8 review.

abdulsyed@ - good for 64

Approving merge for M64. Branch:3282

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

commit 97b48a07e3cb603a46bcdbea089ef5bbbea6e90e
Author: jgruber <jgruber@chromium.org>
Date: Mon Jan 08 09:45:35 2018

Merged: [regexp] Restrict unicode property value expressions

The unicode property escape syntax restricts unicode property names and
unicode property values to consist only of characters taken from the
[a-zA-Z0-9_] character class. See the spec at:

https://tc39.github.io/proposal-regexp-unicode-property-escapes/

In most cases, we do not actually need to validate that this is the
case, since subsequent property lookup in ICU will fail (and throw a
SyntaxError) if the given property does not exist.

However, there one special case. The ICU lookup takes the property name
as a null-terminated string, so it will accept carefully malformed
property names (e.g. '\p{Number\0[}'). This can end up confusing the
regexp parser.

With this CL, we explicitly restrict potential property names / values
to the character set as specified.

NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
TBR=hablich@chromium.org

Bug:  v8:4743 ,  chromium:793793 
Change-Id: Ic97deea8602571ec6793b79c4bb858e1c7597405
Reviewed-on: https://chromium-review.googlesource.com/824272
Reviewed-by: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#50130}
Reviewed-on: https://chromium-review.googlesource.com/853860
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.4@{#41}
Cr-Branched-From: 0407506af3d9d7e2718be1d8759296165b218fcf-refs/heads/6.4.388@{#1}
Cr-Branched-From: a5fc4e085ee543cb608eb11034bc8f147ba388e1-refs/heads/master@{#49724}
[modify] https://crrev.com/97b48a07e3cb603a46bcdbea089ef5bbbea6e90e/src/regexp/regexp-parser.cc
[add] https://crrev.com/97b48a07e3cb603a46bcdbea089ef5bbbea6e90e/test/mjsunit/regress/regress-793793.js

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