Security: zone->Allocate api encourages integer overflow

[turnidge]

We can get integer overflow easily with our current uses of zone->Allocate(). We should consider moving to a calloc-style, size+count interface which checks explicitly for integer overflow.

This causes potential overflow in some of the following places:

runtime/lib/string.cc:

DEFINE_NATIVE_ENTRY(StringBase_createFromCodePoints, 1) {
[...]
const Array& a = Array::CheckedHandle(arguments->At(0));
[...]
intptr_t len = a.Length();
[...]
uint32_t* temp = reinterpret_cast<uint32_t*>(
      zone->Allocate(len * sizeof(uint32_t))); // NOLINT ← Here!!!! len is controllable from the script

lib/regexp_jsc.cc (x2)

 intptr_t size = str.Length() * sizeof(uint16_t);
  Zone* zone = Isolate::Current()->current_zone();
  uint16_t* two_byte_str = reinterpret_cast<uint16_t*>(zone->Allocate(size));

 int offsets_length = (num_bracket_expressions + 1) * kJscreMultiple;
  int* offsets = NULL;
  int offsets_array_size = offsets_length * sizeof(offsets[0]);
  offsets = reinterpret_cast<int*>(zone->Allocate(offsets_array_size));

[turnidge]

Changed the title to: "Security: zone->Allocate api encourages integer overflow".

[iposva-google]

Added Security label.

[iposva-google]

Added this to the M1 milestone.

[turnidge]

Set owner to @turnidge.
Added Started label.

[turnidge]

Fixed in revision 10254.

Changed the allocation api to look like this:

  zone->Alloc<Type>(len)

Now that we know the Type, we can do the overflow check. The cases that cannot use this new api call zone->AllocUnsafe, which has comments about the need to check for overflow before calling the function.

---

Added Fixed label.