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}
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));
}