Simple Cache: validate lengths before allocations.

net/disk_cache/simple/simple_synchronous_entry.cc

Index: net/disk_cache/simple/simple_synchronous_entry.cc
diff --git a/net/disk_cache/simple/simple_synchronous_entry.cc b/net/disk_cache/simple/simple_synchronous_entry.cc
index 71f1cb9fffb40364fb38864f2f76146bb0402769..1b8536e57ed89c649f0b42d790e04f93cc32e9db 100644
--- a/net/disk_cache/simple/simple_synchronous_entry.cc
+++ b/net/disk_cache/simple/simple_synchronous_entry.cc
@@ -665,7 +665,7 @@ void SimpleSynchronousEntry::CheckEOFRecord(int index,
   uint32_t crc32;
   bool has_crc32;
   bool has_key_sha256;
-  int stream_size;
+  int32_t stream_size;
   *out_result = GetEOFRecordData(index, entry_stat, &has_crc32, &has_key_sha256,
                                  &crc32, &stream_size);
   if (*out_result != net::OK) {
@@ -1177,18 +1177,20 @@ int SimpleSynchronousEntry::ReadAndValidateStream0(
   bool has_crc32;
   bool has_key_sha256;
   uint32_t read_crc32;
-  int stream_0_size;
+  int32_t stream_0_size;
   int ret_value_crc32 =
       GetEOFRecordData(0, *out_entry_stat, &has_crc32, &has_key_sha256,
                        &read_crc32, &stream_0_size);
   if (ret_value_crc32 != net::OK)
     return ret_value_crc32;
+
   // Calculate and set the real values for data size.
-  int stream_1_size = out_entry_stat->data_size(1) - stream_0_size;
+  int32_t stream_1_size = out_entry_stat->data_size(1);
   if (!has_key_sha256)
     stream_1_size += sizeof(net::SHA256HashValue);
-  if (stream_1_size < 0)
+  if (stream_0_size > stream_1_size)

[Julia Tuttle, 2016/06/27 20:39:10]

This conditional sounds nonsensical. It makes sense in the context of
"stream_1_size is actually the length of the whole thing and I'm about to
subtract stream_0_size from it", but it'd make more sense if you used another
name for stream_1_size and then did "int32_t stream_1_size = total_size -
stream_0_size" or something like that.

[gavinp, 2016/07/26 17:32:34]

Done.

     return net::ERR_FAILED;
+  stream_1_size -= stream_0_size;
   out_entry_stat->set_data_size(0, stream_0_size);
   out_entry_stat->set_data_size(1, stream_1_size);
 
@@ -1264,6 +1266,9 @@ int SimpleSynchronousEntry::GetEOFRecordData(int index,
     return net::ERR_CACHE_CHECKSUM_READ_FAILURE;
   }
 
+  if (!base::IsValueInRangeForNumericType<int>(eof_record.stream_size))

[Julia Tuttle, 2016/06/27 20:39:10]

int, or int32_t? (Also, do you want to check for negative here?)

[gavinp, 2016/07/26 17:32:34]

Great catch. Fixed the signature. Thanks.

Don't need to check for negative. eof_record.steam_size is an unsigned, so it's
UNPOSSIBLE.

+    return net::ERR_FAILED;
+
   *out_has_crc32 = (eof_record.flags & SimpleFileEOF::FLAG_HAS_CRC32) ==
                    SimpleFileEOF::FLAG_HAS_CRC32;
   *out_has_key_sha256 =