Skip to content

Commit

Permalink
[Backport] CVE-2023-3079: Type Confusion in V8
Browse files Browse the repository at this point in the history
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/v8/v8/+/4590637:
Fix store handler selection for arguments objects

M108 merge issues:
    src/diagnostics/objects-printer.cc:
      Type conflicts for the handler variable on and
      the IsWeakFixedArray() check isn't present in 108; kept
      the code changes from the fix.

Drive-by: fix printing of handlers in --trace-feedback-updates mode.

(cherry picked from commit e144f3b71e64e01d6ffd247eb15ca1ff56f6287b)

Bug: chromium:1450481
Change-Id: I1c0084701f7f8959da508481cab7a81a2bca3c8d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4584248
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#88021}
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4590637
Commit-Queue: Roger Felipe Zanoni da Silva <rzanoni@google.com>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/branch-heads/10.8@{#66}
Cr-Branched-From: f1bc03fd6b4c201abd9f0fd9d51fb989150f97b9-refs/heads/10.8.168@{#1}
Cr-Branched-From: 237de893e1c0a0628a57d0f5797483d3add7f005-refs/heads/main@{#83672}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/486078
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
  • Loading branch information
isheludko authored and mibrunin committed Jun 20, 2023
1 parent fd7386d commit cd7492c
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 7 deletions.
14 changes: 12 additions & 2 deletions chromium/v8/src/diagnostics/objects-printer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1308,8 +1308,18 @@ void FeedbackNexus::Print(std::ostream& os) {
case FeedbackSlotKind::kSetKeyedStrict: {
os << InlineCacheState2String(ic_state());
if (ic_state() == InlineCacheState::MONOMORPHIC) {
os << "\n " << Brief(GetFeedback()) << ": ";
StoreHandler::PrintHandler(GetFeedbackExtra().GetHeapObjectOrSmi(), os);
HeapObject feedback = GetFeedback().GetHeapObject();
HeapObject feedback_extra = GetFeedbackExtra().GetHeapObject();
if (feedback.IsName()) {
os << " with name " << Brief(feedback);
WeakFixedArray array = WeakFixedArray::cast(feedback_extra);
os << "\n " << Brief(array.Get(0)) << ": ";
Object handler = array.Get(1).GetHeapObjectOrSmi();
StoreHandler::PrintHandler(handler, os);
} else {
os << "\n " << Brief(feedback) << ": ";
StoreHandler::PrintHandler(feedback_extra, os);
}
} else if (ic_state() == InlineCacheState::POLYMORPHIC) {
WeakFixedArray array =
WeakFixedArray::cast(GetFeedback().GetHeapObject());
Expand Down
5 changes: 4 additions & 1 deletion chromium/v8/src/ic/handler-configuration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -584,8 +584,11 @@ void StoreHandler::PrintHandler(Object handler, std::ostream& os) {
os << ", validity cell = ";
store_handler.validity_cell().ShortPrint(os);
os << ")" << std::endl;
} else if (handler.IsMap()) {
os << "StoreHandler(field transition to " << Brief(handler) << ")"
<< std::endl;
} else {
os << "StoreHandler(<unexpected>)(" << Brief(handler) << ")";
os << "StoreHandler(<unexpected>)(" << Brief(handler) << ")" << std::endl;
}
}

Expand Down
16 changes: 12 additions & 4 deletions chromium/v8/src/ic/ic.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2296,10 +2296,18 @@ Handle<Object> KeyedStoreIC::StoreElementHandler(
receiver_map->has_sealed_elements() ||
receiver_map->has_nonextensible_elements() ||
receiver_map->has_typed_array_or_rab_gsab_typed_array_elements()) {
// TODO(jgruber): Update counter name.
TRACE_HANDLER_STATS(isolate(), KeyedStoreIC_StoreFastElementStub);
code = StoreHandler::StoreFastElementBuiltin(isolate(), store_mode);
if (receiver_map->has_typed_array_or_rab_gsab_typed_array_elements()) {
return code;
if (receiver_map->IsJSArgumentsObjectMap() &&
receiver_map->has_fast_packed_elements()) {
// Allow fast behaviour for in-bounds stores while making it miss and
// properly handle the out of bounds store case.
code = StoreHandler::StoreFastElementBuiltin(isolate(), STANDARD_STORE);
} else {
code = StoreHandler::StoreFastElementBuiltin(isolate(), store_mode);
if (receiver_map->has_typed_array_or_rab_gsab_typed_array_elements()) {
return code;
}
}
} else if (IsStoreInArrayLiteralIC()) {
// TODO(jgruber): Update counter name.
Expand All @@ -2310,7 +2318,7 @@ Handle<Object> KeyedStoreIC::StoreElementHandler(
TRACE_HANDLER_STATS(isolate(), KeyedStoreIC_StoreElementStub);
DCHECK(DICTIONARY_ELEMENTS == receiver_map->elements_kind() ||
receiver_map->has_frozen_elements());
code = StoreHandler::StoreSlow(isolate(), store_mode);
return StoreHandler::StoreSlow(isolate(), store_mode);
}

if (IsAnyDefineOwn() || IsStoreInArrayLiteralIC()) return code;
Expand Down
4 changes: 4 additions & 0 deletions chromium/v8/src/objects/map-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,10 @@ bool Map::has_fast_elements() const {
return IsFastElementsKind(elements_kind());
}

bool Map::has_fast_packed_elements() const {
return IsFastPackedElementsKind(elements_kind());
}

bool Map::has_sloppy_arguments_elements() const {
return IsSloppyArgumentsElementsKind(elements_kind());
}
Expand Down
1 change: 1 addition & 0 deletions chromium/v8/src/objects/map.h
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ class Map : public TorqueGeneratedMap<Map, HeapObject> {
inline bool has_fast_smi_or_object_elements() const;
inline bool has_fast_double_elements() const;
inline bool has_fast_elements() const;
inline bool has_fast_packed_elements() const;
inline bool has_sloppy_arguments_elements() const;
inline bool has_fast_sloppy_arguments_elements() const;
inline bool has_fast_string_wrapper_elements() const;
Expand Down

0 comments on commit cd7492c

Please sign in to comment.