Catch potential integer overflows in bigint operations.

runtime/vm/bigint_operations.cc

Index: runtime/vm/bigint_operations.cc
diff --git a/runtime/vm/bigint_operations.cc b/runtime/vm/bigint_operations.cc
index b7c7a69ff6ff2ea7e4b2e4c1b6ab191066b233eb..db9eacf956e86c138e97fa639f2378acbe2c318f 100644
--- a/runtime/vm/bigint_operations.cc
+++ b/runtime/vm/bigint_operations.cc
@@ -2,6 +2,7 @@
 
 #include "vm/bigint_operations.h"
 
+#include "platform/assert.h"
 #include "platform/utils.h"
 
 #include "vm/double_internals.h"
@@ -106,7 +107,10 @@ RawBigint* BigintOperations::NewFromCString(const char* str,
     return result.raw();
   }
 
-  intptr_t str_length = strlen(str);
+  const intptr_t str_length = strlen(str);
+  if (str_length < 0) {
+    FATAL("Fatal error in BigintOperations::NewFromCString: string too long");
+  }
   if ((str_length > 2) &&
       (str[0] == '0') &&
       ((str[1] == 'x') || (str[1] == 'X'))) {
@@ -121,7 +125,11 @@ RawBigint* BigintOperations::NewFromCString(const char* str,
 
 intptr_t BigintOperations::ComputeChunkLength(const char* hex_string) {
   ASSERT(kDigitBitSize % 4 == 0);
-  intptr_t hex_length = strlen(hex_string);
+  const intptr_t hex_length = strlen(hex_string);
+  if (hex_length < 0) {
+    FATAL("Fatal error in BigintOperations::ComputeChunkLength: "
+          "string too long");
+  }
   // Round up.
   intptr_t bigint_length = ((hex_length - 1) / kHexCharsPerDigit) + 1;
   return bigint_length;
@@ -158,7 +166,11 @@ RawBigint* BigintOperations::FromDecimalCString(const char* str,
   const Chunk kTenMultiplier = 100000000;
   ASSERT(kDigitBitSize >= 27);
 
-  intptr_t str_length = strlen(str);
+  const intptr_t str_length = strlen(str);
+  if (str_length < 0) {
+    FATAL("Fatal error in BigintOperations::FromDecimalCString: "
+          "string too long");
+  }
   intptr_t str_pos = 0;
 
   // Read first digit separately. This avoids a multiplication and addition.
@@ -251,7 +263,7 @@ const char* BigintOperations::ToHexCString(intptr_t length,
   Chunk* chunk_data = reinterpret_cast<Chunk*>(data);
   if (length == 0) {
     const char* zero = "0x0";
-    const int kLength = strlen(zero);
+    const intptr_t kLength = strlen(zero);
     char* result = reinterpret_cast<char*>(allocator(kLength + 1));
     ASSERT(result != NULL);
     memmove(result, zero, kLength);
@@ -278,6 +290,9 @@ const char* BigintOperations::ToHexCString(intptr_t length,
   required_size += leading_hex_digits;
   required_size += (chunk_length - 1) * kHexCharsPerDigit;
   required_size++;  // For the trailing '\0'.
+  if (required_size < 0) {
+    FATAL("Fatal error in BigintOperations::ToHexCString: string too long");
+  }

[siva, 2013/09/17 00:19:52]

As discussed offline the overflow could happen before the last computation
making it positive.

We should probably compute a kMaxChunkLen and compare chunk_length against that
and error out.

[Michael Lippautz (Google), 2013/09/17 03:36:50]

Done.

   char* result = reinterpret_cast<char*>(allocator(required_size));
   // Print the number into the string.
   // Start from the last position.
@@ -351,7 +366,10 @@ const char* BigintOperations::ToDecimalCString(
   if (bigint.IsNegative()) {
     required_size++;
   }
-  ASSERT(required_size == static_cast<intptr_t>(required_size));
+  if (required_size != static_cast<intptr_t>(required_size)) {

[siva, 2013/09/17 00:19:52]

Ditto comment here too.

[Michael Lippautz (Google), 2013/09/17 03:36:50]

Removed the check, because it's already there (as far as I see) :)
line 345: kMaxAllowedDigitLength (conservative estimate)

One more thing: The code here (line 350+) throws an OOM exception, while the new
checks use the FATAL() macro. Change the new checks to throw an OOM exception,
or leave them?

+    FATAL("Fatal error in BigintOperations::ToDecimalCString: "
+          "string too long");
+  }
   // We will fill the result in the inverse order and then exchange at the end.
   char* result =
       reinterpret_cast<char*>(allocator(static_cast<intptr_t>(required_size)));
@@ -1341,7 +1359,10 @@ void BigintOperations::FromHexCString(const char* hex_string,
   // given string has it's lsd at the last position.
   // The hex_i index, pointing into the string, starts therefore at the end,
   // whereas the bigint-index (i) starts at 0.
-  intptr_t hex_length = strlen(hex_string);
+  const intptr_t hex_length = strlen(hex_string);
+  if (hex_length < 0) {
+    FATAL("Fatal error in BigintOperations::FromHexCString: string too long");
+  }
   intptr_t hex_i = hex_length - 1;
   for (intptr_t i = 0; i < bigint_length; i++) {
     Chunk digit = 0;