commit 66c8de2cdac10cad9e622ecededda411b44ac5b3
author Samuel Groß <saelo@chromium.org> Tue Apr 19 12:49:01 2022
committer V8 LUCI CQ <v8-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Apr 27 08:18:18 2022
tree 34e5a7d8152c90c50d3dd35182d01fe883086fcb
parent dbff30456e14ec3fec54869b3e41d091d2489461

Harden Map.prototype.delete and related methods

These can be tricked into corrupting memory when an attacker can leak
the "hole" value due to a bug. This CL simply adds CHECKs to prevent
this. A longer-term solution might be to introduce "special-purpose
holes" so that a leaked "hole" value can no longer be used to confuse
unrelated code like the JSMap implementation because that would then use
a different "hole" value.

Bug: chromium:1315901
Change-Id: Id6c432d39fb97002fa67efe90d34014fc5408ba3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3593783
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80201}

src/builtins/builtins-collections-gen.cc

diff --git a/src/builtins/builtins-collections-gen.cc b/src/builtins/builtins-collections-gen.cc
index 00cad7b..d590ffd 100644
--- a/src/builtins/builtins-collections-gen.cc
+++ b/src/builtins/builtins-collections-gen.cc
@@ -1762,6 +1762,9 @@
   ThrowIfNotInstanceType(context, receiver, JS_MAP_TYPE,
                          "Map.prototype.delete");
 
+  // This check breaks a known exploitation technique. See crbug.com/1263462
+  CSA_CHECK(this, TaggedNotEqual(key, TheHoleConstant()));
+
   const TNode<OrderedHashMap> table =
       LoadObjectField<OrderedHashMap>(CAST(receiver), JSMap::kTableOffset);
 
@@ -1930,6 +1933,9 @@
   ThrowIfNotInstanceType(context, receiver, JS_SET_TYPE,
                          "Set.prototype.delete");
 
+  // This check breaks a known exploitation technique. See crbug.com/1263462
+  CSA_CHECK(this, TaggedNotEqual(key, TheHoleConstant()));
+
   const TNode<OrderedHashSet> table =
       LoadObjectField<OrderedHashSet>(CAST(receiver), JSMap::kTableOffset);
 
@@ -2878,6 +2884,9 @@
   ThrowIfNotInstanceType(context, receiver, JS_WEAK_MAP_TYPE,
                          "WeakMap.prototype.delete");
 
+  // This check breaks a known exploitation technique. See crbug.com/1263462
+  CSA_CHECK(this, TaggedNotEqual(key, TheHoleConstant()));
+
   Return(CallBuiltin(Builtin::kWeakCollectionDelete, context, receiver, key));
 }
 
@@ -2926,6 +2935,9 @@
   ThrowIfNotInstanceType(context, receiver, JS_WEAK_SET_TYPE,
                          "WeakSet.prototype.delete");
 
+  // This check breaks a known exploitation technique. See crbug.com/1263462
+  CSA_CHECK(this, TaggedNotEqual(value, TheHoleConstant()));
+
   Return(CallBuiltin(Builtin::kWeakCollectionDelete, context, receiver, value));
 }