summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakob Gruber <jgruber@chromium.org>2020-04-22 06:35:06 +0000
committerMichael Brüning <michael.bruning@qt.io>2020-07-29 10:54:37 +0000
commita5e8bd5e8c93d8cd52423b3189ffe9a04279149c (patch)
tree54557fbecd129bc7bc8fa9a059f1e5e6d7020ce2
parent1cf3807d934912aec23a22d10dcedb0773f75eca (diff)
downloadqtwebengine-chromium-a5e8bd5e8c93d8cd52423b3189ffe9a04279149c.tar.gz
[Backport] CVE-2020-6533: Type Confusion in V8.
Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/2157382: [protectors] Move regexp species protector back to the isolate This reverts the changes made in https://chromium-review.googlesource.com/c/v8/v8/+/1695465 https://chromium-review.googlesource.com/c/v8/v8/+/1776078 We originally moved this protector to the native context to avoid cross-native-context pollution of protector state. Ideally, invalidating a protector in one NC should not affect any other NC. But as it turns out, having the protector on the NC causes more problems than it solves since all affected callers now need to find the correct native context to check. Sometimes (e.g. in CSA regexp builtins) it is possible to blindly check the current NC, but the reasoning behind this optimization is tricky to understand. Sometimes, fetching the correct NC is not possible due to access restrictions. These implementation complexities outweigh the (unknown) potential performance benefits. In the future we should attempt to move away from the protector concept for these kinds of checks. Bug: chromium:1069964,v8:9463 Change-Id: I2cbb2ec7266282165dae5e4a6c8bdbda520c50a9 Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Dominik Inführ <dinfuehr@chromium.org> Commit-Queue: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#67415} Reviewed-by: Michal Klocek <michal.klocek@qt.io>
-rw-r--r--chromium/v8/src/builtins/builtins-regexp-gen.cc8
-rw-r--r--chromium/v8/src/codegen/code-stub-assembler.cc8
-rw-r--r--chromium/v8/src/codegen/code-stub-assembler.h4
-rw-r--r--chromium/v8/src/execution/protectors-inl.h9
-rw-r--r--chromium/v8/src/execution/protectors.cc19
-rw-r--r--chromium/v8/src/execution/protectors.h15
-rw-r--r--chromium/v8/src/heap/setup-heap-internal.cc7
-rw-r--r--chromium/v8/src/objects/lookup.cc43
-rw-r--r--chromium/v8/src/regexp/regexp-utils.cc5
-rw-r--r--chromium/v8/src/roots/roots.h1
-rw-r--r--chromium/v8/tools/v8heapconst.py27
11 files changed, 41 insertions, 105 deletions
diff --git a/chromium/v8/src/builtins/builtins-regexp-gen.cc b/chromium/v8/src/builtins/builtins-regexp-gen.cc
index 68da45c0344..237589af1c7 100644
--- a/chromium/v8/src/builtins/builtins-regexp-gen.cc
+++ b/chromium/v8/src/builtins/builtins-regexp-gen.cc
@@ -729,13 +729,9 @@ void RegExpBuiltinsAssembler::BranchIfFastRegExp(
// This should only be needed for String.p.(split||matchAll), but we are
// conservative here.
- // Note: we are using the current native context here, which may or may not
- // match the object's native context. That's fine: in case of a mismatch, we
- // will bail in the next step when comparing the object's map against the
- // current native context's initial regexp map.
- TNode<NativeContext> native_context = LoadNativeContext(context);
- GotoIf(IsRegExpSpeciesProtectorCellInvalid(native_context), if_ismodified);
+ GotoIf(IsRegExpSpeciesProtectorCellInvalid(), if_ismodified);
+ TNode<NativeContext> native_context = LoadNativeContext(context);
TNode<JSFunction> regexp_fun =
CAST(LoadContextElement(native_context, Context::REGEXP_FUNCTION_INDEX));
TNode<Map> initial_map = CAST(
diff --git a/chromium/v8/src/codegen/code-stub-assembler.cc b/chromium/v8/src/codegen/code-stub-assembler.cc
index 3c23d6fc64e..8e5def1666b 100644
--- a/chromium/v8/src/codegen/code-stub-assembler.cc
+++ b/chromium/v8/src/codegen/code-stub-assembler.cc
@@ -5941,12 +5941,10 @@ TNode<BoolT> CodeStubAssembler::IsTypedArraySpeciesProtectorCellInvalid() {
return TaggedEqual(cell_value, invalid);
}
-TNode<BoolT> CodeStubAssembler::IsRegExpSpeciesProtectorCellInvalid(
- TNode<NativeContext> native_context) {
- TNode<PropertyCell> cell = CAST(LoadContextElement(
- native_context, Context::REGEXP_SPECIES_PROTECTOR_INDEX));
- TNode<Object> cell_value = LoadObjectField(cell, PropertyCell::kValueOffset);
+TNode<BoolT> CodeStubAssembler::IsRegExpSpeciesProtectorCellInvalid() {
TNode<Smi> invalid = SmiConstant(Protectors::kProtectorInvalid);
+ TNode<PropertyCell> cell = RegExpSpeciesProtectorConstant();
+ TNode<Object> cell_value = LoadObjectField(cell, PropertyCell::kValueOffset);
return TaggedEqual(cell_value, invalid);
}
diff --git a/chromium/v8/src/codegen/code-stub-assembler.h b/chromium/v8/src/codegen/code-stub-assembler.h
index a4ed8a12f0e..9c5fe4ed32a 100644
--- a/chromium/v8/src/codegen/code-stub-assembler.h
+++ b/chromium/v8/src/codegen/code-stub-assembler.h
@@ -77,6 +77,7 @@ enum class PrimitiveType { kBoolean, kNumber, kString, kSymbol };
V(PromiseSpeciesProtector, promise_species_protector, \
PromiseSpeciesProtector) \
V(PromiseThenProtector, promise_then_protector, PromiseThenProtector) \
+ V(RegExpSpeciesProtector, regexp_species_protector, RegExpSpeciesProtector) \
V(SetIteratorProtector, set_iterator_protector, SetIteratorProtector) \
V(SingleCharacterStringCache, single_character_string_cache, \
SingleCharacterStringCache) \
@@ -2586,8 +2587,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
TNode<BoolT> IsPromiseThenProtectorCellInvalid();
TNode<BoolT> IsArraySpeciesProtectorCellInvalid();
TNode<BoolT> IsTypedArraySpeciesProtectorCellInvalid();
- TNode<BoolT> IsRegExpSpeciesProtectorCellInvalid(
- TNode<NativeContext> native_context);
+ TNode<BoolT> IsRegExpSpeciesProtectorCellInvalid();
TNode<BoolT> IsPromiseSpeciesProtectorCellInvalid();
TNode<BoolT> IsMockArrayBufferAllocatorFlag() {
diff --git a/chromium/v8/src/execution/protectors-inl.h b/chromium/v8/src/execution/protectors-inl.h
index b2428063e1e..8fe8bed107c 100644
--- a/chromium/v8/src/execution/protectors-inl.h
+++ b/chromium/v8/src/execution/protectors-inl.h
@@ -13,14 +13,6 @@
namespace v8 {
namespace internal {
-#define DEFINE_PROTECTOR_ON_NATIVE_CONTEXT_CHECK(name, cell) \
- bool Protectors::Is##name##Intact(Handle<NativeContext> native_context) { \
- PropertyCell species_cell = native_context->cell(); \
- return species_cell.value().IsSmi() && \
- Smi::ToInt(species_cell.value()) == kProtectorValid; \
- }
-DECLARED_PROTECTORS_ON_NATIVE_CONTEXT(DEFINE_PROTECTOR_ON_NATIVE_CONTEXT_CHECK)
-
#define DEFINE_PROTECTOR_ON_ISOLATE_CHECK(name, root_index, unused_cell) \
bool Protectors::Is##name##Intact(Isolate* isolate) { \
PropertyCell cell = \
@@ -29,6 +21,7 @@ DECLARED_PROTECTORS_ON_NATIVE_CONTEXT(DEFINE_PROTECTOR_ON_NATIVE_CONTEXT_CHECK)
Smi::ToInt(cell.value()) == kProtectorValid; \
}
DECLARED_PROTECTORS_ON_ISOLATE(DEFINE_PROTECTOR_ON_ISOLATE_CHECK)
+#undef DEFINE_PROTECTORS_ON_ISOLATE_CHECK
} // namespace internal
} // namespace v8
diff --git a/chromium/v8/src/execution/protectors.cc b/chromium/v8/src/execution/protectors.cc
index b5b4c47a1bb..6c23ac3ed5b 100644
--- a/chromium/v8/src/execution/protectors.cc
+++ b/chromium/v8/src/execution/protectors.cc
@@ -33,25 +33,6 @@ void TraceProtectorInvalidation(const char* protector_name) {
}
} // namespace
-#define INVALIDATE_PROTECTOR_ON_NATIVE_CONTEXT_DEFINITION(name, cell) \
- void Protectors::Invalidate##name(Isolate* isolate, \
- Handle<NativeContext> native_context) { \
- DCHECK_EQ(*native_context, isolate->raw_native_context()); \
- DCHECK(native_context->cell().value().IsSmi()); \
- DCHECK(Is##name##Intact(native_context)); \
- if (FLAG_trace_protector_invalidation) { \
- TraceProtectorInvalidation(#name); \
- } \
- Handle<PropertyCell> species_cell(native_context->cell(), isolate); \
- PropertyCell::SetValueWithInvalidation( \
- isolate, #cell, species_cell, \
- handle(Smi::FromInt(kProtectorInvalid), isolate)); \
- DCHECK(!Is##name##Intact(native_context)); \
- }
-DECLARED_PROTECTORS_ON_NATIVE_CONTEXT(
- INVALIDATE_PROTECTOR_ON_NATIVE_CONTEXT_DEFINITION)
-#undef INVALIDATE_PROTECTOR_ON_NATIVE_CONTEXT_DEFINITION
-
#define INVALIDATE_PROTECTOR_ON_ISOLATE_DEFINITION(name, unused_index, cell) \
void Protectors::Invalidate##name(Isolate* isolate) { \
DCHECK(isolate->factory()->cell()->value().IsSmi()); \
diff --git a/chromium/v8/src/execution/protectors.h b/chromium/v8/src/execution/protectors.h
index 4601f16cf01..c4ca49d948a 100644
--- a/chromium/v8/src/execution/protectors.h
+++ b/chromium/v8/src/execution/protectors.h
@@ -15,9 +15,6 @@ class Protectors : public AllStatic {
static const int kProtectorValid = 1;
static const int kProtectorInvalid = 0;
-#define DECLARED_PROTECTORS_ON_NATIVE_CONTEXT(V) \
- V(RegExpSpeciesLookupChainProtector, regexp_species_protector)
-
#define DECLARED_PROTECTORS_ON_ISOLATE(V) \
V(ArrayBufferDetaching, ArrayBufferDetachingProtector, \
array_buffer_detaching_protector) \
@@ -41,6 +38,8 @@ class Protectors : public AllStatic {
/* property holder is the %IteratorPrototype%. Note that this also */ \
/* invalidates the SetIterator protector (see below). */ \
V(MapIteratorLookupChain, MapIteratorProtector, map_iterator_protector) \
+ V(RegExpSpeciesLookupChain, RegExpSpeciesProtector, \
+ regexp_species_protector) \
V(PromiseHook, PromiseHookProtector, promise_hook_protector) \
V(PromiseThenLookupChain, PromiseThenProtector, promise_then_protector) \
V(PromiseResolveLookupChain, PromiseResolveProtector, \
@@ -82,19 +81,9 @@ class Protectors : public AllStatic {
V(TypedArraySpeciesLookupChain, TypedArraySpeciesProtector, \
typed_array_species_protector)
-#define DECLARE_PROTECTOR_ON_NATIVE_CONTEXT(name, unused_cell) \
- V8_EXPORT_PRIVATE static inline bool Is##name##Intact( \
- Handle<NativeContext> native_context); \
- V8_EXPORT_PRIVATE static void Invalidate##name( \
- Isolate* isolate, Handle<NativeContext> native_context);
-
- DECLARED_PROTECTORS_ON_NATIVE_CONTEXT(DECLARE_PROTECTOR_ON_NATIVE_CONTEXT)
-#undef DECLARE_PROTECTOR_ON_NATIVE_CONTEXT
-
#define DECLARE_PROTECTOR_ON_ISOLATE(name, unused_root_index, unused_cell) \
V8_EXPORT_PRIVATE static inline bool Is##name##Intact(Isolate* isolate); \
V8_EXPORT_PRIVATE static void Invalidate##name(Isolate* isolate);
-
DECLARED_PROTECTORS_ON_ISOLATE(DECLARE_PROTECTOR_ON_ISOLATE)
#undef DECLARE_PROTECTOR_ON_ISOLATE
};
diff --git a/chromium/v8/src/heap/setup-heap-internal.cc b/chromium/v8/src/heap/setup-heap-internal.cc
index f633b3fe0a8..80fd5c10f57 100644
--- a/chromium/v8/src/heap/setup-heap-internal.cc
+++ b/chromium/v8/src/heap/setup-heap-internal.cc
@@ -899,6 +899,13 @@ void Heap::CreateInitialObjects() {
Handle<PropertyCell> cell =
factory->NewPropertyCell(factory->empty_string());
cell->set_value(Smi::FromInt(Protectors::kProtectorValid));
+ set_regexp_species_protector(*cell);
+ }
+
+ {
+ Handle<PropertyCell> cell =
+ factory->NewPropertyCell(factory->empty_string());
+ cell->set_value(Smi::FromInt(Protectors::kProtectorValid));
set_string_iterator_protector(*cell);
}
diff --git a/chromium/v8/src/objects/lookup.cc b/chromium/v8/src/objects/lookup.cc
index 246e9dc917c..1e2a7d480a2 100644
--- a/chromium/v8/src/objects/lookup.cc
+++ b/chromium/v8/src/objects/lookup.cc
@@ -232,21 +232,12 @@ void LookupIterator::InternalUpdateProtector(Isolate* isolate,
if (!receiver_generic->IsHeapObject()) return;
Handle<HeapObject> receiver = Handle<HeapObject>::cast(receiver_generic);
- // Getting the native_context from the isolate as a fallback. If possible, we
- // use the receiver's creation context instead.
- Handle<NativeContext> native_context = isolate->native_context();
-
ReadOnlyRoots roots(isolate);
if (*name == roots.constructor_string()) {
- // Fetching the context in here since the operation is rather expensive.
- if (receiver->IsJSReceiver()) {
- native_context = Handle<JSReceiver>::cast(receiver)->GetCreationContext();
- }
if (!Protectors::IsArraySpeciesLookupChainIntact(isolate) &&
!Protectors::IsPromiseSpeciesLookupChainIntact(isolate) &&
- !Protectors::IsRegExpSpeciesLookupChainProtectorIntact(
- native_context) &&
+ !Protectors::IsRegExpSpeciesLookupChainIntact(isolate) &&
!Protectors::IsTypedArraySpeciesLookupChainIntact(isolate)) {
return;
}
@@ -262,12 +253,8 @@ void LookupIterator::InternalUpdateProtector(Isolate* isolate,
Protectors::InvalidatePromiseSpeciesLookupChain(isolate);
return;
} else if (receiver->IsJSRegExp(isolate)) {
- if (!Protectors::IsRegExpSpeciesLookupChainProtectorIntact(
- native_context)) {
- return;
- }
- Protectors::InvalidateRegExpSpeciesLookupChainProtector(isolate,
- native_context);
+ if (!Protectors::IsRegExpSpeciesLookupChainIntact(isolate)) return;
+ Protectors::InvalidateRegExpSpeciesLookupChain(isolate);
return;
} else if (receiver->IsJSTypedArray(isolate)) {
if (!Protectors::IsTypedArraySpeciesLookupChainIntact(isolate)) return;
@@ -293,12 +280,8 @@ void LookupIterator::InternalUpdateProtector(Isolate* isolate,
Protectors::InvalidatePromiseSpeciesLookupChain(isolate);
} else if (isolate->IsInAnyContext(*receiver,
Context::REGEXP_PROTOTYPE_INDEX)) {
- if (!Protectors::IsRegExpSpeciesLookupChainProtectorIntact(
- native_context)) {
- return;
- }
- Protectors::InvalidateRegExpSpeciesLookupChainProtector(isolate,
- native_context);
+ if (!Protectors::IsRegExpSpeciesLookupChainIntact(isolate)) return;
+ Protectors::InvalidateRegExpSpeciesLookupChain(isolate);
} else if (isolate->IsInAnyContext(
receiver->map(isolate).prototype(isolate),
Context::TYPED_ARRAY_PROTOTYPE_INDEX)) {
@@ -334,15 +317,9 @@ void LookupIterator::InternalUpdateProtector(Isolate* isolate,
Protectors::InvalidateStringIteratorLookupChain(isolate);
}
} else if (*name == roots.species_symbol()) {
- // Fetching the context in here since the operation is rather expensive.
- if (receiver->IsJSReceiver()) {
- native_context = Handle<JSReceiver>::cast(receiver)->GetCreationContext();
- }
-
if (!Protectors::IsArraySpeciesLookupChainIntact(isolate) &&
!Protectors::IsPromiseSpeciesLookupChainIntact(isolate) &&
- !Protectors::IsRegExpSpeciesLookupChainProtectorIntact(
- native_context) &&
+ !Protectors::IsRegExpSpeciesLookupChainIntact(isolate) &&
!Protectors::IsTypedArraySpeciesLookupChainIntact(isolate)) {
return;
}
@@ -359,12 +336,8 @@ void LookupIterator::InternalUpdateProtector(Isolate* isolate,
Protectors::InvalidatePromiseSpeciesLookupChain(isolate);
} else if (isolate->IsInAnyContext(*receiver,
Context::REGEXP_FUNCTION_INDEX)) {
- if (!Protectors::IsRegExpSpeciesLookupChainProtectorIntact(
- native_context)) {
- return;
- }
- Protectors::InvalidateRegExpSpeciesLookupChainProtector(isolate,
- native_context);
+ if (!Protectors::IsRegExpSpeciesLookupChainIntact(isolate)) return;
+ Protectors::InvalidateRegExpSpeciesLookupChain(isolate);
} else if (IsTypedArrayFunctionInAnyContext(isolate, *receiver)) {
if (!Protectors::IsTypedArraySpeciesLookupChainIntact(isolate)) return;
Protectors::InvalidateTypedArraySpeciesLookupChain(isolate);
diff --git a/chromium/v8/src/regexp/regexp-utils.cc b/chromium/v8/src/regexp/regexp-utils.cc
index 73c2015dd91..556edbdac88 100644
--- a/chromium/v8/src/regexp/regexp-utils.cc
+++ b/chromium/v8/src/regexp/regexp-utils.cc
@@ -185,10 +185,7 @@ bool RegExpUtils::IsUnmodifiedRegExp(Isolate* isolate, Handle<Object> obj) {
// property. Similar spots in CSA would use BranchIfFastRegExp_Strict in this
// case.
- if (!Protectors::IsRegExpSpeciesLookupChainProtectorIntact(
- recv.GetCreationContext())) {
- return false;
- }
+ if (!Protectors::IsRegExpSpeciesLookupChainIntact(isolate)) return false;
// The smi check is required to omit ToLength(lastIndex) calls with possible
// user-code execution on the fast path.
diff --git a/chromium/v8/src/roots/roots.h b/chromium/v8/src/roots/roots.h
index 57eb2e4e976..db6583dead6 100644
--- a/chromium/v8/src/roots/roots.h
+++ b/chromium/v8/src/roots/roots.h
@@ -211,6 +211,7 @@ class Symbol;
V(PropertyCell, array_species_protector, ArraySpeciesProtector) \
V(PropertyCell, typed_array_species_protector, TypedArraySpeciesProtector) \
V(PropertyCell, promise_species_protector, PromiseSpeciesProtector) \
+ V(PropertyCell, regexp_species_protector, RegExpSpeciesProtector) \
V(PropertyCell, string_length_protector, StringLengthProtector) \
V(PropertyCell, array_iterator_protector, ArrayIteratorProtector) \
V(PropertyCell, array_buffer_detaching_protector, \
diff --git a/chromium/v8/tools/v8heapconst.py b/chromium/v8/tools/v8heapconst.py
index a05ebb9e404..75617b42c05 100644
--- a/chromium/v8/tools/v8heapconst.py
+++ b/chromium/v8/tools/v8heapconst.py
@@ -400,19 +400,20 @@ KNOWN_OBJECTS = {
("old_space", 0x004d9): "ArraySpeciesProtector",
("old_space", 0x004ed): "TypedArraySpeciesProtector",
("old_space", 0x00501): "PromiseSpeciesProtector",
- ("old_space", 0x00515): "StringLengthProtector",
- ("old_space", 0x00529): "ArrayIteratorProtector",
- ("old_space", 0x0053d): "ArrayBufferDetachingProtector",
- ("old_space", 0x00551): "PromiseHookProtector",
- ("old_space", 0x00565): "PromiseResolveProtector",
- ("old_space", 0x00579): "MapIteratorProtector",
- ("old_space", 0x0058d): "PromiseThenProtector",
- ("old_space", 0x005a1): "SetIteratorProtector",
- ("old_space", 0x005b5): "StringIteratorProtector",
- ("old_space", 0x005c9): "SingleCharacterStringCache",
- ("old_space", 0x009d1): "StringSplitCache",
- ("old_space", 0x00dd9): "RegExpMultipleCache",
- ("old_space", 0x011e1): "BuiltinsConstantsTable",
+ ("old_space", 0x00515): "RegExpSpeciesProtector",
+ ("old_space", 0x00529): "StringLengthProtector",
+ ("old_space", 0x0053d): "ArrayIteratorProtector",
+ ("old_space", 0x00551): "ArrayBufferDetachingProtector",
+ ("old_space", 0x00565): "PromiseHookProtector",
+ ("old_space", 0x00579): "PromiseResolveProtector",
+ ("old_space", 0x0058d): "MapIteratorProtector",
+ ("old_space", 0x005a1): "PromiseThenProtector",
+ ("old_space", 0x005b5): "SetIteratorProtector",
+ ("old_space", 0x005c9): "StringIteratorProtector",
+ ("old_space", 0x005dd): "SingleCharacterStringCache",
+ ("old_space", 0x009e5): "StringSplitCache",
+ ("old_space", 0x00ded): "RegExpMultipleCache",
+ ("old_space", 0x011f5): "BuiltinsConstantsTable",
}
# Lower 32 bits of first page addresses for various heap spaces.