Check interrupts in runtime BigInt parser

The BigInt constructor has quadratic complexity while parsing strings,
and the input is unbounded. Interrupts should be checked during this
operation to ensure the embedder has control over runaway execution.

since the implicit cast from string may now throw.

BigInt: :CompareToString and BigInt::EqualToString now return Maybe<..>
Change-Id: Iccb85fafac4df69075a34d1de647cb4f0184cb12
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2392629Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69720}

AUTHORS

@@ -135,6 +135,7 @@ Loo Rong Jie <loorongjie@gmail.com>
 Luis Reis <luis.m.reis@gmail.com>
 Luke Zarko <lukezarko@gmail.com>
 Maciej Małecki <me@mmalecki.com>
+Marcel Laverdet <marcel@laverdet.com>
 Marcin Cieślak <saper@marcincieslak.com>
 Marcin Wiącek <marcin@mwiacek.com>
 Martin Bidlingmaier <martin.bidlingmaier@gmail.com>

src/numbers/conversions.cc

@@ -209,6 +209,7 @@ class StringToIntHelper {
   void ParseInt();
 
   // Subclasses may override this.
+  virtual bool CheckTermination() { return false; }
   virtual void HandleSpecialCases() {}
 
   // Subclass constructors should call these for configuration before calling
@@ -249,7 +250,7 @@ class StringToIntHelper {
   template <class Char>
   void DetectRadixInternal(Char current, int length);
   template <class Char>
-  void ParseInternal(Char start);
+  bool ParseChunkInternal(Char start);
 
   LocalIsolate* isolate_;
   Handle<String> subject_;
@@ -280,18 +281,31 @@ void StringToIntHelper<LocalIsolate>::ParseInt() {
   AllocateResult();
   HandleSpecialCases();
   if (state_ != State::kRunning) return;
-  {
-    DisallowHeapAllocation no_gc;
-    if (IsOneByte()) {
-      Vector<const uint8_t> vector = GetOneByteVector();
-      DCHECK_EQ(length_, vector.length());
-      ParseInternal(vector.begin());
-    } else {
-      Vector<const uc16> vector = GetTwoByteVector();
-      DCHECK_EQ(length_, vector.length());
-      ParseInternal(vector.begin());
+  do {
+    {
+      DisallowHeapAllocation no_gc;
+      if (IsOneByte()) {
+        Vector<const uint8_t> vector = GetOneByteVector();
+        DCHECK_EQ(length_, vector.length());
+        if (ParseChunkInternal(vector.begin())) {
+          break;
+        }
+      } else {
+        Vector<const uc16> vector = GetTwoByteVector();
+        DCHECK_EQ(length_, vector.length());
+        if (ParseChunkInternal(vector.begin())) {
+          break;
+        }
+      }
     }
-  }
+
+    // The flat vector handle is temporarily released after parsing 10kb
+    // in order to invoke interrupts which may in turn invoke GC.
+    if (CheckTermination()) {
+      set_state(State::kError);
+      break;
+    }
+  } while (true);
   DCHECK_NE(state_, State::kRunning);
 }
 
@@ -377,9 +391,11 @@ void StringToIntHelper<LocalIsolate>::DetectRadixInternal(Char current,
 
 template <typename LocalIsolate>
 template <class Char>
-void StringToIntHelper<LocalIsolate>::ParseInternal(Char start) {
+bool StringToIntHelper<LocalIsolate>::ParseChunkInternal(Char start) {
+  const int kChunkSize = 10240;
   Char current = start + cursor_;
   Char end = start + length_;
+  Char break_pos = current + kChunkSize;
 
   // The following code causes accumulating rounding error for numbers greater
   // than ~2^56. It's explicitly allowed in the spec: "if R is not 2, 4, 8, 10,
@@ -433,13 +449,20 @@ void StringToIntHelper<LocalIsolate>::ParseInternal(Char start) {
 
     // Update the value and skip the part in the string.
     ResultMultiplyAdd(multiplier, part);
-  } while (!done);
 
-  if (!allow_trailing_junk_ && AdvanceToNonspace(&current, end)) {
-    return set_state(State::kJunk);
-  }
+    // Set final state
+    if (done) {
+      if (!allow_trailing_junk_ && AdvanceToNonspace(&current, end)) {
+        set_state(State::kJunk);
+      } else {
+        set_state(State::kDone);
+      }
+      return true;
+    }
+  } while (current < break_pos);
 
-  return set_state(State::kDone);
+  cursor_ = static_cast<int>(current - start);
+  return false;
 }
 
 class NumberParseIntHelper : public StringToIntHelper<Isolate> {
@@ -904,6 +927,8 @@ class StringToBigIntHelper : public StringToIntHelper<LocalIsolate> {
                                static_cast<uintptr_t>(part));
   }
 
+  bool CheckTermination() override;
+
   AllocationType allocation_type() {
     // For literals, we pretenure the allocated BigInt, since it's about
     // to be stored in the interpreter's constants array.
@@ -916,6 +941,18 @@ class StringToBigIntHelper : public StringToIntHelper<LocalIsolate> {
   Behavior behavior_;
 };
 
+template <typename LocalIsolate>
+bool StringToBigIntHelper<LocalIsolate>::CheckTermination() {
+  return false;
+}
+
+template <>
+bool StringToBigIntHelper<Isolate>::CheckTermination() {
+  StackLimitCheck interrupt_check(isolate());
+  return interrupt_check.InterruptRequested() &&
+         isolate()->stack_guard()->HandleInterrupts().IsException(isolate());
+}
+
 MaybeHandle<BigInt> StringToBigInt(Isolate* isolate, Handle<String> string) {
   string = String::Flatten(isolate, string);
   StringToBigIntHelper<Isolate> helper(isolate, string);

src/objects/bigint.cc

@@ -822,32 +822,39 @@ MaybeHandle<BigInt> BigInt::Decrement(Isolate* isolate, Handle<BigInt> x) {
   return MutableBigInt::MakeImmutable(result);
 }
 
-ComparisonResult BigInt::CompareToString(Isolate* isolate, Handle<BigInt> x,
-                                         Handle<String> y) {
+Maybe<ComparisonResult> BigInt::CompareToString(Isolate* isolate,
+                                                Handle<BigInt> x,
+                                                Handle<String> y) {
   // a. Let ny be StringToBigInt(y);
   MaybeHandle<BigInt> maybe_ny = StringToBigInt(isolate, y);
   // b. If ny is NaN, return undefined.
   Handle<BigInt> ny;
   if (!maybe_ny.ToHandle(&ny)) {
-    DCHECK(!isolate->has_pending_exception());
-    return ComparisonResult::kUndefined;
+    if (isolate->has_pending_exception()) {
+      return Nothing<ComparisonResult>();
+    } else {
+      return Just(ComparisonResult::kUndefined);
+    }
   }
   // c. Return BigInt::lessThan(x, ny).
-  return CompareToBigInt(x, ny);
+  return Just(CompareToBigInt(x, ny));
 }
 
-bool BigInt::EqualToString(Isolate* isolate, Handle<BigInt> x,
-                           Handle<String> y) {
+Maybe<bool> BigInt::EqualToString(Isolate* isolate, Handle<BigInt> x,
+                                  Handle<String> y) {
   // a. Let n be StringToBigInt(y).
   MaybeHandle<BigInt> maybe_n = StringToBigInt(isolate, y);
   // b. If n is NaN, return false.
   Handle<BigInt> n;
   if (!maybe_n.ToHandle(&n)) {
-    DCHECK(!isolate->has_pending_exception());
-    return false;
+    if (isolate->has_pending_exception()) {
+      return Nothing<bool>();
+    } else {
+      return Just(false);
+    }
   }
   // c. Return the result of x == n.
-  return EqualToBigInt(*x, *n);
+  return Just(EqualToBigInt(*x, *n));
 }
 
 bool BigInt::EqualToNumber(Handle<BigInt> x, Handle<Object> y) {
@@ -1047,9 +1054,13 @@ MaybeHandle<BigInt> BigInt::FromObject(Isolate* isolate, Handle<Object> obj) {
   if (obj->IsString()) {
     Handle<BigInt> n;
     if (!StringToBigInt(isolate, Handle<String>::cast(obj)).ToHandle(&n)) {
-      THROW_NEW_ERROR(isolate,
-                      NewSyntaxError(MessageTemplate::kBigIntFromObject, obj),
-                      BigInt);
+      if (isolate->has_pending_exception()) {
+        return MaybeHandle<BigInt>();
+      } else {
+        THROW_NEW_ERROR(isolate,
+                        NewSyntaxError(MessageTemplate::kBigIntFromObject, obj),
+                        BigInt);
+      }
     }
     return n;
   }

src/objects/bigint.h

@@ -189,11 +189,12 @@ class BigInt : public BigIntBase {
 
   bool IsNegative() const { return sign(); }
 
-  static bool EqualToString(Isolate* isolate, Handle<BigInt> x,
-                            Handle<String> y);
+  static Maybe<bool> EqualToString(Isolate* isolate, Handle<BigInt> x,
+                                   Handle<String> y);
   static bool EqualToNumber(Handle<BigInt> x, Handle<Object> y);
-  static ComparisonResult CompareToString(Isolate* isolate, Handle<BigInt> x,
-                                          Handle<String> y);
+  static Maybe<ComparisonResult> CompareToString(Isolate* isolate,
+                                                 Handle<BigInt> x,
+                                                 Handle<String> y);
   static ComparisonResult CompareToNumber(Handle<BigInt> x, Handle<Object> y);
   // Exposed for tests, do not call directly. Use CompareToNumber() instead.
   V8_EXPORT_PRIVATE static ComparisonResult CompareToDouble(Handle<BigInt> x,

src/objects/objects.cc

@@ -671,12 +671,18 @@ Maybe<ComparisonResult> Object::Compare(Isolate* isolate, Handle<Object> x,
                                 Handle<String>::cast(y)));
   }
   if (x->IsBigInt() && y->IsString()) {
-    return Just(BigInt::CompareToString(isolate, Handle<BigInt>::cast(x),
-                                        Handle<String>::cast(y)));
+    return BigInt::CompareToString(isolate, Handle<BigInt>::cast(x),
+                                   Handle<String>::cast(y));
   }
   if (x->IsString() && y->IsBigInt()) {
-    return Just(Reverse(BigInt::CompareToString(
-        isolate, Handle<BigInt>::cast(y), Handle<String>::cast(x))));
+    Maybe<ComparisonResult> maybe_result = BigInt::CompareToString(
+        isolate, Handle<BigInt>::cast(y), Handle<String>::cast(x));
+    ComparisonResult result;
+    if (maybe_result.To(&result)) {
+      return Just(Reverse(result));
+    } else {
+      return Nothing<ComparisonResult>();
+    }
   }
   // ES6 section 7.2.11 Abstract Relational Comparison step 6.
   if (!Object::ToNumeric(isolate, x).ToHandle(&x) ||
@@ -735,8 +741,8 @@ Maybe<bool> Object::Equals(Isolate* isolate, Handle<Object> x,
         return Just(
             StrictNumberEquals(*x, Handle<Oddball>::cast(y)->to_number()));
       } else if (y->IsBigInt()) {
-        return Just(BigInt::EqualToString(isolate, Handle<BigInt>::cast(y),
-                                          Handle<String>::cast(x)));
+        return BigInt::EqualToString(isolate, Handle<BigInt>::cast(y),
+                                     Handle<String>::cast(x));
       } else if (y->IsJSReceiver()) {
         if (!JSReceiver::ToPrimitive(Handle<JSReceiver>::cast(y))
                  .ToHandle(&y)) {

src/runtime/runtime-bigint.cc

@@ -39,9 +39,11 @@ RUNTIME_FUNCTION(Runtime_BigIntCompareToString) {
   CONVERT_ARG_HANDLE_CHECKED(Smi, mode, 0);
   CONVERT_ARG_HANDLE_CHECKED(BigInt, lhs, 1);
   CONVERT_ARG_HANDLE_CHECKED(String, rhs, 2);
-  bool result =
-      ComparisonResultToBool(static_cast<Operation>(mode->value()),
-                             BigInt::CompareToString(isolate, lhs, rhs));
+  Maybe<ComparisonResult> maybe_result =
+      BigInt::CompareToString(isolate, lhs, rhs);
+  MAYBE_RETURN(maybe_result, ReadOnlyRoots(isolate).exception());
+  bool result = ComparisonResultToBool(static_cast<Operation>(mode->value()),
+                                       maybe_result.FromJust());
   return *isolate->factory()->ToBoolean(result);
 }
 
@@ -68,8 +70,9 @@ RUNTIME_FUNCTION(Runtime_BigIntEqualToString) {
   DCHECK_EQ(2, args.length());
   CONVERT_ARG_HANDLE_CHECKED(BigInt, lhs, 0);
   CONVERT_ARG_HANDLE_CHECKED(String, rhs, 1);
-  bool result = BigInt::EqualToString(isolate, lhs, rhs);
-  return *isolate->factory()->ToBoolean(result);
+  Maybe<bool> maybe_result = BigInt::EqualToString(isolate, lhs, rhs);
+  MAYBE_RETURN(maybe_result, ReadOnlyRoots(isolate).exception());
+  return *isolate->factory()->ToBoolean(maybe_result.FromJust());
 }
 
 RUNTIME_FUNCTION(Runtime_BigIntToBoolean) {