New issue
Advanced search Search tips

Issue 848165 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug



Sign in to add a comment

enumeration_index out-of-bound

Reported by scdengy...@gmail.com, May 31 2018

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.181 Safari/537.36

Steps to reproduce the problem:
class cls0 {
    static get length(){ return 42;  };
    static get [1](){ return 21;  };
};
Object.defineProperty(cls0, "length", {value:'1'});

What is the expected behavior?

What went wrong?
# Fatal error in ../../src/lookup.cc, line 507

# Debug check failed: enumeration_index > 0 (0 vs. 0).

template <typename Dictionary, typename Key>
void AddToDictionaryTemplate(Isolate* isolate, Handle<Dictionary> dictionary,
                             Key key, int key_index,
                             ClassBoilerplate::ValueKind value_kind,
                             Object* value) {
...
PropertyDetails details(kAccessor, DONT_ENUM,
                                PropertyCellType::kNoCell); <---missed the enum_order,make the enumeration_index out-of-bound
        dictionary->DetailsAtPut(entry, details);

Did this work before? N/A 

Chrome version: 66.0.3359.181  Channel: n/a
OS Version: OS X 10.13.4
Flash Version:
 
Project Member

Comment 1 by ClusterFuzz, May 31 2018

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=6630464685867008.
Components: Blink>JavaScript
Owner: ishell@chromium.org
Status: Assigned (was: Unconfirmed)
scdengyuan: Thanks for the report!

ishell: Can you please take a look or reassign? Thanks.
Project Member

Comment 3 by ClusterFuzz, Jun 1 2018

ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=4698965111734272.
Project Member

Comment 4 by ClusterFuzz, Jun 1 2018

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

Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: DCHECK failure
Crash Address: 
Crash State:
  enumeration_index > 0 in lookup.cc
  v8::internal::LookupIterator::ReconfigureDataProperty
  v8::internal::JSObject::DefineOwnPropertyIgnoreAttributes
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=49456:49457

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

See https://github.com/google/clusterfuzz-tools for more information.
Labels: Security_Impact-Stable Security_Severity-High
https://chromium.googlesource.com/v8/v8/+/cc9e77abe8497578a967259f643dcfb12e134fdb is the only CL in the regression range.


Project Member

Comment 6 by sheriffbot@chromium.org, Jun 2 2018

Labels: Target-67 M-67
Project Member

Comment 7 by sheriffbot@chromium.org, Jun 2 2018

Labels: -Pri-2 Pri-1

Comment 8 by palmer@chromium.org, Jun 11 2018

Cc: titzer@chromium.org rmcilroy@chromium.org
Labels: OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Windows
Friendly ping. :)
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 18 2018

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

commit e602c90abcc60f039708d46810ea0cc660ac5bfc
Author: Igor Sheludko <ishell@chromium.org>
Date: Mon Jun 18 12:45:02 2018

Properly set enumeration order for accessor properties in class literals.

Bug:  chromium:848165 
Change-Id: I1ec18bf12f53c24f388dbd529fe62e990fbc8783
Reviewed-on: https://chromium-review.googlesource.com/1104175
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53793}
[modify] https://crrev.com/e602c90abcc60f039708d46810ea0cc660ac5bfc/src/objects/literal-objects.cc
[add] https://crrev.com/e602c90abcc60f039708d46810ea0cc660ac5bfc/test/mjsunit/regress/regress-crbug-848165.js

Labels: M-68 Merge-Request-68
Status: Fixed (was: Assigned)
Thank you for the report!
Project Member

Comment 11 by sheriffbot@chromium.org, Jun 18 2018

Labels: -Merge-Request-68 Hotlist-Merge-Review Merge-Review-68
This bug requires manual review: M68 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), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

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

Comment 12 by sheriffbot@chromium.org, Jun 18 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: reward-topanel
Please verify the fix in canary.
Cc: awhalley@chromium.org
+ awhalley@ for M68 merge review after canary verification/coverage.
Project Member

Comment 16 by ClusterFuzz, Jun 19 2018

ClusterFuzz has detected this issue as fixed in range 53792:53793.

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

Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: DCHECK failure
Crash Address: 
Crash State:
  enumeration_index > 0 in lookup.cc
  v8::internal::LookupIterator::ReconfigureDataProperty
  v8::internal::JSObject::DefineOwnPropertyIgnoreAttributes
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=49456:49457
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=53792:53793

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

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 17 by ClusterFuzz, Jun 19 2018

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Hi ishell@ - the VRP panel was wondering if you could provide some insight into what could go wrong here, we couldn't see a clear route to RCE.
I'm sorry, I think there's no route to RCE, it's a correctness issue with a trivial fix.
awhalley@ is a merge still needed for this?
Labels: -Type-Bug-Security -Hotlist-Merge-Review -reward-topanel -Security_Impact-Stable -Security_Severity-High -Merge-Review-68 Type-Bug
abdulsyed@ - no merge needed from a security point of view. ishell@, please re-add merge-request if the correctness issue requires a merge. Cheers!
Project Member

Comment 22 by sheriffbot@chromium.org, Sep 24

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