From 59c82b5167720172245e2471c0b3dd52a202edca Mon Sep 17 00:00:00 2001 From: Georg Neis Date: Tue, 20 Apr 2021 18:19:42 +0200 Subject: [PATCH] [Backport] CVE-2021-30513: Type Confusion in V8 Cherry-pick of commit originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/2883780: Reland "[compiler] Fix more truncation bugs in SimplifiedLowering" This is a reland of 47077d94492cb604e3a7f02c0d7c3c495ff6b713 without changes. The revert was false alarm. [M86]: Resolved simple conflicts. Original change's description: > [compiler] Fix more truncation bugs in SimplifiedLowering > > Bug: chromium:1200490 > Change-Id: I3555b6d99bdb4b4e7c302a43a82c17e8bff84ebe > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2840452 > Reviewed-by: Nico Hartmann > Commit-Queue: Georg Neis > Cr-Commit-Position: refs/heads/master@{#74097} (cherry picked from commit e4a580c9104e42968e8e13b8c7d933f0b2eda2a3) (cherry picked from commit 97ad04543438f7b235b21346fdd198f81028cd5e) Bug: chromium:1200490 Tbr: nicohartmann@chromium.org No-Try: true No-Presubmit: true No-Tree-Checks: true Change-Id: Iedddcf2d0117fa59dc9d7a3604ef203808ad2903 Reviewed-by: Georg Neis Commit-Queue: Georg Neis Cr-Original-Commit-Position: refs/branch-heads/9.0@{#47} Cr-Original-Branched-From: bd0108b4c88e0d6f2350cb79b5f363fbd02f3eb7-refs/heads/9.0.257@{#1} Cr-Original-Branched-From: 349bcc6a075411f1a7ce2d866c3dfeefc2efa39d-refs/heads/master@{#73001} Reviewed-by: Jana Grill Commit-Queue: Victor-Gabriel Savu Cr-Commit-Position: refs/branch-heads/8.6@{#95} Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1} Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472} Reviewed-by: Allan Sandfeld Jensen --- .../v8/src/compiler/simplified-lowering.cc | 69 ++++++++++++------- 1 file changed, 43 insertions(+), 26 deletions(-) diff --git a/chromium/v8/src/compiler/simplified-lowering.cc b/chromium/v8/src/compiler/simplified-lowering.cc index c2f0d744c2f..a14dd717852 100644 --- a/chromium/v8/src/compiler/simplified-lowering.cc +++ b/chromium/v8/src/compiler/simplified-lowering.cc @@ -1398,17 +1398,32 @@ class RepresentationSelector { return jsgraph_->simplified(); } - void LowerToCheckedInt32Mul(Node* node, Truncation truncation, - Type input0_type, Type input1_type) { - // If one of the inputs is positive and/or truncation is being applied, - // there is no need to return -0. - CheckForMinusZeroMode mz_mode = - truncation.IdentifiesZeroAndMinusZero() || - IsSomePositiveOrderedNumber(input0_type) || - IsSomePositiveOrderedNumber(input1_type) - ? CheckForMinusZeroMode::kDontCheckForMinusZero - : CheckForMinusZeroMode::kCheckForMinusZero; - NodeProperties::ChangeOp(node, simplified()->CheckedInt32Mul(mz_mode)); + template + void VisitForCheckedInt32Mul(Node* node, Truncation truncation, + Type input0_type, Type input1_type, + UseInfo input_use) { + DCHECK_EQ(node->opcode(), IrOpcode::kSpeculativeNumberMultiply); + // A -0 input is impossible or will cause a deopt. + DCHECK(BothInputsAre(node, Type::Signed32()) || + !input_use.truncation().IdentifiesZeroAndMinusZero()); + + CheckForMinusZeroMode mz_mode; + Type restriction; + if (IsSomePositiveOrderedNumber(input0_type) || + IsSomePositiveOrderedNumber(input1_type)) { + mz_mode = CheckForMinusZeroMode::kDontCheckForMinusZero; + restriction = Type::Signed32(); + } else if (truncation.IdentifiesZeroAndMinusZero()) { + mz_mode = CheckForMinusZeroMode::kDontCheckForMinusZero; + restriction = Type::Signed32OrMinusZero(); + } else { + mz_mode = CheckForMinusZeroMode::kCheckForMinusZero; + restriction = Type::Signed32(); + } + + VisitBinop(node, input_use, MachineRepresentation::kWord32, restriction); + if (lower()) + NodeProperties::ChangeOp(node, simplified()->CheckedInt32Mul(mz_mode)); } void ChangeToInt32OverflowOp(Node* node) { @@ -1600,12 +1615,22 @@ class RepresentationSelector { VisitBinop(node, lhs_use, rhs_use, MachineRepresentation::kWord32); if (lower()) DeferReplacement(node, lowering->Int32Mod(node)); } else if (BothInputsAre(node, Type::Unsigned32OrMinusZeroOrNaN())) { + Type const restriction = + truncation.IdentifiesZeroAndMinusZero() && + TypeOf(node->InputAt(0)).Maybe(Type::MinusZero()) + ? Type::Unsigned32OrMinusZero() + : Type::Unsigned32(); VisitBinop(node, lhs_use, rhs_use, MachineRepresentation::kWord32, - Type::Unsigned32()); + restriction); if (lower()) ChangeToUint32OverflowOp(node); } else { + Type const restriction = + truncation.IdentifiesZeroAndMinusZero() && + TypeOf(node->InputAt(0)).Maybe(Type::MinusZero()) + ? Type::Signed32OrMinusZero() + : Type::Signed32(); VisitBinop(node, lhs_use, rhs_use, MachineRepresentation::kWord32, - Type::Signed32()); + restriction); if (lower()) ChangeToInt32OverflowOp(node); } return; @@ -2165,23 +2190,17 @@ class RepresentationSelector { // If both inputs and feedback are int32, use the overflow op. if (hint == NumberOperationHint::kSignedSmall || hint == NumberOperationHint::kSigned32) { - VisitBinop(node, UseInfo::TruncatingWord32(), - MachineRepresentation::kWord32, Type::Signed32()); - if (lower()) { - LowerToCheckedInt32Mul(node, truncation, input0_type, - input1_type); - } + VisitForCheckedInt32Mul(node, truncation, input0_type, + input1_type, + UseInfo::TruncatingWord32()); return; } } if (hint == NumberOperationHint::kSignedSmall || hint == NumberOperationHint::kSigned32) { - VisitBinop(node, CheckedUseInfoAsWord32FromHint(hint), - MachineRepresentation::kWord32, Type::Signed32()); - if (lower()) { - LowerToCheckedInt32Mul(node, truncation, input0_type, input1_type); - } + VisitForCheckedInt32Mul(node, truncation, input0_type, input1_type, + CheckedUseInfoAsWord32FromHint(hint)); return; } @@ -3901,7 +3920,6 @@ template <> void RepresentationSelector::SetOutput( Node* node, MachineRepresentation representation, Type restriction_type) { NodeInfo* const info = GetInfo(node); - DCHECK(info->restriction_type().Is(restriction_type)); DCHECK(restriction_type.Is(info->restriction_type())); info->set_output(representation); } @@ -3911,7 +3929,6 @@ void RepresentationSelector::SetOutput( Node* node, MachineRepresentation representation, Type restriction_type) { NodeInfo* const info = GetInfo(node); DCHECK_EQ(info->representation(), representation); - DCHECK(info->restriction_type().Is(restriction_type)); DCHECK(restriction_type.Is(info->restriction_type())); USE(info); }