[Backport] CVE-2024-0519: Out of bounds memory access in V8

Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/v8/v8/+/5192447:
Merged: [runtime] Drop fast last-property deletion

This interacts badly with other optimizations and isn't particularly
common.

Bug: chromium:1517354
(cherry picked from commit 389ea9be7d68bb189e16da79f6414edbd4f7594f)

Change-Id: Ie16aa38e8984c4879491c0d9a0ca9df0e041fd1d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5192447
Auto-Submit: Toon Verwaest <[email protected]>
Reviewed-by: Leszek Swirski <[email protected]>
Cr-Commit-Position: refs/branch-heads/12.0@{#32}
Cr-Branched-From: ed7b4caf1fb8184ad9e24346c84424055d4d430a-refs/heads/12.0.267@{#1}
Cr-Branched-From: 210e75b19db4352c9b78dce0bae11c2dc3077df4-refs/heads/main@{#90651}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/531980
Reviewed-by: Allan Sandfeld Jensen <[email protected]>

Author: verwaest

chromium/v8/src/runtime/runtime-object.cc

@@ -71,184 +71,10 @@ MaybeHandle<Object> Runtime::HasProperty(Isolate* isolate,
   return ReadOnlyRoots(isolate).boolean_value_handle(maybe.FromJust());
 }
 
-namespace {
-
-// This function sets the sentinel value in a deleted field. Thes sentinel has
-// to look like a proper standalone object because the slack tracking may
-// complete at any time. For this reason we use the filler map word.
-// If V8_MAP_PACKING is enabled, then the filler map word is a packed filler
-// map. Otherwise, the filler map word is the same as the filler map.
-inline void ClearField(Isolate* isolate, JSObject object, FieldIndex index) {
-  if (index.is_inobject()) {
-    MapWord filler_map_word =
-        ReadOnlyRoots(isolate).one_pointer_filler_map_word();
-#ifndef V8_MAP_PACKING
-    DCHECK_EQ(filler_map_word.ToMap(),
-              ReadOnlyRoots(isolate).one_pointer_filler_map());
-#endif
-    int offset = index.offset();
-    TaggedField<MapWord>::Release_Store(object, offset, filler_map_word);
-  } else {
-    object->property_array()->set(
-        index.outobject_array_index(),
-        ReadOnlyRoots(isolate).one_pointer_filler_map());
-  }
-}
-
-void GeneralizeAllTransitionsToFieldAsMutable(Isolate* isolate, Handle<Map> map,
-                                              Handle<Name> name) {
-  InternalIndex descriptor(map->NumberOfOwnDescriptors());
-
-  Handle<Map> target_maps[kPropertyAttributesCombinationsCount];
-  int target_maps_count = 0;
-
-  // Collect all outgoing field transitions.
-  {
-    DisallowGarbageCollection no_gc;
-    TransitionsAccessor transitions(isolate, *map);
-    transitions.ForEachTransitionTo(
-        *name,
-        [&](Map target) {
-          DCHECK_EQ(descriptor, target->LastAdded());
-          DCHECK_EQ(*name, target->GetLastDescriptorName(isolate));
-          PropertyDetails details = target->GetLastDescriptorDetails(isolate);
-          // Currently, we track constness only for fields.
-          if (details.kind() == PropertyKind::kData &&
-              details.constness() == PropertyConstness::kConst) {
-            target_maps[target_maps_count++] = handle(target, isolate);
-          }
-          DCHECK_IMPLIES(details.kind() == PropertyKind::kAccessor,
-                         details.constness() == PropertyConstness::kConst);
-        },
-        &no_gc);
-    CHECK_LE(target_maps_count, kPropertyAttributesCombinationsCount);
-  }
-
-  for (int i = 0; i < target_maps_count; i++) {
-    Handle<Map> target = target_maps[i];
-    PropertyDetails details =
-        target->instance_descriptors(isolate)->GetDetails(descriptor);
-    Handle<FieldType> field_type(
-        target->instance_descriptors(isolate)->GetFieldType(descriptor),
-        isolate);
-    MapUpdater::GeneralizeField(isolate, target, descriptor,
-                                PropertyConstness::kMutable,
-                                details.representation(), field_type);
-    DCHECK_EQ(PropertyConstness::kMutable, target->instance_descriptors(isolate)
-                                               ->GetDetails(descriptor)
-                                               .constness());
-  }
-}
-
-bool DeleteObjectPropertyFast(Isolate* isolate, Handle<JSReceiver> receiver,
-                              Handle<Object> raw_key) {
-  // This implements a special case for fast property deletion: when the
-  // last property in an object is deleted, then instead of normalizing
-  // the properties, we can undo the last map transition, with a few
-  // prerequisites:
-  // (1) The receiver must be a regular object and the key a unique name.
-  Handle<Map> receiver_map(receiver->map(), isolate);
-  if (IsSpecialReceiverMap(*receiver_map)) return false;
-  DCHECK(IsJSObjectMap(*receiver_map));
-
-  if (!IsUniqueName(*raw_key)) return false;
-  Handle<Name> key = Handle<Name>::cast(raw_key);
-  // (2) The property to be deleted must be the last property.
-  int nof = receiver_map->NumberOfOwnDescriptors();
-  if (nof == 0) return false;
-  InternalIndex descriptor(nof - 1);
-  Handle<DescriptorArray> descriptors(
-      receiver_map->instance_descriptors(isolate), isolate);
-  if (descriptors->GetKey(descriptor) != *key) return false;
-  // (3) The property to be deleted must be deletable.
-  PropertyDetails details = descriptors->GetDetails(descriptor);
-  if (!details.IsConfigurable()) return false;
-  // (4) The map must have a back pointer.
-  Handle<Object> backpointer(receiver_map->GetBackPointer(), isolate);
-  if (!IsMap(*backpointer)) return false;
-  Handle<Map> parent_map = Handle<Map>::cast(backpointer);
-  // (5) The last transition must have been caused by adding a property
-  // (and not any kind of special transition).
-  if (parent_map->NumberOfOwnDescriptors() != nof - 1) return false;
-
-  // Preconditions successful. No more bailouts after this point.
-
-  // Zap the property to avoid keeping objects alive. Zapping is not necessary
-  // for properties stored in the descriptor array.
-  if (details.location() == PropertyLocation::kField) {
-    DisallowGarbageCollection no_gc;
-
-    // Invalidate slots manually later in case we delete an in-object tagged
-    // property. In this case we might later store an untagged value in the
-    // recorded slot.
-    isolate->heap()->NotifyObjectLayoutChange(*receiver, no_gc,
-                                              InvalidateRecordedSlots::kNo);
-    FieldIndex index =
-        FieldIndex::ForPropertyIndex(*receiver_map, details.field_index());
-    // Special case deleting the last out-of object property.
-    if (!index.is_inobject() && index.outobject_array_index() == 0) {
-      DCHECK(!parent_map->HasOutOfObjectProperties());
-      // Clear out the properties backing store.
-      receiver->SetProperties(ReadOnlyRoots(isolate).empty_fixed_array());
-    } else {
-      ClearField(isolate, JSObject::cast(*receiver), index);
-      if (index.is_inobject()) {
-        // We need to clear the recorded slot in this case because in-object
-        // slack tracking might not be finished. This ensures that we don't
-        // have recorded slots in free space.
-        isolate->heap()->ClearRecordedSlot(*receiver,
-                                           receiver->RawField(index.offset()));
-      }
-    }
-  }
-  // If the {receiver_map} was marked stable before, then there could be
-  // optimized code that depends on the assumption that no object that
-  // reached this {receiver_map} transitions away from it without triggering
-  // the "deoptimize dependent code" mechanism.
-  receiver_map->NotifyLeafMapLayoutChange(isolate);
-  // Finally, perform the map rollback.
-  receiver->set_map(*parent_map, kReleaseStore);
-#if VERIFY_HEAP
-  if (v8_flags.verify_heap) {
-    receiver->HeapObjectVerify(isolate);
-    receiver->property_array()->PropertyArrayVerify(isolate);
-  }
-#endif
-
-  // If the {descriptor} was "const" so far, we need to update the
-  // {receiver_map} here, otherwise we could get the constants wrong, i.e.
-  //
-  //   o.x = 1;
-  //   [change o.x's attributes or reconfigure property kind]
-  //   delete o.x;
-  //   o.x = 2;
-  //
-  // could trick V8 into thinking that `o.x` is still 1 even after the second
-  // assignment.
-
-  // Step 1: Migrate object to an up-to-date shape.
-  if (parent_map->is_deprecated()) {
-    JSObject::MigrateInstance(isolate, Handle<JSObject>::cast(receiver));
-    parent_map = handle(receiver->map(), isolate);
-  }
-
-  // Step 2: Mark outgoing transitions from the up-to-date version of the
-  // parent_map to same property name of any kind or attributes as mutable.
-  // Also migrate object to the up-to-date map to make the object shapes
-  // converge sooner.
-  GeneralizeAllTransitionsToFieldAsMutable(isolate, parent_map, key);
-
-  return true;
-}
-
-}  // namespace
-
 Maybe<bool> Runtime::DeleteObjectProperty(Isolate* isolate,
                                           Handle<JSReceiver> receiver,
                                           Handle<Object> key,
                                           LanguageMode language_mode) {
-  if (DeleteObjectPropertyFast(isolate, receiver, key)) return Just(true);
-
   bool success = false;
   PropertyKey lookup_key(isolate, key, &success);
   if (!success) return Nothing<bool>();