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

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Security

Blocked on:
issue v8:4923



Sign in to add a comment
link

Issue 604897: Compiled regexps execute incorrectly on function source strings

Reported by max.kore...@gmail.com, Apr 19 2016

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.75 Safari/537.36

Steps to reproduce the problem:
See the result of the following code snippet (or attached screenshots):
http://plnkr.co/edit/RFXffpqJRg3L6xoCAbIY?p=preview

What is the expected behavior?
Matching results should be the same

What went wrong?
Calling identical .match() second time results in "null"

Did this work before? Yes Chrome 49

Chrome version: 50.0.2661.75  Channel: stable
OS Version: 10.0
Flash Version: Shockwave Flash 21.0 r0

This regex code is used inside angular.js to extract function parameters for dependency injection.
This bug is causing a crash ("Aw, Snap!") on the real angular project. 
(see Crash ID 97878f1200000000 (d9c92400-5011-485c-a164-aa2544110381) )
 
Chrome_50.png
132 KB View Download
Chrome_49.png
158 KB View Download

Comment 1 by dpranke@chromium.org, Apr 19 2016

Components: -Blink Blink>JavaScript
Status: Untriaged (was: Unconfirmed)
Yup.

Comment 2 by hablich@chromium.org, Apr 20 2016

Cc: adamk@chromium.org yangguo@chromium.org littledan@chromium.org
Components: -Blink>JavaScript Blink>JavaScript>Language
Status: Available (was: Untriaged)
Maybe this is because of the RegExp customization hooks we shipped in M50?

Comment 3 by hablich@chromium.org, Apr 20 2016

Labels: -Pri-2 Pri-1

Comment 4 by yangguo@chromium.org, Apr 20 2016

I can reproduce this with 50.0.2661.75 on the given test site.
However, if I copy the code into the console, it works just fine.

Comment 5 by yangguo@chromium.org, Apr 20 2016

Cc: -yangguo@chromium.org
Owner: yangguo@chromium.org
I have a strong suspicion that this is the same bug as v8:4923.

Comment 6 by yangguo@chromium.org, Apr 20 2016

Reduced repro. Also just confirmed that the fix for v8:4923 fixes this issue as well.

https://bugs.chromium.org/p/v8/issues/detail?id=4923

Comment 7 by yangguo@chromium.org, Apr 20 2016

Cc: yangguo@chromium.org
 Issue 604797  has been merged into this issue.

Comment 8 by bugdroid1@chromium.org, Apr 20 2016

Project Member

Comment 9 by yangguo@chromium.org, Apr 20 2016

Labels: Merge-Request-51 Merge-Request-50
Summary: Compiled regexps execute incorrectly on function source strings (was: arrow function toString() + regex bug)
This needs to be merged as soon as possible, once we have a mips64 port for the fix.

Comment 10 by hablich@chromium.org, Apr 20 2016

Blockedon: v8:4923

Comment 11 by hablich@chromium.org, Apr 20 2016

Labels: -Type-Bug Type-Bug-Security

Comment 12 by hablich@chromium.org, Apr 20 2016

Cc: timwillis@chromium.org infe...@chromium.org
Labels: Restrict-View-SecurityTeam

Comment 13 by vakh@chromium.org, Apr 20 2016

Status: Assigned (was: Available)

Comment 14 by vakh@chromium.org, Apr 20 2016

Labels: Security_Impact-Stable

Comment 15 by vakh@chromium.org, Apr 20 2016

Labels: Security_Severity-High

Comment 16 by bugdroid1@chromium.org, Apr 20 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/7d8e279a7b11cb3c1cbeb424118a20210f9b8eea

commit 7d8e279a7b11cb3c1cbeb424118a20210f9b8eea
Author: bjaideep <bjaideep@ca.ibm.com>
Date: Wed Apr 20 16:52:36 2016

PPC: [regexp] do not assume short external strings have a minimum size.

Port 3518e492c0939759ae1a2623bbd606646ee172f1

Original commit message:

    Short external strings do not cache the resource data, and may be used
    for compressible strings. The assumptions about their lengths is
    invalid and may lead to oob reads.

R=yangguo@chromium.org, joransiu@ca.ibm.com, jyan@ca.ibm.com, michael_dawson@ca.ibm.com, mbrandy@us.ibm.com

BUG=v8:4923, chromium:604897 
LOG=N

Review URL: https://codereview.chromium.org/1901593005

Cr-Commit-Position: refs/heads/master@{#35671}

[modify] https://crrev.com/7d8e279a7b11cb3c1cbeb424118a20210f9b8eea/src/ppc/code-stubs-ppc.cc

Comment 17 by ClusterFuzz, Apr 20 2016

Project Member
Status: Fixed (was: Assigned)
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges.

- Your friendly ClusterFuzz

Comment 18 by bugdroid1@chromium.org, Apr 21 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/14c9cbd4cfaa235801e0372178e8af039cb2233f

commit 14c9cbd4cfaa235801e0372178e8af039cb2233f
Author: jyan <jyan@ca.ibm.com>
Date: Thu Apr 21 05:02:43 2016

S390: [regexp] do not assume short external strings have a minimum size.

Port 3518e492c0939759ae1a2623bbd606646ee172f1

Original commit message:

    Short external strings do not cache the resource data, and may be used
    for compressible strings. The assumptions about their lengths is
    invalid and may lead to oob reads.

R=yangguo@chromium.org, joransiu@ca.ibm.com, bjaideep@ca.ibm.com, michael_dawson@ca.ibm.com, mbrandy@us.ibm.com

BUG=v8:4923, chromium:604897 
LOG=N

Review URL: https://codereview.chromium.org/1911633002

Cr-Commit-Position: refs/heads/master@{#35682}

[modify] https://crrev.com/14c9cbd4cfaa235801e0372178e8af039cb2233f/src/s390/code-stubs-s390.cc

Comment 19 by bugdroid1@chromium.org, Apr 21 2016

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

commit b4697727e946e8d96d5345a4db7ff72842d60466
Author: yangguo <yangguo@chromium.org>
Date: Thu Apr 21 05:58:08 2016

MIPS64: [regexp] do not assume short external strings have a minimum size.

Port 3518e492c0939759ae1a2623bbd606646ee172f1

Original commit message:
    Short external strings do not cache the resource data, and may be used
    for compressible strings. The assumptions about their lengths is
    invalid and may lead to oob reads.

R=bmeurer@chromium.org
BUG=v8:4923, chromium:604897 
LOG=N

Review URL: https://codereview.chromium.org/1902393004

Cr-Commit-Position: refs/heads/master@{#35683}

[modify] https://crrev.com/b4697727e946e8d96d5345a4db7ff72842d60466/src/mips64/code-stubs-mips64.cc
[modify] https://crrev.com/b4697727e946e8d96d5345a4db7ff72842d60466/test/cctest/cctest.status

Comment 20 by yangguo@chromium.org, Apr 21 2016

Patches to back merge:

3518e492c0939759ae1a2623bbd606646ee172f1 original patch (ia32, x64, arm, arm64, mips ports)
7d8e279a7b11cb3c1cbeb424118a20210f9b8eea ppc port
644bade748fafb1f2e8ab25ca33473a4c77c006d x87 port
b4697727e946e8d96d5345a4db7ff72842d60466 mips64 port
14c9cbd4cfaa235801e0372178e8af039cb2233f s390 port

The s390 port only needs to be merged to M51, since M50 does not include s390 support yet.

Comment 21 by ClusterFuzz, Apr 21 2016

Project Member
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify

Comment 22 by tin...@google.com, Apr 22 2016

Labels: -Merge-Request-50 Merge-Review-50 Hotlist-Merge-Review
[Automated comment] There appears to be on-going work (i.e. bugroid changes), needs manual review.

Comment 23 by tin...@google.com, Apr 22 2016

Labels: -Merge-Request-51 Merge-Review-51
[Automated comment] There appears to be on-going work (i.e. bugroid changes), needs manual review.

Comment 24 by tin...@google.com, Apr 22 2016

Labels: -Merge-Request-50 Merge-Review-50 Hotlist-Merge-Review
[Automated comment] There appears to be on-going work (i.e. bugroid changes), needs manual review.

Comment 25 by yangguo@chromium.org, Apr 22 2016

Another MIPS64 fix to include in the back merges: 71dd5c4380e191f0a4cd1d5b6b044bdf1c04f0f9

Comment 26 by sshru...@google.com, Apr 23 2016

Looks like there is ongoing work here. What are all these changes, and do we have a full list of what we want to merge into M51 yet?

Comment 27 by yangguo@chromium.org, Apr 23 2016

The fix is dependent on CPU architecture, and while the initial fix covered 5 archs, the missing 4 were ported by their respective external maintainers. The last CL to fix the MIPS64 port should be the last one. I don't expect any further changes.

Comment 28 by yangguo@chromium.org, Apr 23 2016

The list of patches are in comment 20 and 25.

Comment 29 by sshru...@google.com, Apr 25 2016

Labels: -Merge-Review-51 Merge-Approved-51
Merge approved for M51 (branch 2704)

Comment 30 by gov...@chromium.org, Apr 26 2016

Please merge your change before 5:00 PM PST tomorrow (Tuesday) to M51 branch 2704, so we can take it for this week beta. Thank you.

Comment 31 by bugdroid1@chromium.org, Apr 26 2016

Project Member
Labels: merge-merged-5.1
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/d05748bc7964ac1f9e599406bf779f3a3bb4395e

commit d05748bc7964ac1f9e599406bf779f3a3bb4395e
Author: Yang Guo <yangguo@chromium.org>
Date: Tue Apr 26 12:42:20 2016

Version 5.1.281.18 (cherry-pick)

Merged 3518e492c0939759ae1a2623bbd606646ee172f1
Merged 7d8e279a7b11cb3c1cbeb424118a20210f9b8eea
Merged 644bade748fafb1f2e8ab25ca33473a4c77c006d
Merged b4697727e946e8d96d5345a4db7ff72842d60466
Merged 14c9cbd4cfaa235801e0372178e8af039cb2233f
Merged 71dd5c4380e191f0a4cd1d5b6b044bdf1c04f0f9

[regexp] do not assume short external strings have a minimum size.

PPC: [regexp] do not assume short external strings have a minimum size.

X87: [regexp] do not assume short external strings have a minimum size.

MIPS64: [regexp] do not assume short external strings have a minimum size.

S390: [regexp] do not assume short external strings have a minimum size.

MIPS64: [regexp] do not assume short external strings have a minimum size.

BUG= chromium:604897 , chromium:604897 , chromium:604897 , chromium:604897 ,v8:4923,v8:4923,v8:4923,v8:4923
LOG=N
R=hablich@chromium.org

Review URL: https://codereview.chromium.org/1923523002 .

Cr-Commit-Position: refs/branch-heads/5.1@{#22}
Cr-Branched-From: 167dc63b4c9a1d0f0fe1b19af93644ac9a561e83-refs/heads/5.1.281@{#1}
Cr-Branched-From: 03953f52bd4a184983a551927c406be6489ef89b-refs/heads/master@{#35282}

[modify] https://crrev.com/d05748bc7964ac1f9e599406bf779f3a3bb4395e/include/v8-version.h
[modify] https://crrev.com/d05748bc7964ac1f9e599406bf779f3a3bb4395e/src/arm/code-stubs-arm.cc
[modify] https://crrev.com/d05748bc7964ac1f9e599406bf779f3a3bb4395e/src/arm64/code-stubs-arm64.cc
[modify] https://crrev.com/d05748bc7964ac1f9e599406bf779f3a3bb4395e/src/ia32/code-stubs-ia32.cc
[modify] https://crrev.com/d05748bc7964ac1f9e599406bf779f3a3bb4395e/src/mips/code-stubs-mips.cc
[modify] https://crrev.com/d05748bc7964ac1f9e599406bf779f3a3bb4395e/src/mips64/code-stubs-mips64.cc
[modify] https://crrev.com/d05748bc7964ac1f9e599406bf779f3a3bb4395e/src/objects.h
[modify] https://crrev.com/d05748bc7964ac1f9e599406bf779f3a3bb4395e/src/ppc/code-stubs-ppc.cc
[modify] https://crrev.com/d05748bc7964ac1f9e599406bf779f3a3bb4395e/src/s390/code-stubs-s390.cc
[modify] https://crrev.com/d05748bc7964ac1f9e599406bf779f3a3bb4395e/src/x64/code-stubs-x64.cc
[modify] https://crrev.com/d05748bc7964ac1f9e599406bf779f3a3bb4395e/src/x87/code-stubs-x87.cc
[modify] https://crrev.com/d05748bc7964ac1f9e599406bf779f3a3bb4395e/test/cctest/test-regexp.cc

Comment 32 by gov...@chromium.org, Apr 26 2016

Labels: -Merge-Approved-51
Per comment #31, this is already merged to M51. So removing "Merge-Approved-51" label.

Comment 33 Deleted

Comment 34 by tin...@google.com, Apr 26 2016

Before we approve merge to M50, Could you please confirm whether this bug is baked and verified in Canary and Beta and safe to merge?

Comment 35 by yangguo@chromium.org, Apr 27 2016

The original fixes on M52 has been getting coverage since Canary 2715, which seems to be stable wrt V8.

Comment 36 by hablich@chromium.org, Apr 27 2016

Labels: -Merge-Review-50 Merge-Approved-5.0

Comment 37 by bugdroid1@chromium.org, Apr 28 2016

Project Member
Labels: merge-merged-5.0
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/dfaec1393ed70c11e2b326e87cdca5630062abc6

commit dfaec1393ed70c11e2b326e87cdca5630062abc6
Author: Yang Guo <yangguo@chromium.org>
Date: Thu Apr 28 07:15:42 2016

Version 5.0.71.40 (cherry-pick)

Merged 3518e492c0939759ae1a2623bbd606646ee172f1
Merged 7d8e279a7b11cb3c1cbeb424118a20210f9b8eea
Merged 644bade748fafb1f2e8ab25ca33473a4c77c006d
Merged b4697727e946e8d96d5345a4db7ff72842d60466
Merged 71dd5c4380e191f0a4cd1d5b6b044bdf1c04f0f9

[regexp] do not assume short external strings have a minimum size.

PPC: [regexp] do not assume short external strings have a minimum size.

X87: [regexp] do not assume short external strings have a minimum size.

MIPS64: [regexp] do not assume short external strings have a minimum size.

MIPS64: [regexp] do not assume short external strings have a minimum size.

BUG= chromium:604897 , chromium:604897 , chromium:604897 ,v8:4923,v8:4923,v8:4923
LOG=N
R=hablich@chromium.org

Review URL: https://codereview.chromium.org/1927003003 .

Cr-Commit-Position: refs/branch-heads/5.0@{#47}
Cr-Branched-From: ad16e6c2cbd2c6b0f2e8ff944ac245561c682ac2-refs/heads/5.0.71@{#1}
Cr-Branched-From: bd9df50d75125ee2ad37b3d92c8f50f0a8b5f030-refs/heads/master@{#34215}

[modify] https://crrev.com/dfaec1393ed70c11e2b326e87cdca5630062abc6/include/v8-version.h
[modify] https://crrev.com/dfaec1393ed70c11e2b326e87cdca5630062abc6/src/arm/code-stubs-arm.cc
[modify] https://crrev.com/dfaec1393ed70c11e2b326e87cdca5630062abc6/src/arm64/code-stubs-arm64.cc
[modify] https://crrev.com/dfaec1393ed70c11e2b326e87cdca5630062abc6/src/ia32/code-stubs-ia32.cc
[modify] https://crrev.com/dfaec1393ed70c11e2b326e87cdca5630062abc6/src/mips/code-stubs-mips.cc
[modify] https://crrev.com/dfaec1393ed70c11e2b326e87cdca5630062abc6/src/mips64/code-stubs-mips64.cc
[modify] https://crrev.com/dfaec1393ed70c11e2b326e87cdca5630062abc6/src/objects.h
[modify] https://crrev.com/dfaec1393ed70c11e2b326e87cdca5630062abc6/src/ppc/code-stubs-ppc.cc
[modify] https://crrev.com/dfaec1393ed70c11e2b326e87cdca5630062abc6/src/x64/code-stubs-x64.cc
[modify] https://crrev.com/dfaec1393ed70c11e2b326e87cdca5630062abc6/src/x87/code-stubs-x87.cc
[modify] https://crrev.com/dfaec1393ed70c11e2b326e87cdca5630062abc6/test/cctest/test-regexp.cc

Comment 38 by sheriffbot@chromium.org, May 1 2016

Project Member
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 39 by yangguo@chromium.org, May 2 2016

Labels: -Merge-Approved-5.0

Comment 40 by timwillis@google.com, May 24 2016

Labels: reward-topanel M-51 Release-0-M51 M-50
Adding reward-topanel to consider this under Chrome's security reward program: https://www.google.com/about/appsecurity/chrome-rewards/

Comment 41 by timwillis@google.com, May 25 2016

Labels: -Security_Severity-High Security_Severity-Medium
Updating severity

Comment 42 by timwillis@google.com, May 25 2016

Labels: -reward-topanel CVE-2016-1688 reward-unpaid reward-1000
Thanks for reporting this issue. Our reward panel decided to award you $1,000 for this report. Congratulations!

We've credited you in our release notes as "Max Korenko": https://googlechromereleases.blogspot.com/2016/05/stable-channel-update_25.html - if you'd like to use a different name, please let me know.

Someone from our finance team will be in contact to collect details for payment within 7 days. If that doesn't happen, please either update this bug or contact me at timwillis@.

The CVE-ID for this issue is CVE-2016-1688. Usual boilerplate text below - let me know if you have any questions.

Thanks again for the report!

*** 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 established 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.

Comment 43 by timwillis@google.com, Jun 8 2016

Labels: reward-inprocess

Comment 44 by timwillis@google.com, Jun 8 2016

Labels: -reward-unpaid

Comment 45 by sheriffbot@chromium.org, Jul 28 2016

Project Member
Labels: -Restrict-View-SecurityNotify
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

Comment 46 by sheriffbot@chromium.org, Oct 1 2016

Project Member
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

Comment 47 by sheriffbot@chromium.org, Oct 2 2016

Project Member
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

Comment 48 by mbarbe...@chromium.org, Oct 2 2016

Labels: allpublic

Comment 49 by awhalley@chromium.org, Apr 25 2018

Labels: CVE_description-submitted

Sign in to add a comment