Fix container-overflow in BinaryReaderInterp (#2747)

src/interp/binary-reader-interp.cc

@@ -153,6 +153,7 @@ class BinaryReaderInterp : public BinaryReaderNop {
 
   Result OnStartFunction(Index func_index) override;
 
+  Result OnFunctionBodyCount(Index count) override;
   Result BeginFunctionBody(Index index, Offset size) override;
   Result OnLocalDeclCount(Index count) override;
   Result OnLocalDecl(Index decl_index, Index count, Type type) override;
@@ -629,6 +630,17 @@ Result BinaryReaderInterp::OnFunction(Index index, Index sig_index) {
   return Result::Ok;
 }
 
+Result BinaryReaderInterp::OnFunctionBodyCount(Index count) {
+  // Can hit this case on a malformed module if we don't stop on first error.
+  if (count != module_.funcs.size()) {
+    PrintError(
+        "number of function bodies in code section does not match "
+        "actual number of funcs in module");
+    return Result::Error;
+  }
+  return Result::Ok;
+}
+
 Result BinaryReaderInterp::OnTableCount(Index count) {
   module_.tables.reserve(std::min(count, kMaxPreallocatedBufferSize));
   return Result::Ok;

src/test-interp.cc

@@ -734,6 +734,27 @@ TEST_F(InterpGCTest, Collect_DeepRecursion) {
   EXPECT_EQ(1u, store_.object_count());
 }
 
+TEST_F(InterpTest, FunctionBodyCountMismatchWithCollectErrors) {
+  // Function section declares 1 func with invalid type index 0 (no Type
+  // section). OnFunction fails -> funcs stays empty. Code section still has 1
+  // body. Without OnFunctionBodyCount guard this container-overflows in
+  // BeginFunctionBody.
+  const std::vector<u8> data = {
+      0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00, 0x03,
+      0x02, 0x01, 0x00, 0x0a, 0x04, 0x01, 0x02, 0x00, 0x0b,
+  };
+
+  Errors errors;
+  ReadBinaryOptions options(Features{}, nullptr, /*read_debug_names=*/false,
+                            /*stop_on_first_error=*/false,
+                            /*fail_on_custom_section_error=*/false);
+  ModuleDesc module_desc;
+  Result result =
+      ReadBinaryInterp("<test>", data, options, &errors, &module_desc);
+  EXPECT_EQ(Result::Error, result);
+  EXPECT_GT(errors.size(), 0u);
+}
+
 // TODO: Test for Thread keeping references alive as locals/params/stack values.
 // This requires better tracking of references than currently exists in the
 // interpreter. (see TODOs in Select/LocalGet/GlobalGet)