ClusterFuzz has detected this issue as fixed in range 44025:44026.

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

Fuzzer: mbarbella_js_mutation
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  size_ <= capacity_ in identity-map.cc
  
Sanitizer: address (ASAN)

Regressed: V8: 43986:43987
Fixed: V8: 44025:44026

Reproducer Testcase: https://clusterfuzz.com/download/AMIfv94xJnGbAqSExJYxqH5BtHAYisEiRKyrZhAB4BJ2YpCQ4t1XrHwtzvsEXQoFzGO8HQ-0S18ZtS-NHZG1zqlAl7c9PVDXvrjXyTIdm4MroBvMYe4_YJe1EjJnNd847FWZdEtuLrBgQp_9R8yNbSbaAAnPhBzj0_yRRPOlvplSXTxkvD-uH9U9rDeheVRT8kaQlIAmtW-6JwIXvF163SNXtUUm_9DoyTEZO0boPyexCbS7om7HDhavQBJNqIHtl3Wsg_V3YoOvIZR_setbAUK00eIuX58sk5a5eZvCk9aeCl2dp-LVYB2NN8j3ZGJBaD6E1DSHaeSY_RZ3-ENOOqs9a7YxJ5G2Eu7-eoURAgQRzWWUqqpHQHaB_61DHIH9mf8wfWqt_1Q1URPeq8dcJXkuCF6m-a6EQA?testcase_id=6601594327269376


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs 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 6601594327269376 is verified as fixed, so closing issue.

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

Nope, this is still an issue, re-opening.

The underlying issues is that short-circuiting breaks assumptions that V8's IdentityMap makes about object identity. With short circuiting it is possible that a GC coalesces two objects that initially had a distinct identity into one object. If such objects are used as keys in an IdentityMap, then one of the two values is picked (at random) and the {size} field of the IdentityMap gets out of sync. The following is a cctest reproducing the problem:

TEST(Shortcut) {
  IdentityMapTester t;

  Handle<String> s1 = t.isolate()->factory()->NewStringFromAsciiChecked("abcd");
  Handle<String> s2 = t.isolate()->factory()->NewStringFromAsciiChecked("abcd");

  t.map.Set(s1, Smi::FromInt(23));
  t.map.Set(s2, Smi::FromInt(42));

  t.CheckFind(s1, Smi::FromInt(23));
  t.CheckFind(s2, Smi::FromInt(42));
  t.CheckGet(s1, Smi::FromInt(23));
  t.CheckGet(s2, Smi::FromInt(42));

  printf("Initial string:\n");
  printf(" %p: ", (void*)*s1); s1->Print();
  printf(" %p: ", (void*)*s2); s2->Print();

  t.isolate()->factory()->InternalizeString(s1);
  t.isolate()->factory()->InternalizeString(s2);

  printf("Internalized:\n");
  printf(" %p: ", (void*)*s1); s1->Print();
  printf(" %p: ", (void*)*s2); s2->Print();

  CcTest::CollectGarbage(NEW_SPACE);
  CcTest::CollectGarbage(NEW_SPACE);

  printf("After GC:\n");
  printf(" %p: ", (void*)*s1); s1->Print();
  printf(" %p: ", (void*)*s2); s2->Print();

  t.CheckFind(s1, Smi::FromInt(23));
  t.CheckFind(s2, Smi::FromInt(42));
  t.CheckGet(s1, Smi::FromInt(23));
  t.CheckGet(s2, Smi::FromInt(42));
}

We can probably fix the size going out of sync by adding some logic during the rehashing operation. My question is, even after this is fixed, would this break invariants we have in the CanonicalHandleScope? I.e., would two objects originally having distinct identity but having a single identity after a GC cause issues anywhere in the compilers?

Yes, I agree with comment #7: Fixing the {size} field is the easy part. But picking one of the two values "at random" changes the contract and I am also not sure users of the CanonicalHandleScope could cope with that (not to mention that describing/documenting the contract in an understandable way will be tricky)

Not really related to the issue at hand, but please note that the sudden coalescing of strings in our GC also has the potential to break TurboFan's type system (which distinguishes between internalized and non-internalized strings). This means the for example the type of a {HeapConstant} node referencing a string might changes over time. The following is a test case exemplifying the potential issue:

=========================

TEST(StringTypeChanges) {
  Isolate* isolate = CcTest::InitIsolateOnce();
  Zone zone(isolate->allocator(), ZONE_NAME);
  HandleScope scope(isolate);

  Handle<String> s = isolate->factory()->NewStringFromAsciiChecked("abcd");

  printf("Initial string: %p: ", (void*)*s); s->Print();
  printf("\t"); Type::Of(*s, &zone)->Print();

  isolate->factory()->InternalizeString(s);

  printf("Internalized: %p: ", (void*)*s); s->Print();
  printf("\t"); Type::Of(*s, &zone)->Print();

  CcTest::CollectGarbage(NEW_SPACE);
  CcTest::CollectGarbage(NEW_SPACE);

  printf("After GC: %p: ", (void*)*s); s->Print();
  printf("\t"); Type::Of(*s, &zone)->Print();
}

=========================

The output of this snippet is as follows:

> Initial string: 0x2e518d187a31: "abcd"
>     OtherString
> Internalized: 0x2e518d187a31: >"abcd"
>     OtherString
> After GC: 0x22859aaa741: #abcd
>     InternalizedString

This can lead to very tricky bugs, because TurboFan optimizes based on types. The issue here is that the type of a value changes over time to something that is not a subtype of the original type. If this is really desired then we could ducktape it in TurboFan by never returning OtherString, but always returning something that includes InternalizedString even for non-internalized Strings.

However it might be better to not do the short-circuiting if other parts of the system also need ducktape.

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

Fuzzer: mbarbella_js_mutation
Job Type: linux_asan_d8_v8_arm64_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  size_ <= capacity_ in identity-map.cc
  
Sanitizer: address (ASAN)

Regressed: V8: 43683:43684

Reproducer Testcase: https://clusterfuzz.com/download/AMIfv96ErECNlC2653V6xk2nBa0yZayorjf96vdh9Ed0JCPXVKIDibpakWRllceCzt5y6r-ppnaAmu0ToySwPRTfxYbFhnp7D0EmTE3nEoAq8bu1t2ItvtknnG_oOBfjBRP3YTzmPfgPSm6ChRvsO6WCAdqElTJExXXmTTCRDhIZkUngeklQztWss5rYBYsul0GGyr6kLdMhlrKgPCIfJzhGX9xDKE30uC10nurL-SThDPkPM23oe0WWQDgj1lRtf2XuM9FCrlN-cgW4JL9b9dW85wIrAohvw2uESmvrVqanXudSzMaKGVyJDPvL5Zy6w_PdqGqEIKyl_cdxNEu9QEtNJLWeIqMAz8DiF58IQZKDfqA8qfKuZkQ?testcase_id=5870748817424384


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

Applying security view restrictions to all v8 CHECK/DCHECK failures.

(CHECKs aren't security, but we have no way to distinguish these right now).

c#12 still reproduces.

 Issue 727224  has been merged into this issue.

This is still happening. Assigning to Michi who already investigated this issue.

mstarzinger: Uh oh! This issue still open and hasn't been updated in the last 64 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

We commit ourselves to a 60 day deadline for fixing for high severity vulnerabilities, and have exceeded it here. If you're unable to look into this soon, could you please find another owner or remove yourself so that this gets back into the security triage queue?

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

I don't plan to block M59 stable on a high sev bug - critical would be a different story.  Punting to M60, if you have a super safe and simple fix you can request a merge and we can discuss at that point.

+awhalley@ as FYI.

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

Fuzzer: mbarbella_js_mutation
Job Type: linux_asan_d8_v8_arm_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  size_ <= capacity_ in identity-map.cc
  v8::internal::IdentityMapBase::LookupOrInsert
  v8::internal::IdentityMapBase::GetEntry
  
Sanitizer: address (ASAN)

Regressed: V8: 45725:45726

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


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

ClusterFuzz has detected this issue as fixed in range 45768:45769.

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

Fuzzer: mbarbella_js_mutation
Job Type: linux_asan_d8_v8_arm_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  size_ <= capacity_ in identity-map.cc
  v8::internal::IdentityMapBase::LookupOrInsert
  v8::internal::IdentityMapBase::GetEntry
  
Sanitizer: address (ASAN)

Regressed: V8: 45725:45726
Fixed: V8: 45768:45769

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


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

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

mstarzinger: Uh oh! This issue still open and hasn't been updated in the last 78 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

ClusterFuzz has detected this issue as fixed in range 45974:45975.

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

Fuzzer: mbarbella_js_mutation
Job Type: linux_asan_d8_v8_arm64_dbg
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  size_ <= capacity_ in identity-map.cc
  
Sanitizer: address (ASAN)

Fixed: V8: 45974:45975

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


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

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

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

commit 811643b49b569af21774c4957b4778813cd2dcfd
Author: Ross McIlroy <rmcilroy@chromium.org>
Date: Wed Jun 21 09:37:20 2017

Reland: [IdentityMap] Fix size if GC short-cuts objects.

BUG= chromium:704132 

Change-Id: I5be333888215718c2680f5a442fe26ffd988f04e
Reviewed-on: https://chromium-review.googlesource.com/541443
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46077}
[modify] https://crrev.com/811643b49b569af21774c4957b4778813cd2dcfd/src/identity-map.cc
[modify] https://crrev.com/811643b49b569af21774c4957b4778813cd2dcfd/test/cctest/test-identity-map.cc

The size/capacity issue is now fix. Michi, do you want to keep this bug open for the short-cutting removal or open a new one?

I have filed issue v8:6521, feel free to close this one.

We should probably merge this into m60, adding merge request label.

This bug requires manual review: M60 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

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

Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

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

Approving merge to M60. 

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

commit 2573829b7dd7af1ee30922bd1123232d0d670819
Author: Ross McIlroy <rmcilroy@chromium.org>
Date: Mon Jul 03 15:19:43 2017

Merged: Reland: [IdentityMap] Fix size if GC short-cuts objects.

Revision: 811643b49b569af21774c4957b4778813cd2dcfd

BUG= chromium:704132 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
TBR=mstarzinger@chromium.org

Change-Id: Ie5203254505d011c8834ea69e39bd5e4c172d2e0
Reviewed-on: https://chromium-review.googlesource.com/558938
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.0@{#55}
Cr-Branched-From: 97dbf624a5eeffb3a8df36d24cdb2a883137385f-refs/heads/6.0.286@{#1}
Cr-Branched-From: 12e6f1cb5cd9616da7b9d4a7655c088778a6d415-refs/heads/master@{#45439}
[modify] https://crrev.com/2573829b7dd7af1ee30922bd1123232d0d670819/src/identity-map.cc
[modify] https://crrev.com/2573829b7dd7af1ee30922bd1123232d0d670819/test/cctest/test-identity-map.cc

We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label.

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