Catch potential integer overflows in bigint operations.

runtime/vm/bigint_operations.cc

 // Copyright 2012 Google Inc. All Rights Reserved.
 
 #include "vm/bigint_operations.h"
 
+#include "platform/assert.h"
 #include "platform/utils.h"
 
 #include "vm/double_internals.h"
 #include "vm/exceptions.h"
 #include "vm/object_store.h"
 #include "vm/zone.h"
 
 namespace dart {
 
 RawBigint* BigintOperations::NewFromSmi(const Smi& smi, Heap::Space space) {
@@ skipping 84 matching lines @@
   // recursive call is not negative.
   // We don't catch leading '-'s for zero. Ex: "--0", or "---".
   if (str[0] == '-') {
     const Bigint& result = Bigint::Handle(NewFromCString(&str[1], space));
     result.ToggleSign();
     ASSERT(result.IsZero() || result.IsNegative());
     ASSERT(IsClamped(result));
     return result.raw();
   }
 
-  intptr_t str_length = strlen(str);
+  const intptr_t str_length = strlen(str);
+  if (str_length < 0) {

[sra1, 2013/09/17 05:32:31]

I don't understand the purpose of this test.

If str_length < 0 then str_length > 2 below will fail and the else branch calls
a function that has its own validation.

[Michael Lippautz (Google), 2013/09/17 17:10:35]

Replaced with a comment saying why we don't need a check.

+    FATAL("Fatal error in BigintOperations::NewFromCString: string too long");
+  }
   if ((str_length > 2) &&
       (str[0] == '0') &&
       ((str[1] == 'x') || (str[1] == 'X'))) {
     const Bigint& result = Bigint::Handle(FromHexCString(&str[2], space));
     ASSERT(IsClamped(result));
     return result.raw();
   } else {
     return FromDecimalCString(str, space);
   }
 }
 
 
 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;
 }
 
 
 RawBigint* BigintOperations::FromHexCString(const char* hex_string,
                                             Heap::Space space) {
   // If the string starts with '-' recursively restart the whole operation
   // without the character and then toggle the sign.
@@ skipping 16 matching lines @@
 
 
 RawBigint* BigintOperations::FromDecimalCString(const char* str,
                                                 Heap::Space space) {
   Isolate* isolate = Isolate::Current();
   // Read 8 digits a time. 10^8 < 2^27.
   const int kDigitsPerIteration = 8;
   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.
   // The first digit might also not have kDigitsPerIteration decimal digits.
   int first_digit_decimal_digits = str_length % kDigitsPerIteration;
   Chunk digit = 0;
   for (intptr_t i = 0; i < first_digit_decimal_digits; i++) {
     char c = str[str_pos++];
     ASSERT(('0' <= c) && (c <= '9'));
     digit = digit * 10 + c - '0';
@@ skipping 68 matching lines @@
 
 
 const char* BigintOperations::ToHexCString(intptr_t length,
                                            bool is_negative,
                                            void* data,
                                            uword (*allocator)(intptr_t size)) {
   NoGCScope no_gc;
 
   ASSERT(kDigitBitSize % 4 == 0);
 
-  intptr_t chunk_length = length;
+  // Conservative maximum chunk length.
+  const intptr_t kMaxChunkLen =
+      (kIntptrMax - 2 /* 0x */
+                  - 1 /* trailing '\0' */
+                  - 1 /* leading '-' */) / kHexCharsPerDigit;
+  const intptr_t chunk_length = length;
+  // Conservative check assuming leading bigint-digit also takes up
+  // kHexCharsPerDigit.
+  if (chunk_length > kMaxChunkLen) {
+    FATAL("Fatal error in BigintOperations::ToHexCString: string too long");
+  }
   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);
     result[kLength] = '\0';
     return result;
   }
   ASSERT(chunk_data != NULL);
 
   // Compute the number of hex-digits that are needed to represent the
   // leading bigint-digit. All other digits need exactly kHexCharsPerDigit
@@ skipping 58 matching lines @@
 const char* BigintOperations::ToDecimalCString(
     const Bigint& bigint, uword (*allocator)(intptr_t size)) {
   // log10(2) ~= 0.30102999566398114.
   const intptr_t kLog2Dividend = 30103;
   const intptr_t kLog2Divisor = 100000;
   // We remove a small constant for rounding imprecision, the \0 character and
   // the negative sign.
   const intptr_t kMaxAllowedDigitLength =
       (kIntptrMax - 10) / kLog2Dividend / kDigitBitSize * kLog2Divisor;
 
-  intptr_t length = bigint.Length();
+  const intptr_t length = bigint.Length();
   Isolate* isolate = Isolate::Current();
   if (length >= kMaxAllowedDigitLength) {
     // Use the preallocated out of memory exception to avoid calling
     // into dart code or allocating any code.
     const Instance& exception =
         Instance::Handle(isolate->object_store()->out_of_memory());
     Exceptions::Throw(exception);
     UNREACHABLE();
   }
 
@@ skipping 990 matching lines @@
 
 
 void BigintOperations::FromHexCString(const char* hex_string,
                                       const Bigint& value) {
   ASSERT(hex_string[0] != '-');
   intptr_t bigint_length = ComputeChunkLength(hex_string);
   // The bigint's least significant digit (lsd) is at position 0, whereas the
   // 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;
     int shift = 0;
     for (int j = 0; j < kHexCharsPerDigit; j++) {
       // Reads a block of hexadecimal digits and stores it in 'digit'.
       // Ex: "0123456" with kHexCharsPerDigit == 3, hex_i == 6, reads "456".
       if (hex_i < 0) {
         break;
       }
@@ skipping 393 matching lines @@
 int BigintOperations::CountBits(Chunk digit) {
   int result = 0;
   while (digit != 0) {
     digit >>= 1;
     result++;
   }
   return result;
 }
 
 }  // namespace dart