Fix integer overflow in section and subsection size checks (#2694)

Author: sumleo

src/binary-reader.cc

@@ -417,7 +417,7 @@ Result BinaryReader::ReadStr(std::string_view* out_str, const char* desc) {
   uint32_t str_len = 0;
   CHECK_RESULT(ReadU32Leb128(&str_len, "string length"));
 
-  ERROR_UNLESS(state_.offset + str_len <= read_end_,
+  ERROR_UNLESS(str_len <= read_end_ - state_.offset,
                "unable to read string: %s", desc);
 
   *out_str = std::string_view(
@@ -442,7 +442,7 @@ Result BinaryReader::ReadBytes(const void** out_data,
 Result BinaryReader::ReadBytesWithSize(const void** out_data,
                                        Offset size,
                                        const char* desc) {
-  ERROR_UNLESS(state_.offset + size <= read_end_, "unable to read data: %s",
+  ERROR_UNLESS(size <= read_end_ - state_.offset, "unable to read data: %s",
                desc);
 
   *out_data = static_cast<const uint8_t*>(state_.data) + state_.offset;
@@ -2033,9 +2033,9 @@ Result BinaryReader::ReadNameSection(Offset section_size) {
     }
     previous_subsection_type = name_type;
     CHECK_RESULT(ReadOffset(&subsection_size, "subsection size"));
-    size_t subsection_end = state_.offset + subsection_size;
-    ERROR_UNLESS(subsection_end <= read_end_,
+    ERROR_UNLESS(subsection_size <= read_end_ - state_.offset,
                  "invalid sub-section size: extends past end");
+    size_t subsection_end = state_.offset + subsection_size;
     ReadEndRestoreGuard guard(this);
     read_end_ = subsection_end;
 
@@ -2224,9 +2224,9 @@ Result BinaryReader::ReadDylink0Section(Offset section_size) {
     Offset subsection_size;
     CHECK_RESULT(ReadU32Leb128(&dylink_type, "type"));
     CHECK_RESULT(ReadOffset(&subsection_size, "subsection size"));
-    size_t subsection_end = state_.offset + subsection_size;
-    ERROR_UNLESS(subsection_end <= read_end_,
+    ERROR_UNLESS(subsection_size <= read_end_ - state_.offset,
                  "invalid sub-section size: extends past end");
+    size_t subsection_end = state_.offset + subsection_size;
     ReadEndRestoreGuard guard(this);
     read_end_ = subsection_end;
 
@@ -2356,9 +2356,9 @@ Result BinaryReader::ReadLinkingSection(Offset section_size) {
     Offset subsection_size;
     CHECK_RESULT(ReadU32Leb128(&linking_type, "type"));
     CHECK_RESULT(ReadOffset(&subsection_size, "subsection size"));
-    size_t subsection_end = state_.offset + subsection_size;
-    ERROR_UNLESS(subsection_end <= read_end_,
+    ERROR_UNLESS(subsection_size <= read_end_ - state_.offset,
                  "invalid sub-section size: extends past end");
+    size_t subsection_end = state_.offset + subsection_size;
     ReadEndRestoreGuard guard(this);
     read_end_ = subsection_end;
 
@@ -3107,6 +3107,8 @@ Result BinaryReader::ReadSections(const ReadSectionsOptions& options) {
     Offset section_size;
     CHECK_RESULT(ReadU8(&section_code, "section code"));
     CHECK_RESULT(ReadOffset(&section_size, "section size"));
+    ERROR_UNLESS(section_size <= state_.size - state_.offset,
+                 "invalid section size: extends past end");
     ReadEndRestoreGuard guard(this);
     read_end_ = state_.offset + section_size;
     if (section_code >= kBinarySectionCount) {

src/test-binary-reader.cc

@@ -99,3 +99,48 @@ TEST(BinaryReader, InvalidFunctionBodySize) {
             reader.first_error.message.find("invalid function body size"))
       << "Got: " << reader.first_error.message;
 }
+
+TEST(BinaryReader, OversizedSectionSize) {
+  // A module whose section size extends past the end of the data.  The
+  // subtraction-based overflow check must reject this before computing
+  // read_end_ = offset + section_size, which would overflow on platforms
+  // where size_t is 32-bit.
+  // TODO: Move this test upstream into the spec repo.
+
+  uint8_t data[] = {
+      0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00,  // magic + version
+      0x01,                                            // section code: Type
+      0x80, 0x80, 0x80, 0x80, 0x08,  // section size: 0x80000000 (LEB128)
+  };
+
+  BinaryReaderError reader;
+  ReadBinaryOptions options;
+  Result result = ReadBinary(data, sizeof(data), &reader, options);
+  EXPECT_EQ(Result::Error, result);
+  EXPECT_NE(std::string::npos,
+            reader.first_error.message.find("invalid section size"))
+      << "Got: " << reader.first_error.message;
+}
+
+TEST(BinaryReader, OversizedSubsectionSize) {
+  // A module with a name section containing a subsection whose size extends
+  // past the section boundary.
+  // TODO: Move this test upstream into the spec repo.
+
+  uint8_t data[] = {
+      0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00,  // magic + version
+      // Custom section (name section)
+      0x00,  // section code: custom
+      0x09,  // section size: 9 bytes
+      0x04,  // name length
+      'n', 'a', 'm', 'e',
+      0x01,              // subsection type: function names
+      0x80, 0x80, 0x04,  // subsection size: 65536 (LEB128), exceeds section
+  };
+
+  BinaryReaderError reader;
+  ReadBinaryOptions options;
+  Result result = ReadBinary(data, sizeof(data), &reader, options);
+  // Custom section errors are not fatal by default, but ensure no crash.
+  (void)result;
+}