Harden untrusted payload parsing against out-of-bounds access

.bazelrc

@@ -8,8 +8,24 @@ build:asan --copt=-fsanitize=address
 build:asan --copt=-fno-omit-frame-pointer
 build:asan --linkopt=-fsanitize=address
 
-test:asan --test_output=errors
-test:asan --test_env=ASAN_OPTIONS=abort_on_error=1:symbolize=1:fast_unwind_on_malloc=0
+# AddressSanitizer + libFuzzer for long receive-path fuzz runs.
+# Requires clang (GCC does not support -fsanitize=fuzzer). Linux recommended:
+#   bazel run --config=fuzz //phaser:receive_fuzz -- -max_total_time=120 \
+#     -artifact_prefix=/tmp/phaser_fuzz/ /path/to/corpus
+build:fuzz --strip=never
+build:fuzz -c dbg
+build:fuzz --repo_env=CC=clang
+build:fuzz --repo_env=CXX=clang++
+build:fuzz --action_env=CC=clang
+build:fuzz --action_env=CXX=clang++
+build:fuzz --action_env=BAZEL_COMPILER=clang
+build:fuzz --copt=-fsanitize=fuzzer,address
+build:fuzz --copt=-fno-omit-frame-pointer
+build:fuzz --linkopt=-fsanitize=fuzzer,address
+build:fuzz --copt=-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+
+test:fuzz --test_output=errors
+test:fuzz --test_env=ASAN_OPTIONS=abort_on_error=1:symbolize=1:detect_leaks=0
 
 # Valgrind (memcheck): bazel test //phaser/... --config=valgrind
 # Build with debug symbols (and no optimization) so valgrind reports useful

MODULE.bazel

@@ -20,6 +20,14 @@ bazel_dep(name = "zlib", version = "1.3.1.bcr.5")
 bazel_dep(name = "cpp_toolbelt", version = "2.1.2")
 bazel_dep(name = "coroutines", version = "3.3.2")
 
+# Until cpp_toolbelt 2.1.3 is published to BCR, pin the bounds-checking fixes
+# from the bounds_fixes branch. Remove this override once the release lands.
+git_override(
+    module_name = "cpp_toolbelt",
+    remote = "https://github.com/dallison/cpp_toolbelt.git",
+    commit = "f69cb51",
+)
+
 # protobuf pulls an older rules_go via gazelle; Bazel 9 needs a current rules_go.
 single_version_override(
     module_name = "rules_go",

phaser/BUILD.bazel

@@ -1,6 +1,6 @@
 load("@com_google_protobuf//bazel:cc_proto_library.bzl", "cc_proto_library")
 load("@com_google_protobuf//bazel:proto_library.bzl", "proto_library")
-load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test")
+load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test")
 load("//phaser:copts.bzl", "PHASER_COPTS")
 
 package(default_visibility = ["//visibility:public"])
@@ -257,3 +257,87 @@ cc_test(
         "@cpp_toolbelt//toolbelt",
     ],
 )
+
+# Shared fuzz driver used by the libFuzzer binary and the corpus regression test.
+cc_library(
+    name = "receive_fuzz_lib",
+    srcs = ["receive_fuzz.cc"],
+    hdrs = ["receive_fuzz.h"],
+    copts = PHASER_COPTS + [
+        # libFuzzer provides LLVMFuzzerTestOneInput; keep a weak stub for the
+        # non-fuzz regression test binary that links this library alone.
+        "-Wno-missing-prototypes",
+    ],
+    deps = [
+        "//phaser/runtime:phaser_runtime",
+        "//phaser/testdata:test_message_phaser",
+    ],
+)
+
+cc_binary(
+    name = "receive_fuzz_seed_gen",
+    srcs = ["receive_fuzz_seed_gen.cc"],
+    copts = PHASER_COPTS,
+    deps = [
+        "//phaser/runtime:phaser_runtime",
+        "//phaser/testdata:test_message_phaser",
+        "@cpp_toolbelt//toolbelt",
+    ],
+)
+
+genrule(
+    name = "receive_fuzz_corpus",
+    outs = [
+        "corpus_empty",
+        "corpus_tiny",
+        "corpus_magic_only",
+        "corpus_valid_raw",
+        "corpus_mode1_empty",
+        "corpus_mode1_xor",
+        "corpus_valid_inflated_full_size",
+    ],
+    cmd = """
+      set -e
+      $(location :receive_fuzz_seed_gen) $(@D)
+      mv $(@D)/empty $(location corpus_empty)
+      mv $(@D)/tiny $(location corpus_tiny)
+      mv $(@D)/magic_only $(location corpus_magic_only)
+      mv $(@D)/valid_raw $(location corpus_valid_raw)
+      mv $(@D)/mode1_empty $(location corpus_mode1_empty)
+      mv $(@D)/mode1_xor $(location corpus_mode1_xor)
+      mv $(@D)/valid_inflated_full_size $(location corpus_valid_inflated_full_size)
+    """,
+    tools = [":receive_fuzz_seed_gen"],
+)
+
+# Always-on CI regression: run seed corpus + deterministic mutations.
+cc_test(
+    name = "receive_fuzz_test",
+    srcs = ["receive_fuzz_test.cc"],
+    copts = PHASER_COPTS,
+    data = [
+        ":receive_fuzz_corpus",
+        "valgrind.supp",
+        "//phaser/testdata:fuzz_crash_union_int64",
+        "//phaser/testdata:fuzz_crash_bounded_string",
+    ],
+    env = {
+        "TEST_CORPUS": "phaser",
+    },
+    deps = [
+        ":receive_fuzz_lib",
+        "@com_google_googletest//:gtest",
+    ],
+)
+
+# Long-running libFuzzer binary. Build/run with --config=fuzz (Linux/clang):
+#   bazel run --config=fuzz //phaser:receive_fuzz -- -max_total_time=60
+cc_binary(
+    name = "receive_fuzz",
+    srcs = ["receive_fuzz_main.cc"],
+    copts = PHASER_COPTS,
+    tags = ["manual"],
+    deps = [
+        ":receive_fuzz_lib",
+    ],
+)

phaser/compiler/message_gen.cc

@@ -1711,6 +1711,19 @@ void MessageGenerator::GenerateCreators(std::ostream& os, bool decl) {
         "reinterpret_cast<::toolbelt::PayloadBuffer "
         "*>(const_cast<void*>(addr));\n"
         "  ::phaser::MessageRuntime runtime(pb, size);\n"
+        "  // Reject payloads whose header is not structurally valid before "
+        "any\n"
+        "  // field offset is dereferenced.  An invalid buffer yields a message "
+        "bound\n"
+        "  // to offset 0 so every accessor safely returns a default value.\n"
+        "  if (addr == nullptr ||\n"
+        "      !::phaser::internal::IsStructurallyValidPhaser(\n"
+        "          absl::Span<const char>(static_cast<const char *>(addr), "
+        "size))) {\n"
+        "    return "
+     << MessageName(message_)
+     << "(BorrowRuntime(runtime), 0);\n"
+        "  }\n"
         "  return "
      << MessageName(message_)
      << "(BorrowRuntime(runtime), pb->message);\n"

phaser/phaser_test.cc

@@ -5,7 +5,10 @@
 
 #include <gtest/gtest.h>
 
+#include <cstdint>
+#include <cstring>
 #include <sstream>
+#include <vector>
 
 #include "absl/strings/str_format.h"
 #include "phaser/runtime/runtime.h"
@@ -167,6 +170,139 @@ TEST(PhaserTest, NewFieldsRepeatedBasic) {
   ASSERT_EQ(msg2.vstr(2), msg.vstr(2));
 }
 
+// A malicious sender can build a structurally valid payload but corrupt the
+// in-buffer offsets/lengths/counts it contains. Attaching to such a buffer with
+// CreateReadonly and reading fields must never read outside the received bytes.
+// Run under AddressSanitizer (--config=asan) to catch any out-of-bounds access.
+TEST(PhaserTest, HostilePayloadIsBounded) {
+  foo::bar::phaser::TestMessage src;
+  src.set_x(1234);
+  src.set_s("hello world");
+  src.add_vi32(0x11111111);
+  src.add_vi32(0x22222222);
+  src.add_vi32(0x33333333);
+  src.mutable_m()->set_str("inner");
+
+  // Copy exactly the shipped bytes so we can corrupt them like a hostile peer.
+  const size_t n = src.Size();
+  const char* base = static_cast<const char*>(src.Data());
+  std::vector<char> recv(base, base + n);
+
+  auto find_u32 = [](const std::vector<char>& b, uint32_t v,
+                     size_t start) -> long {
+    for (size_t i = start; i + sizeof(uint32_t) <= b.size(); ++i) {
+      uint32_t w;
+      std::memcpy(&w, b.data() + i, sizeof(w));
+      if (w == v) {
+        return static_cast<long>(i);
+      }
+    }
+    return -1;
+  };
+
+  // Baseline: the copied buffer parses and reads back correctly.
+  {
+    auto msg = foo::bar::phaser::TestMessage::CreateReadonly(recv.data(),
+                                                             recv.size());
+    ASSERT_EQ(1234, msg.x());
+    ASSERT_EQ("hello world", msg.s());
+    ASSERT_EQ(3, msg.vi32_size());
+    ASSERT_EQ("inner", msg.m().str());
+  }
+
+  // 1) Inflating full_size (header offset 12) must not let accessors read past
+  //    the received size. Keep it inflated for the remaining corruptions too.
+  {
+    uint32_t huge = 0xffffffffu;
+    std::memcpy(recv.data() + 12, &huge, sizeof(huge));
+  }
+  {
+    auto msg = foo::bar::phaser::TestMessage::CreateReadonly(recv.data(),
+                                                             recv.size());
+    ASSERT_EQ(1234, msg.x());
+    ASSERT_EQ("hello world", msg.s());
+    ASSERT_EQ(3, msg.vi32_size());
+  }
+
+  // 2) A hostile string length must be clamped to the buffer, not trusted.
+  long s_pos = -1;
+  for (size_t i = 0; i + 11 <= recv.size(); ++i) {
+    if (std::memcmp(recv.data() + i, "hello world", 11) == 0) {
+      s_pos = static_cast<long>(i);
+      break;
+    }
+  }
+  ASSERT_GE(s_pos, 4);
+  {
+    uint32_t huge = 0xffffffffu;
+    std::memcpy(recv.data() + s_pos - 4, &huge, sizeof(huge));
+  }
+  {
+    auto msg = foo::bar::phaser::TestMessage::CreateReadonly(recv.data(),
+                                                             recv.size());
+    std::string_view s = msg.s();
+    ASSERT_LE(s.size(), recv.size());
+    ASSERT_EQ(0, s.compare(0, 11, "hello world"));
+  }
+
+  // 3) A hostile repeated-field element count must be clamped. Locate the vi32
+  //    data, then the VectorHeader { num_elements=3, data_offset } pointing at
+  //    it, and blow up the count.
+  const long data_pos = find_u32(recv, 0x11111111u, 0);
+  ASSERT_GE(data_pos, 0);
+  const uint32_t data_off = static_cast<uint32_t>(data_pos);
+  long hdr_pos = -1;
+  for (size_t i = 0; i + 2 * sizeof(uint32_t) <= recv.size(); ++i) {
+    uint32_t num, off;
+    std::memcpy(&num, recv.data() + i, sizeof(num));
+    std::memcpy(&off, recv.data() + i + sizeof(uint32_t), sizeof(off));
+    if (num == 3 && off == data_off) {
+      hdr_pos = static_cast<long>(i);
+      break;
+    }
+  }
+  ASSERT_GE(hdr_pos, 0);
+  {
+    uint32_t huge = 0xffffffffu;
+    std::memcpy(recv.data() + hdr_pos, &huge, sizeof(huge));
+  }
+  {
+    auto msg = foo::bar::phaser::TestMessage::CreateReadonly(recv.data(),
+                                                             recv.size());
+    const int count = msg.vi32_size();
+    ASSERT_LE(static_cast<size_t>(count),
+              (recv.size() - data_off) / sizeof(int32_t));
+    // Iterating the clamped range must stay in-bounds (ASan verifies this).
+    long long sum = 0;
+    for (int i = 0; i < count; ++i) {
+      sum += msg.vi32(i);
+    }
+    (void)sum;
+    // capacity() must not underflow-read before a hostile data offset.
+    ASSERT_GE(msg.vi32().capacity(), 0u);
+  }
+
+  // 4) Presence bits / has_* and nested-message access must stay in-bounds
+  //    even with inflated full_size (already set above).
+  {
+    auto msg = foo::bar::phaser::TestMessage::CreateReadonly(recv.data(),
+                                                             recv.size());
+    (void)msg.has_x();
+    (void)msg.has_s();
+    (void)msg.has_m();
+    (void)msg.m().str();
+    // String size()/data() used by Serialize must be size-aware.
+    ASSERT_LE(msg.s().size(), recv.size());
+    const char* sdata = msg.s().data();
+    if (sdata != nullptr) {
+      ASSERT_GE(static_cast<const void*>(sdata),
+                static_cast<const void*>(recv.data()));
+      ASSERT_LT(static_cast<const void*>(sdata),
+                static_cast<const void*>(recv.data() + recv.size()));
+    }
+  }
+}
+
 TEST(PhaserTest, DeletedFieldsBasic) {
   foo::bar::phaser::TestMessage msg;
   msg.set_x(1234);