[compiler] add more typer hardening Bug: v8:8806 Change-Id: I7ec9e29cfee7ce5a2e40016601df4e83fab84054 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4454339 Commit-Queue: Tobias Tebbi <tebbi@chromium.org> Reviewed-by: Samuel Groß <saelo@chromium.org> Cr-Commit-Position: refs/heads/main@{#88020}
diff --git a/src/compiler/js-call-reducer.cc b/src/compiler/js-call-reducer.cc index 1a1f2b5..034389e 100644 --- a/src/compiler/js-call-reducer.cc +++ b/src/compiler/js-call-reducer.cc
@@ -101,7 +101,8 @@ TNode<Smi> CheckSmi(TNode<Object> value); TNode<Number> CheckNumber(TNode<Object> value); TNode<String> CheckString(TNode<Object> value); - TNode<Number> CheckBounds(TNode<Number> value, TNode<Number> limit); + TNode<Number> CheckBounds(TNode<Number> value, TNode<Number> limit, + CheckBoundsFlags flags = {}); // Common operators. TNode<Smi> TypeGuardUnsignedSmall(TNode<Object> value); @@ -776,9 +777,11 @@ } TNode<Number> JSCallReducerAssembler::CheckBounds(TNode<Number> value, - TNode<Number> limit) { - return AddNode<Number>(graph()->NewNode(simplified()->CheckBounds(feedback()), - value, limit, effect(), control())); + TNode<Number> limit, + CheckBoundsFlags flags) { + return AddNode<Number>( + graph()->NewNode(simplified()->CheckBounds(feedback(), flags), value, + limit, effect(), control())); } TNode<Smi> JSCallReducerAssembler::TypeGuardUnsignedSmall(TNode<Object> value) { @@ -1161,6 +1164,10 @@ UndefinedConstant()); GotoIfNot(NumberLessThan(real_index_num, length), &out, UndefinedConstant()); + if (v8_flags.turbo_typer_hardening) { + real_index_num = CheckBounds(real_index_num, length, + CheckBoundsFlag::kAbortOnOutOfBounds); + } // Retrieving element at index. TNode<Object> element = LoadElement<Object>( @@ -5784,12 +5791,12 @@ Node* new_length = graph()->NewNode(simplified()->NumberSubtract(), 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); + if (v8_flags.turbo_typer_hardening) { + new_length = efalse = graph()->NewNode( + simplified()->CheckBounds(p.feedback(), + CheckBoundsFlag::kAbortOnOutOfBounds), + new_length, length, efalse, if_false); + } // Store the new {length} to the {receiver}. efalse = graph()->NewNode( @@ -5998,12 +6005,12 @@ Node* new_length = graph()->NewNode(simplified()->NumberSubtract(), 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); + if (v8_flags.turbo_typer_hardening) { + new_length = etrue1 = graph()->NewNode( + simplified()->CheckBounds(p.feedback(), + CheckBoundsFlag::kAbortOnOutOfBounds), + new_length, length, etrue1, if_true1); + } // Store the new {length} to the {receiver}. etrue1 = graph()->NewNode( @@ -6369,10 +6376,12 @@ { // This extra check exists to refine the type of {index} but also to break // an exploitation technique that abuses typer mismatches. - index = etrue = graph()->NewNode( - simplified()->CheckBounds(p.feedback(), - CheckBoundsFlag::kAbortOnOutOfBounds), - index, length, etrue, if_true); + if (v8_flags.turbo_typer_hardening) { + index = etrue = graph()->NewNode( + simplified()->CheckBounds(p.feedback(), + CheckBoundsFlag::kAbortOnOutOfBounds), + index, length, etrue, if_true); + } done_true = jsgraph()->FalseConstant(); if (iteration_kind == IterationKind::kKeys) {
diff --git a/src/compiler/js-native-context-specialization.cc b/src/compiler/js-native-context-specialization.cc index 1f8ffa6..896d612 100644 --- a/src/compiler/js-native-context-specialization.cc +++ b/src/compiler/js-native-context-specialization.cc
@@ -3263,11 +3263,14 @@ // Do a real bounds check against {length}. This is in order to // protect against a potential typer bug leading to the elimination of // the NumberLessThan above. - index = etrue = graph()->NewNode( - simplified()->CheckBounds( - FeedbackSource(), CheckBoundsFlag::kConvertStringAndMinusZero | - CheckBoundsFlag::kAbortOnOutOfBounds), - index, length, etrue, if_true); + if (v8_flags.turbo_typer_hardening) { + index = etrue = + graph()->NewNode(simplified()->CheckBounds( + FeedbackSource(), + CheckBoundsFlag::kConvertStringAndMinusZero | + CheckBoundsFlag::kAbortOnOutOfBounds), + index, length, etrue, if_true); + } // Perform the actual load vtrue = etrue = @@ -3682,12 +3685,14 @@ // Do a real bounds check against {length}. This is in order to // protect against a potential typer bug leading to the elimination // of the NumberLessThan above. - index = etrue = - graph()->NewNode(simplified()->CheckBounds( - FeedbackSource(), - CheckBoundsFlag::kConvertStringAndMinusZero | - CheckBoundsFlag::kAbortOnOutOfBounds), - index, length, etrue, if_true); + if (v8_flags.turbo_typer_hardening) { + index = etrue = graph()->NewNode( + simplified()->CheckBounds( + FeedbackSource(), + CheckBoundsFlag::kConvertStringAndMinusZero | + CheckBoundsFlag::kAbortOnOutOfBounds), + index, length, etrue, if_true); + } // Perform the actual load vtrue = etrue = graph()->NewNode( @@ -3762,12 +3767,14 @@ // Do a real bounds check against {length}. This is in order to // protect against a potential typer bug leading to the elimination // of the NumberLessThan above. - index = etrue = - graph()->NewNode(simplified()->CheckBounds( - FeedbackSource(), - CheckBoundsFlag::kConvertStringAndMinusZero | - CheckBoundsFlag::kAbortOnOutOfBounds), - index, length, etrue, if_true); + if (v8_flags.turbo_typer_hardening) { + index = etrue = graph()->NewNode( + simplified()->CheckBounds( + FeedbackSource(), + CheckBoundsFlag::kConvertStringAndMinusZero | + CheckBoundsFlag::kAbortOnOutOfBounds), + index, length, etrue, if_true); + } // Perform the actual store. etrue = graph()->NewNode( @@ -3842,11 +3849,14 @@ // Do a real bounds check against {length}. This is in order to protect // against a potential typer bug leading to the elimination of the // NumberLessThan above. - Node* etrue = index = graph()->NewNode( - simplified()->CheckBounds(FeedbackSource(), - CheckBoundsFlag::kConvertStringAndMinusZero | - CheckBoundsFlag::kAbortOnOutOfBounds), - index, length, *effect, if_true); + Node* etrue = *effect; + if (v8_flags.turbo_typer_hardening) { + etrue = index = graph()->NewNode( + simplified()->CheckBounds( + FeedbackSource(), CheckBoundsFlag::kConvertStringAndMinusZero | + CheckBoundsFlag::kAbortOnOutOfBounds), + index, length, etrue, if_true); + } Node* vtrue = etrue = graph()->NewNode(simplified()->StringCharCodeAt(), receiver, index, etrue, if_true); vtrue = graph()->NewNode(simplified()->StringFromSingleCharCode(), vtrue);
diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc index 89e07fc..10cc10b 100644 --- a/src/compiler/simplified-lowering.cc +++ b/src/compiler/simplified-lowering.cc
@@ -1890,7 +1890,12 @@ // The bounds check is redundant if we already know that // the index is within the bounds of [0.0, length[. // TODO(neis): Move this into TypedOptimization? - new_flags |= CheckBoundsFlag::kAbortOnOutOfBounds; + if (v8_flags.turbo_typer_hardening) { + new_flags |= CheckBoundsFlag::kAbortOnOutOfBounds; + } else { + DeferReplacement(node, NodeProperties::GetValueInput(node, 0)); + return; + } } ChangeOp(node, simplified()->CheckedUint32Bounds(feedback, new_flags));
diff --git a/src/compiler/typed-optimization.cc b/src/compiler/typed-optimization.cc index d23c5ac..7f0d3bd 100644 --- a/src/compiler/typed-optimization.cc +++ b/src/compiler/typed-optimization.cc
@@ -176,12 +176,17 @@ if (!index_type.IsNone() && !length_type.IsNone() && index_type.Max() < length_type.Min()) { - Node* check_bounds = graph()->NewNode( - simplified()->CheckBounds(FeedbackSource{}, - CheckBoundsFlag::kAbortOnOutOfBounds), - index, length, effect, control); - ReplaceWithValue(node, elements, check_bounds); - return Replace(check_bounds); + if (v8_flags.turbo_typer_hardening) { + Node* check_bounds = graph()->NewNode( + simplified()->CheckBounds(FeedbackSource{}, + CheckBoundsFlag::kAbortOnOutOfBounds), + index, length, effect, control); + ReplaceWithValue(node, elements, check_bounds); + return Replace(check_bounds); + } else { + RelaxEffectsAndControls(node); + return Replace(elements); + } } return NoChange();
diff --git a/src/flags/flag-definitions.h b/src/flags/flag-definitions.h index 5c6bf17..2d88f17 100644 --- a/src/flags/flag-definitions.h +++ b/src/flags/flag-definitions.h
@@ -1076,6 +1076,9 @@ DEFINE_BOOL(turbo_store_elimination, true, "enable store-store elimination in TurboFan") DEFINE_BOOL(trace_store_elimination, false, "trace store elimination") +DEFINE_BOOL_READONLY(turbo_typer_hardening, true, + "extra bounds checks to protect against some known typer " + "mismatch exploit techniques (best effort)") #if defined(V8_TARGET_ARCH_X64) || defined(V8_TARGET_ARCH_IA32) DEFINE_BOOL(turbo_rewrite_far_jumps, true,