Commit d4aafa40 authored by Sergei Glazunov's avatar Sergei Glazunov Committed by Commit Bot

[turbofan] Harden ArrayPrototypePop and ArrayPrototypeShift

An exploitation technique that abuses `pop` and `shift` to create a JS
array with a negative length was publicly disclosed some time ago.

Add extra checks to break the technique.

Bug: chromium:1198696
Change-Id: Ie008e9ae60bbdc3b25ca3a986d3cdc5e3cc00431
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2823707Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Commit-Queue: Sergei Glazunov <glazunov@google.com>
Cr-Commit-Position: refs/heads/master@{#73973}
parent e1cae86e
...@@ -5393,24 +5393,31 @@ Reduction JSCallReducer::ReduceArrayPrototypePop(Node* node) { ...@@ -5393,24 +5393,31 @@ Reduction JSCallReducer::ReduceArrayPrototypePop(Node* node) {
} }
// Compute the new {length}. // Compute the new {length}.
length = graph()->NewNode(simplified()->NumberSubtract(), length, Node* new_length = graph()->NewNode(simplified()->NumberSubtract(),
jsgraph()->OneConstant()); length, jsgraph()->OneConstant());
// This extra check exists solely to break an exploitation technique
// that abuses typer mismatches.
new_length = efalse = graph()->NewNode(
simplified()->CheckBounds(p.feedback(),
CheckBoundsFlag::kAbortOnOutOfBounds),
new_length, length, efalse, if_false);
// Store the new {length} to the {receiver}. // Store the new {length} to the {receiver}.
efalse = graph()->NewNode( efalse = graph()->NewNode(
simplified()->StoreField(AccessBuilder::ForJSArrayLength(kind)), simplified()->StoreField(AccessBuilder::ForJSArrayLength(kind)),
receiver, length, efalse, if_false); receiver, new_length, efalse, if_false);
// Load the last entry from the {elements}. // Load the last entry from the {elements}.
vfalse = efalse = graph()->NewNode( vfalse = efalse = graph()->NewNode(
simplified()->LoadElement(AccessBuilder::ForFixedArrayElement(kind)), simplified()->LoadElement(AccessBuilder::ForFixedArrayElement(kind)),
elements, length, efalse, if_false); elements, new_length, efalse, if_false);
// Store a hole to the element we just removed from the {receiver}. // Store a hole to the element we just removed from the {receiver}.
efalse = graph()->NewNode( efalse = graph()->NewNode(
simplified()->StoreElement( simplified()->StoreElement(
AccessBuilder::ForFixedArrayElement(GetHoleyElementsKind(kind))), AccessBuilder::ForFixedArrayElement(GetHoleyElementsKind(kind))),
elements, length, jsgraph()->TheHoleConstant(), efalse, if_false); elements, new_length, jsgraph()->TheHoleConstant(), efalse, if_false);
} }
control = graph()->NewNode(common()->Merge(2), if_true, if_false); control = graph()->NewNode(common()->Merge(2), if_true, if_false);
...@@ -5586,19 +5593,27 @@ Reduction JSCallReducer::ReduceArrayPrototypeShift(Node* node) { ...@@ -5586,19 +5593,27 @@ Reduction JSCallReducer::ReduceArrayPrototypeShift(Node* node) {
} }
// Compute the new {length}. // Compute the new {length}.
length = graph()->NewNode(simplified()->NumberSubtract(), length, Node* new_length = graph()->NewNode(simplified()->NumberSubtract(),
jsgraph()->OneConstant()); length, jsgraph()->OneConstant());
// This extra check exists solely to break an exploitation technique
// that abuses typer mismatches.
new_length = etrue1 = graph()->NewNode(
simplified()->CheckBounds(p.feedback(),
CheckBoundsFlag::kAbortOnOutOfBounds),
new_length, length, etrue1, if_true1);
// Store the new {length} to the {receiver}. // Store the new {length} to the {receiver}.
etrue1 = graph()->NewNode( etrue1 = graph()->NewNode(
simplified()->StoreField(AccessBuilder::ForJSArrayLength(kind)), simplified()->StoreField(AccessBuilder::ForJSArrayLength(kind)),
receiver, length, etrue1, if_true1); receiver, new_length, etrue1, if_true1);
// Store a hole to the element we just removed from the {receiver}. // Store a hole to the element we just removed from the {receiver}.
etrue1 = graph()->NewNode( etrue1 = graph()->NewNode(
simplified()->StoreElement(AccessBuilder::ForFixedArrayElement( simplified()->StoreElement(AccessBuilder::ForFixedArrayElement(
GetHoleyElementsKind(kind))), GetHoleyElementsKind(kind))),
elements, length, jsgraph()->TheHoleConstant(), etrue1, if_true1); elements, new_length, jsgraph()->TheHoleConstant(), etrue1,
if_true1);
} }
Node* if_false1 = graph()->NewNode(common()->IfFalse(), branch1); Node* if_false1 = graph()->NewNode(common()->IfFalse(), branch1);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment