[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/+/2823707
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Sergei Glazunov <glazunov@google.com>
Cr-Commit-Position: refs/heads/master@{#73973}
diff --git a/src/compiler/js-call-reducer.cc b/src/compiler/js-call-reducer.cc
index 1a56f79..64fd85f 100644
--- a/src/compiler/js-call-reducer.cc
+++ b/src/compiler/js-call-reducer.cc
@@ -5393,24 +5393,31 @@
       }
 
       // Compute the new {length}.
-      length = graph()->NewNode(simplified()->NumberSubtract(), length,
-                                jsgraph()->OneConstant());
+      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);
 
       // Store the new {length} to the {receiver}.
       efalse = graph()->NewNode(
           simplified()->StoreField(AccessBuilder::ForJSArrayLength(kind)),
-          receiver, length, efalse, if_false);
+          receiver, new_length, efalse, if_false);
 
       // Load the last entry from the {elements}.
       vfalse = efalse = graph()->NewNode(
           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}.
       efalse = graph()->NewNode(
           simplified()->StoreElement(
               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);
@@ -5586,19 +5593,27 @@
         }
 
         // Compute the new {length}.
-        length = graph()->NewNode(simplified()->NumberSubtract(), length,
-                                  jsgraph()->OneConstant());
+        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);
 
         // Store the new {length} to the {receiver}.
         etrue1 = graph()->NewNode(
             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}.
         etrue1 = graph()->NewNode(
             simplified()->StoreElement(AccessBuilder::ForFixedArrayElement(
                 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);