From ca52f2ada70d0fa72d2052e3c80ff42f373d6d8a Mon Sep 17 00:00:00 2001 From: David Allison Date: Fri, 28 Aug 2026 18:42:23 -0700 Subject: [PATCH 1/6] Harden untrusted payload parsing against out-of-bounds access. Validate buffers in CreateReadonly, thread received size through field and vector accessors, clamp hostile string lengths and element counts, and fix ProtoBuffer overflow so malicious wire data cannot read past the buffer. --- MODULE.bazel | 8 +++ MODULE.bazel.lock | 32 +++++++-- phaser/compiler/message_gen.cc | 13 ++++ phaser/phaser_test.cc | 114 +++++++++++++++++++++++++++++++++ phaser/runtime/fields.h | 41 ++++++------ phaser/runtime/iterators.h | 21 +++++- phaser/runtime/message.h | 106 ++++++++++++++++++++++++++++-- phaser/runtime/union.h | 4 +- phaser/runtime/vectors.h | 101 ++++++++++++++++++++++------- phaser/runtime/wireformat.h | 9 ++- 10 files changed, 389 insertions(+), 60 deletions(-) diff --git a/MODULE.bazel b/MODULE.bazel index 66f3584..7402736 100644 --- a/MODULE.bazel +++ b/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 = "f0ec6a4", +) + # protobuf pulls an older rules_go via gazelle; Bazel 9 needs a current rules_go. single_version_override( module_name = "rules_go", diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 4618626..85d57b1 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -79,8 +79,6 @@ "https://bcr.bazel.build/modules/buildozer/8.5.1/source.json": "e3386e6ff4529f2442800dee47ad28d3e6487f36a1f75ae39ae56c70f0cd2fbd", "https://bcr.bazel.build/modules/coroutines/3.3.2/MODULE.bazel": "ad1395ae9758ee5d113acab755b4d972c368c4dab66513bfe31423136f3ee608", "https://bcr.bazel.build/modules/coroutines/3.3.2/source.json": "fb47a4c3e13d730a1a58ed6a34add8f6aaeef655a47d0f179427f71bf58ba150", - "https://bcr.bazel.build/modules/cpp_toolbelt/2.1.2/MODULE.bazel": "9db22ab8e1bf493e7e449d14255dbc8f2bad5bb9b8bb2635ae53a5211cd6fcb3", - "https://bcr.bazel.build/modules/cpp_toolbelt/2.1.2/source.json": "2922fb5eb35a57a8bc599120f68843acfc23df85eb85325e50b99a8a64453f33", "https://bcr.bazel.build/modules/gawk/5.3.2.bcr.1/MODULE.bazel": "cdf8cbe5ee750db04b78878c9633cc76e80dcf4416cbe982ac3a9222f80713c8", "https://bcr.bazel.build/modules/gawk/5.3.2.bcr.1/source.json": "fa7b512dfcb5eafd90ce3959cf42a2a6fe96144ebbb4b3b3928054895f2afac2", "https://bcr.bazel.build/modules/gazelle/0.45.0/MODULE.bazel": "ecd19ebe9f8e024e1ccffb6d997cc893a974bcc581f1ae08f386bdd448b10687", @@ -313,8 +311,8 @@ }, "@@rules_android+//bzlmod_extensions:apksig.bzl%apksig_extension": { "general": { - "bzlTransitiveDigest": "IiT2UgJGnHaKiyP2A1yh3U/QWN4W9g/Byolrm78hC/s=", - "usagesDigest": "0FXD4PX+vQ/jVne2oV4v3Cw5Mc9DZQ4yTcoRkAjj/X4=", + "bzlTransitiveDigest": "O/gCjP4/VVnaP+zTRGN3DFrkkxLE5GMbE4M1HG3noGQ=", + "usagesDigest": "S8lLnnZxdeYUYq3kIGhVMk0wQ9Fd6elmCskvn+SL6iw=", "recordedInputs": [ "REPO_MAPPING:rules_android+,bazel_tools bazel_tools" ], @@ -322,13 +320,37 @@ "apksig": { "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", "attributes": { - "url": "https://android.googlesource.com/platform/tools/apksig/+archive/24e3075e68ebe17c0b529bb24bfda819db5e2f3b.tar.gz", + "urls": [ + "https://mirror.bazel.build/android.googlesource.com/platform/tools/apksig/+archive/24e3075e68ebe17c0b529bb24bfda819db5e2f3b.tar.gz" + ], + "sha256": "12e44fdbd219c5e1cc62099c2a01d775957603d2d4f693f8285f9d95d9a04e77", "build_file": "@@rules_android+//bzlmod_extensions:apksig.BUILD" } } } } }, + "@@rules_android+//bzlmod_extensions:com_android_dex.bzl%com_android_dex_extension": { + "general": { + "bzlTransitiveDigest": "fVTI/3B6KjJ93jRf+pOa6ExSALb1hgRndpTPBrJKoZQ=", + "usagesDigest": "0hluQmaWiWak6sVMP5L4wXhNyIwv9fw0y5JJ8lnPb1c=", + "recordedInputs": [ + "REPO_MAPPING:rules_android+,bazel_tools bazel_tools" + ], + "generatedRepoSpecs": { + "com_android_dex": { + "repoRuleId": "@@bazel_tools//tools/build_defs/repo:http.bzl%http_archive", + "attributes": { + "urls": [ + "https://mirror.bazel.build/android.googlesource.com/platform/dalvik/+archive/5a81c499a569731e2395f7c8d13c0e0d4e17a2b6.tar.gz" + ], + "build_file": "@@rules_android+//bzlmod_extensions:com_android_dex.BUILD", + "sha256": "86b4848c038bf687fadc812239cb01fb8d1d15cef3125b480a0448360992b95d" + } + } + } + } + }, "@@rules_android+//rules/android_sdk_repository:rule.bzl%android_sdk_repository_extension": { "general": { "bzlTransitiveDigest": "qHbR00gVzVzkxX+PRtv4UGcUFMtBz7TK9CNYUWH8nIE=", diff --git a/phaser/compiler/message_gen.cc b/phaser/compiler/message_gen.cc index a2221fc..6a87735 100644 --- a/phaser/compiler/message_gen.cc +++ b/phaser/compiler/message_gen.cc @@ -1711,6 +1711,19 @@ void MessageGenerator::GenerateCreators(std::ostream& os, bool decl) { "reinterpret_cast<::toolbelt::PayloadBuffer " "*>(const_cast(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(static_cast(addr), " + "size))) {\n" + " return " + << MessageName(message_) + << "(BorrowRuntime(runtime), 0);\n" + " }\n" " return " << MessageName(message_) << "(BorrowRuntime(runtime), pb->message);\n" diff --git a/phaser/phaser_test.cc b/phaser/phaser_test.cc index 49ace76..dd5c8d7 100644 --- a/phaser/phaser_test.cc +++ b/phaser/phaser_test.cc @@ -5,7 +5,10 @@ #include +#include +#include #include +#include #include "absl/strings/str_format.h" #include "phaser/runtime/runtime.h" @@ -167,6 +170,117 @@ 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(src.Data()); + std::vector recv(base, base + n); + + auto find_u32 = [](const std::vector& 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(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(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(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(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(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; + } +} + TEST(PhaserTest, DeletedFieldsBasic) { foo::bar::phaser::TestMessage msg; msg.set_x(1234); diff --git a/phaser/runtime/fields.h b/phaser/runtime/fields.h index d122b73..7a1d39e 100644 --- a/phaser/runtime/fields.h +++ b/phaser/runtime/fields.h @@ -154,9 +154,15 @@ class Field { if (offset < 0) { \ return type(); \ } \ - return GetBuffer()->template Get( \ - GetMessageBinaryStart() + \ - static_cast<::toolbelt::BufferOffset>(offset)); \ + const type* _phaser_addr = \ + Message::GetRuntime(this, source_offset_) \ + ->template ToAddress( \ + GetMessageBinaryStart() + \ + static_cast<::toolbelt::BufferOffset>(offset)); \ + if (_phaser_addr == nullptr) { \ + return type(); \ + } \ + return *_phaser_addr; \ } \ type GetForPrinting() const { return Get(); } \ bool IsPresent() const { \ @@ -282,16 +288,7 @@ class EnumField : public Field { return *this; } - Enum Get() const { - int32_t offset = FindFieldOffset(source_offset_); - if (offset < 0) { - return static_cast(0); - } - return static_cast( - GetBuffer()->template Get::type>( - GetMessageBinaryStart() + - static_cast<::toolbelt::BufferOffset>(offset))); - } + Enum Get() const { return static_cast(GetUnderlying()); } std::string GetForPrinting() const { return ToString(); } @@ -309,9 +306,15 @@ class EnumField : public Field { if (offset < 0) { return 0; } - return GetBuffer()->template Get::type>( - GetMessageBinaryStart() + - static_cast<::toolbelt::BufferOffset>(offset)); + const T* addr = + Message::GetRuntime(this, source_offset_) + ->template ToAddress( + GetMessageBinaryStart() + + static_cast<::toolbelt::BufferOffset>(offset)); + if (addr == nullptr) { + return 0; + } + return *addr; } void Set(Enum e) { @@ -414,7 +417,7 @@ class StringField : public Field { if (offset < 0) { return std::string_view(); } - return GetBuffer()->GetStringView( + return GetRuntime()->GetStringView( GetMessageBinaryStart() + static_cast<::toolbelt::BufferOffset>(offset)); } @@ -428,7 +431,7 @@ class StringField : public Field { GetRuntime()->ToAddress( GetMessageBinaryStart() + static_cast<::toolbelt::BufferOffset>(offset)); - return *addr != 0; + return addr != nullptr && *addr != 0; } template @@ -606,7 +609,7 @@ class NonEmbeddedStringField { if (IsPlaceholder()) { return {}; } - return GetBuffer()->GetStringView(absolute_binary_offset_); + return msg_->runtime->GetStringView(absolute_binary_offset_); } template diff --git a/phaser/runtime/iterators.h b/phaser/runtime/iterators.h index 715907f..a19c208 100644 --- a/phaser/runtime/iterators.h +++ b/phaser/runtime/iterators.h @@ -11,6 +11,7 @@ #include #include +#include #include #include "absl/status/status.h" @@ -54,7 +55,14 @@ struct FieldIterator { return FieldIterator(field, field->BaseOffset() - i * sizeof(T)); } T& operator*() const { - T* addr = field->GetBuffer()->template ToAddress(offset); + // remove_const so the sentinel stays assignable when T is const + // (const iterators instantiate this with T = const value type). + static std::remove_const_t empty; + empty = std::remove_const_t(); + T* addr = field->GetRuntime()->template ToAddress(offset); + if (addr == nullptr) { + return empty; + } return *addr; } @@ -109,7 +117,7 @@ struct StringFieldIterator { field, field->BaseOffset() - i * sizeof(::toolbelt::BufferOffset)); } std::string_view operator*() const { - return field->GetBuffer()->GetStringView(field->BaseOffset() + offset); + return field->GetRuntime()->GetStringView(field->BaseOffset() + offset); } bool operator==(const StringFieldIterator& it) const { @@ -168,7 +176,14 @@ struct EnumFieldIterator { T& operator*() const { using U = typename std::underlying_type::type; - U* addr = field->GetBuffer()->template ToAddress(offset); + // remove_const so the sentinel stays assignable when T is const + // (const iterators instantiate this with T = const value type). + static std::remove_const_t empty; + empty = static_cast>(0); + U* addr = field->GetRuntime()->template ToAddress(offset); + if (addr == nullptr) { + return empty; + } // An enum and its fixed underlying type share representation; route the // cast through void* so it is not flagged as a dereference of an unrelated // reinterpret_cast. diff --git a/phaser/runtime/message.h b/phaser/runtime/message.h index 8b20d72..9aa3e86 100644 --- a/phaser/runtime/message.h +++ b/phaser/runtime/message.h @@ -142,6 +142,53 @@ inline FieldLocation FindHybridField(const HybridFieldData* field_data, return {}; } +// Overflow-safe check that a table of 'count' elements of 'elem_size' bytes +// starting at 'offset' lies entirely within a buffer of 'limit' bytes. +inline bool MetadataRangeFits(uint32_t offset, size_t count, size_t elem_size, + size_t limit) { + if (offset > limit) { + return false; + } + const size_t remaining = limit - offset; + if (elem_size != 0 && count > remaining / elem_size) { + return false; + } + return true; +} + +// Validates that the hybrid metadata header and its dense/sparse tables fit +// within a buffer of 'limit' bytes before any entry is dereferenced. +inline bool HybridTableFits(uint32_t metadata_offset, + const HybridFieldData* field_data, size_t limit) { + if (!MetadataRangeFits(metadata_offset, 1, sizeof(HybridFieldData), limit)) { + return false; + } + const uint32_t dense_offset = + metadata_offset + static_cast(sizeof(HybridFieldData)); + if (!MetadataRangeFits(dense_offset, field_data->dense_span, + sizeof(FieldValue), limit)) { + return false; + } + const uint32_t sparse_offset = + dense_offset + + static_cast(field_data->dense_span * sizeof(FieldValue)); + return MetadataRangeFits(sparse_offset, field_data->sparse_count, + sizeof(SparseFieldData), limit); +} + +// Validates that the legacy metadata header and its field table fit within a +// buffer of 'limit' bytes before any entry is dereferenced. +inline bool LegacyTableFits(uint32_t metadata_offset, + const FieldData* field_data, size_t limit) { + if (!MetadataRangeFits(metadata_offset, 1, sizeof(uint32_t), limit)) { + return false; + } + const uint32_t entries_offset = + metadata_offset + static_cast(sizeof(uint32_t)); + return MetadataRangeFits(entries_offset, field_data->num, + sizeof(field_data->fields[0]), limit); +} + } // namespace internal enum class FieldType { @@ -351,6 +398,40 @@ struct MessageRuntime { return pb->ToAddress(offset, buffer_size); } + // Number of bytes that are trusted to be available in the buffer. On the + // readonly receive path this is the size passed to CreateReadonly; otherwise + // the buffer is one we own and full_size is authoritative. + size_t TrustedSize() const { + if (buffer_size != 0) { + return buffer_size; + } + return pb != nullptr ? size_t(pb->full_size) : 0; + } + + // Size-aware string reader. Uses the trusted buffer size so a hostile length + // word in received data cannot cause an out-of-bounds read. + std::string_view GetStringView(toolbelt::BufferOffset header_offset) const { + if (pb == nullptr) { + return {}; + } + return pb->GetStringView(header_offset, buffer_size); + } + + // Clamps an attacker-influenced element count to the number of 'elem_size' + // elements that actually fit in the buffer starting at 'data_offset'. + size_t ClampElementCount(toolbelt::BufferOffset data_offset, size_t claimed, + size_t elem_size) const { + if (data_offset == 0 || elem_size == 0) { + return 0; + } + const size_t limit = TrustedSize(); + if (data_offset >= limit) { + return 0; + } + const size_t available = (limit - data_offset) / elem_size; + return claimed < available ? claimed : available; + } + template toolbelt::BufferOffset ToOffset(const T* addr) const { return pb->ToOffset(addr, buffer_size); @@ -624,18 +705,33 @@ struct Message { if (field_data == nullptr) { return {}; } - const void* metadata = runtime->ToAddress(*field_data); + const ::toolbelt::BufferOffset metadata_offset = *field_data; + const void* metadata = runtime->ToAddress(metadata_offset); if (metadata == nullptr) { return {}; } + // The metadata tables are indexed using counts stored in the buffer, so we + // must confirm those tables lie within the trusted buffer bounds before + // dereferencing any entry (received data cannot be trusted). + const size_t limit = runtime->TrustedSize(); + if (!internal::MetadataRangeFits(metadata_offset, 1, sizeof(uint32_t), + limit)) { + return {}; + } const uint32_t first_word = *static_cast(metadata); if (first_word == kHybridFieldDataMagic) { - return internal::FindHybridField( - static_cast(metadata), field_number); + const auto* hybrid = static_cast(metadata); + if (!internal::HybridTableFits(metadata_offset, hybrid, limit)) { + return {}; + } + return internal::FindHybridField(hybrid, field_number); + } + const auto* legacy = static_cast(metadata); + if (!internal::LegacyTableFits(metadata_offset, legacy, limit)) { + return {}; } - return internal::FindLegacyField(static_cast(metadata), - field_number); + return internal::FindLegacyField(legacy, field_number); } void* BinaryData() const { diff --git a/phaser/runtime/union.h b/phaser/runtime/union.h index c788e3b..71e7f2b 100644 --- a/phaser/runtime/union.h +++ b/phaser/runtime/union.h @@ -218,7 +218,7 @@ class UnionStringField : public UnionMemberField { if (runtime == nullptr) { return ""; } - return GetBuffer(runtime)->GetStringView(abs_offset); + return runtime->GetStringView(abs_offset); } void Print(std::ostream& os, int /*indent*/, @@ -231,7 +231,7 @@ class UnionStringField : public UnionMemberField { uint32_t abs_offset) const { const ::toolbelt::BufferOffset* addr = runtime->ToAddress(abs_offset); - return *addr != 0; + return addr != nullptr && *addr != 0; } void SetOffset(const std::shared_ptr& /*runtime*/, diff --git a/phaser/runtime/vectors.h b/phaser/runtime/vectors.h index 2b3ddad..a83fbed 100644 --- a/phaser/runtime/vectors.h +++ b/phaser/runtime/vectors.h @@ -139,6 +139,10 @@ class PrimitiveVectorField : public Field { T& operator[](int index) { static T empty; + empty = T(); + if (index < 0 || static_cast(index) >= size()) { + return empty; + } T* base = GetRuntime()->template ToAddress(BaseOffset()); if (base == nullptr) { return empty; @@ -147,18 +151,20 @@ class PrimitiveVectorField : public Field { } T operator[](int index) const { - static T empty; + if (index < 0 || static_cast(index) >= size()) { + return T(); + } T* base = GetRuntime()->template ToAddress(BaseOffset()); if (base == nullptr) { - return empty; + return T(); } return base[index]; } T front() { return (*this)[0]; } const T front() const { return (*this)[0]; } - T back() { return (*this)[size() - 1]; } - const T back() const { return (*this)[size() - 1]; } + T back() { return (*this)[static_cast(size()) - 1]; } + const T back() const { return (*this)[static_cast(size()) - 1]; } T Get(size_t index) const { return (*this)[static_cast(index)]; } @@ -215,7 +221,7 @@ class PrimitiveVectorField : public Field { Clear(); reserve(other.size()); for (size_t i = 0; i < other.size(); i++) { - push_back(other[i]); + push_back(other[static_cast(i)]); } ResetFieldCache(); return *this; @@ -233,12 +239,16 @@ class PrimitiveVectorField : public Field { absl::Span AsMutableSpan() { toolbelt::VectorHeader* hdr = Header(relative_binary_offset_); + if (hdr == nullptr) { + return absl::Span(); + } T* base = GetRuntime()->template ToAddress(hdr->data); if (base == nullptr) { return absl::Span(); } - - return absl::Span(base, hdr->num_elements); + const size_t n = GetRuntime()->ClampElementCount( + hdr->data, hdr->num_elements, sizeof(T)); + return absl::Span(base, n); } absl::Span AsSpan() const { @@ -247,12 +257,16 @@ class PrimitiveVectorField : public Field { return absl::Span(); } toolbelt::VectorHeader* hdr = Header(static_cast(offset)); + if (hdr == nullptr) { + return absl::Span(); + } const T* base = GetRuntime()->template ToAddress(hdr->data); if (base == nullptr) { return absl::Span(); } - - return absl::Span(base, hdr->num_elements); + const size_t n = GetRuntime()->ClampElementCount( + hdr->data, hdr->num_elements, sizeof(T)); + return absl::Span(base, n); } bool empty() const { return size() == 0; } @@ -464,7 +478,12 @@ class PrimitiveVectorField : public Field { if (offset < 0) { return 0; } - return Header(static_cast(offset))->num_elements; + const toolbelt::VectorHeader* hdr = Header(static_cast(offset)); + if (hdr == nullptr) { + return 0; + } + return GetRuntime()->ClampElementCount(hdr->data, hdr->num_elements, + sizeof(T)); } ::toolbelt::PayloadBuffer* GetBuffer() const { @@ -502,15 +521,22 @@ class EnumVectorField : public Field { using T = typename std::underlying_type::type; Enum& operator[](int index) { + static Enum empty = static_cast(0); + empty = static_cast(0); + if (index < 0 || static_cast(index) >= size()) { + return empty; + } T* base = GetRuntime()->template ToAddress(BaseOffset()); if (base == nullptr) { - static Enum empty = static_cast(0); - return *reinterpret_cast(&empty); + return empty; } return *reinterpret_cast(&base[index]); } const Enum operator[](int index) const { + if (index < 0 || static_cast(index) >= size()) { + return static_cast(0); + } const T* base = GetRuntime()->template ToAddress(BaseOffset()); if (base == nullptr) { return static_cast(0); @@ -520,8 +546,8 @@ class EnumVectorField : public Field { Enum front() { return (*this)[0]; } const Enum front() const { return (*this)[0]; } - Enum back() { return (*this)[size() - 1]; } - const Enum back() const { return (*this)[size() - 1]; } + Enum back() { return (*this)[static_cast(size()) - 1]; } + const Enum back() const { return (*this)[static_cast(size()) - 1]; } const std::vector Get() const { size_t n = size(); @@ -601,7 +627,7 @@ class EnumVectorField : public Field { Clear(); reserve(other.size()); for (size_t i = 0; i < other.size(); i++) { - push_back(other[i]); + push_back(other[static_cast(i)]); } ResetFieldCache(); return *this; @@ -776,7 +802,12 @@ class EnumVectorField : public Field { if (offset < 0) { return 0; } - return Header(static_cast(offset))->num_elements; + const toolbelt::VectorHeader* hdr = Header(static_cast(offset)); + if (hdr == nullptr) { + return 0; + } + return GetRuntime()->ClampElementCount(hdr->data, hdr->num_elements, + sizeof(T)); } ::toolbelt::PayloadBuffer* GetBuffer() const { @@ -814,17 +845,20 @@ class MessageVectorField : public Field { MessageVectorField(MessageVectorField&&) = default; T operator[](int index) const { + if (index < 0 || static_cast(index) >= NumElements()) { + return T(InternalDefault{}); + } int32_t offset = FindFieldOffset(source_offset_); if (offset == -1) { return T(InternalDefault{}); } auto hdr = Header(static_cast(offset)); - if (static_cast(index) >= hdr->num_elements) { + if (hdr == nullptr) { return T(InternalDefault{}); } ::toolbelt::BufferOffset* data = GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data); - if (data[index] == 0) { + if (data == nullptr || data[index] == 0) { return T(InternalDefault{}); } return T(GetRuntime(), data[index]); @@ -1180,7 +1214,12 @@ class MessageVectorField : public Field { if (offset < 0) { return 0; } - return Header(static_cast(offset))->num_elements; + const toolbelt::VectorHeader* hdr = Header(static_cast(offset)); + if (hdr == nullptr) { + return 0; + } + return GetRuntime()->ClampElementCount(hdr->data, hdr->num_elements, + sizeof(::toolbelt::BufferOffset)); } ::toolbelt::PayloadBuffer* GetBuffer() const { @@ -1234,33 +1273,42 @@ class StringVectorField : public Field { relative_binary_offset_(other.relative_binary_offset_) {} std::string_view operator[](int index) const { + if (index < 0 || static_cast(index) >= NumElements()) { + return {}; + } int32_t offset = FindFieldOffset(source_offset_); if (offset == -1) { return {}; } auto hdr = Header(static_cast(offset)); - if (static_cast(index) >= hdr->num_elements) { + if (hdr == nullptr) { return {}; } ::toolbelt::BufferOffset* data = GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data); - if (data[index] == 0) { + if (data == nullptr || data[index] == 0) { return {}; } - return GetBuffer()->GetStringView(data[index]); + return GetRuntime()->GetStringView(data[index]); } NonEmbeddedStringField operator[](int index) { + if (index < 0 || static_cast(index) >= NumElements()) { + return {}; + } int32_t offset = FindFieldOffset(source_offset_); if (offset == -1) { return {}; } auto hdr = Header(static_cast(offset)); - if (static_cast(index) >= hdr->num_elements) { + if (hdr == nullptr) { return {}; } ::toolbelt::BufferOffset* data = GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data); + if (data == nullptr) { + return {}; + } return NonEmbeddedStringField(Message::GetMessage(this, source_offset_), data[index]); } @@ -1556,7 +1604,12 @@ class StringVectorField : public Field { if (offset < 0) { return 0; } - return Header(static_cast(offset))->num_elements; + const toolbelt::VectorHeader* hdr = Header(static_cast(offset)); + if (hdr == nullptr) { + return 0; + } + return GetRuntime()->ClampElementCount(hdr->data, hdr->num_elements, + sizeof(::toolbelt::BufferOffset)); } ::toolbelt::PayloadBuffer* GetBuffer() const { diff --git a/phaser/runtime/wireformat.h b/phaser/runtime/wireformat.h index 39ee98c..e6eba0f 100644 --- a/phaser/runtime/wireformat.h +++ b/phaser/runtime/wireformat.h @@ -632,8 +632,13 @@ class ProtoBuffer { } absl::Status Check(size_t n) { - char* next = addr_ + n; - if (next <= end_) { + // Compare against the remaining byte count instead of forming 'addr_ + n', + // which overflows (undefined behavior, and can wrap to appear in-bounds) + // when 'n' comes from a hostile length on the wire. + if (addr_ > end_) { + return absl::InternalError("End of buffer"); + } + if (n <= static_cast(end_ - addr_)) { return absl::OkStatus(); } return absl::InternalError("End of buffer"); From f6153b7c89214f8cf6e17266645ae04157488dc7 Mon Sep 17 00:00:00 2001 From: David Allison Date: Sat, 29 Aug 2026 12:00:02 -0700 Subject: [PATCH 2/6] Close remaining CreateReadonly OOB gaps from the re-scan. Make presence, nested-message slots, string size/data, fixed-array counts, vector BaseOffset/capacity, and union discriminators size-aware so hostile payloads cannot read past the received buffer. --- phaser/phaser_test.cc | 22 +++++++++++++ phaser/runtime/arrays.h | 25 +++++++++++--- phaser/runtime/fields.h | 38 +++++++++++++--------- phaser/runtime/message.h | 50 ++++++++++++++++++++++++++++ phaser/runtime/union.h | 16 ++++++--- phaser/runtime/vectors.h | 65 +++++++++++++++---------------------- phaser/runtime/wireformat.h | 1 + 7 files changed, 154 insertions(+), 63 deletions(-) diff --git a/phaser/phaser_test.cc b/phaser/phaser_test.cc index dd5c8d7..9c6fead 100644 --- a/phaser/phaser_test.cc +++ b/phaser/phaser_test.cc @@ -278,6 +278,28 @@ TEST(PhaserTest, HostilePayloadIsBounded) { 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(sdata), + static_cast(recv.data())); + ASSERT_LT(static_cast(sdata), + static_cast(recv.data() + recv.size())); + } } } diff --git a/phaser/runtime/arrays.h b/phaser/runtime/arrays.h index 9f19db0..a0d718b 100644 --- a/phaser/runtime/arrays.h +++ b/phaser/runtime/arrays.h @@ -452,7 +452,8 @@ class PrimitiveArrayField : public Field { if (hdr == nullptr) { return 0; } - return hdr->num_elements; + return GetRuntime()->ClampElementCount(hdr->data, hdr->num_elements, + sizeof(T)); } void SetActiveCount(size_t count) { @@ -800,7 +801,8 @@ class EnumArrayField : public Field { if (hdr == nullptr) { return 0; } - return hdr->num_elements; + return GetRuntime()->ClampElementCount(hdr->data, hdr->num_elements, + sizeof(T)); } void SetActiveCount(size_t count) { @@ -1369,7 +1371,8 @@ class StringArrayField : public Field { if (hdr == nullptr) { return 0; } - return hdr->num_elements; + return GetRuntime()->ClampElementCount(hdr->data, hdr->num_elements, + sizeof(::toolbelt::BufferOffset)); } void SetActiveCount(size_t count) { @@ -1437,7 +1440,8 @@ class StringArrayField : public Field { if (hdr == nullptr || hdr->data == 0) { return empty_; } - const size_t count = hdr->num_elements; + const size_t count = GetRuntime()->ClampElementCount( + hdr->data, hdr->num_elements, sizeof(::toolbelt::BufferOffset)); if (index >= count) { return empty_; } @@ -1446,7 +1450,7 @@ class StringArrayField : public Field { } ::toolbelt::BufferOffset* data = GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data); - if (data[index] == 0) { + if (data == nullptr || data[index] == 0) { return empty_; } auto* self = const_cast(this); @@ -1479,8 +1483,19 @@ class StringArrayField : public Field { if (hdr == nullptr || hdr->data == 0) { return; } + const size_t count = GetRuntime()->ClampElementCount( + hdr->data, hdr->num_elements, sizeof(::toolbelt::BufferOffset)); + if (start >= count) { + return; + } + if (end > count) { + end = count; + } ::toolbelt::BufferOffset* data = GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data); + if (data == nullptr) { + return; + } auto* self = const_cast(this); for (size_t i = start; i < end; i++) { if (data[i] == 0) { diff --git a/phaser/runtime/fields.h b/phaser/runtime/fields.h index 7a1d39e..03ce1d6 100644 --- a/phaser/runtime/fields.h +++ b/phaser/runtime/fields.h @@ -50,12 +50,13 @@ class Field { buffer->ClearPresenceBit(static_cast(id_), binary_offset); } - bool IsPresent(uint32_t field_id, ::toolbelt::PayloadBuffer* buffer, + bool IsPresent(uint32_t field_id, const void* field, uint32_t source_offset, uint32_t binary_offset) const { if (field_id == static_cast(-1)) { return false; } - return buffer->IsPresent(field_id, binary_offset); + return Message::GetRuntime(field, source_offset) + ->IsPresent(field_id, binary_offset); } int Id() const { return id_; } @@ -167,8 +168,8 @@ class Field { type GetForPrinting() const { return Get(); } \ bool IsPresent() const { \ return Field::IsPresent( \ - static_cast(FindFieldId(source_offset_)), GetBuffer(), \ - GetPresenceMaskStart()); \ + static_cast(FindFieldId(source_offset_)), this, \ + source_offset_, GetPresenceMaskStart()); \ } \ \ void Set(type v) { \ @@ -294,7 +295,7 @@ class EnumField : public Field { bool IsPresent() const { return Field::IsPresent(static_cast(FindFieldId(source_offset_)), - GetBuffer(), GetPresenceMaskStart()); + this, source_offset_, GetPresenceMaskStart()); } std::string ToString() const { return Stringizer()(Get()); } @@ -476,7 +477,7 @@ class StringField : public Field { if (offset < 0) { return 0; } - return GetBuffer()->StringSize( + return GetRuntime()->StringSize( GetMessageBinaryStart() + static_cast<::toolbelt::BufferOffset>(offset)); } @@ -486,7 +487,7 @@ class StringField : public Field { if (offset < 0) { return nullptr; } - return GetBuffer()->StringData( + return GetRuntime()->StringData( GetMessageBinaryStart() + static_cast<::toolbelt::BufferOffset>(offset)); } @@ -640,14 +641,14 @@ class NonEmbeddedStringField { if (IsPlaceholder()) { return 0; } - return GetBuffer()->StringSize(absolute_binary_offset_); + return msg_->runtime->StringSize(absolute_binary_offset_); } const char* data() const { if (IsPlaceholder()) { return ""; } - return GetBuffer()->StringData(absolute_binary_offset_); + return msg_->runtime->StringData(absolute_binary_offset_); } bool empty() const { return size() == 0; } @@ -736,7 +737,7 @@ class IndirectMessageField : public Field { } ::toolbelt::BufferOffset* addr = GetIndirectAddress(static_cast(offset)); - if (*addr == 0) { + if (addr == nullptr || *addr == 0) { return DefaultMessage(); } // Load up the message if it's already been allocated. @@ -752,13 +753,13 @@ class IndirectMessageField : public Field { } ::toolbelt::BufferOffset* addr = GetIndirectAddress(static_cast(offset)); - return *addr != 0; + return addr != nullptr && *addr != 0; } MessageType* Mutable() { ::toolbelt::BufferOffset* addr = GetIndirectAddress(relative_binary_offset_); - if (*addr != 0) { + if (addr != nullptr && *addr != 0) { // Already allocated. msg_.runtime = GetRuntime(); msg_.absolute_binary_offset = *addr; @@ -784,6 +785,9 @@ class IndirectMessageField : public Field { void SetOffset(toolbelt::BufferOffset offset) { ::toolbelt::BufferOffset* addr = GetIndirectAddress(relative_binary_offset_); + if (addr == nullptr) { + return; + } if (*addr != 0) { // Already set, clear the exising message Clear(); @@ -796,7 +800,7 @@ class IndirectMessageField : public Field { void Clear() { ::toolbelt::BufferOffset* addr = GetIndirectAddress(relative_binary_offset_); - if (*addr == 0) { + if (addr == nullptr || *addr == 0) { return; } const ::toolbelt::BufferOffset old_offset = *addr; @@ -808,7 +812,9 @@ class IndirectMessageField : public Field { GetBuffer()->Free(GetRuntime()->ToAddress(old_offset)); // Zero out the offset to the message. addr = GetIndirectAddress(relative_binary_offset_); - *addr = 0; + if (addr != nullptr) { + *addr = 0; + } } bool operator==(const IndirectMessageField& other) const { @@ -825,7 +831,7 @@ class IndirectMessageField : public Field { } ::toolbelt::BufferOffset* addr = GetIndirectAddress(static_cast(offset)); - if (*addr != 0) { + if (addr != nullptr && *addr != 0) { // Load up the message if it's already been allocated. msg_.runtime = GetRuntime(); msg_.absolute_binary_offset = *addr; @@ -903,7 +909,7 @@ class IndirectMessageField : public Field { } ::toolbelt::BufferOffset* GetIndirectAddress(uint32_t abs_offset) const { - return GetBuffer()->template ToAddress<::toolbelt::BufferOffset>( + return GetRuntime()->template ToAddress<::toolbelt::BufferOffset>( GetMessageBinaryStart() + abs_offset); } diff --git a/phaser/runtime/message.h b/phaser/runtime/message.h index 9aa3e86..f0b0f31 100644 --- a/phaser/runtime/message.h +++ b/phaser/runtime/message.h @@ -417,6 +417,36 @@ struct MessageRuntime { return pb->GetStringView(header_offset, buffer_size); } + size_t StringSize(toolbelt::BufferOffset header_offset) const { + if (pb == nullptr) { + return 0; + } + return pb->StringSize(header_offset, buffer_size); + } + + const char* StringData(toolbelt::BufferOffset header_offset) const { + if (pb == nullptr) { + return nullptr; + } + return pb->StringData(header_offset, buffer_size); + } + + // Size-aware presence-bit read. A hostile field id or inflated full_size + // must not cause an out-of-bounds load of the presence mask. + bool IsPresent(uint32_t bit, uint32_t presence_mask_offset) const { + if (pb == nullptr || bit == static_cast(-1)) { + return false; + } + const uint32_t word = bit / 32; + bit %= 32; + const uint32_t* p = ToAddress( + presence_mask_offset + word * static_cast(sizeof(uint32_t))); + if (p == nullptr) { + return false; + } + return (*p & (1U << bit)) != 0; + } + // Clamps an attacker-influenced element count to the number of 'elem_size' // elements that actually fit in the buffer starting at 'data_offset'. size_t ClampElementCount(toolbelt::BufferOffset data_offset, size_t claimed, @@ -432,6 +462,26 @@ struct MessageRuntime { return claimed < available ? claimed : available; } + // Returns the capacity of a PayloadBuffer vector allocation whose data + // starts at 'data_offset'. The size word lives immediately before the + // data; a hostile data_offset near the start of the buffer must not cause + // an underflow read. + size_t AllocatedCapacity(toolbelt::BufferOffset data_offset, + size_t elem_size) const { + if (elem_size == 0 || + data_offset < sizeof(toolbelt::BufferOffset)) { + return 0; + } + const toolbelt::BufferOffset size_offset = + data_offset - + static_cast(sizeof(toolbelt::BufferOffset)); + const auto* size_word = ToAddress(size_offset); + if (size_word == nullptr) { + return 0; + } + return *size_word / elem_size; + } + template toolbelt::BufferOffset ToOffset(const T* addr) const { return pb->ToOffset(addr, buffer_size); diff --git a/phaser/runtime/union.h b/phaser/runtime/union.h index 71e7f2b..6abc9ca 100644 --- a/phaser/runtime/union.h +++ b/phaser/runtime/union.h @@ -259,12 +259,12 @@ class UnionStringField : public UnionMemberField { size_t size(const std::shared_ptr& runtime, uint32_t abs_offset) const { - return GetBuffer(runtime)->StringSize(abs_offset); + return runtime->StringSize(abs_offset); } const char* data(const std::shared_ptr& runtime, uint32_t abs_offset) const { - return GetBuffer(runtime)->StringData(abs_offset); + return runtime->StringData(abs_offset); } void Clear(const std::shared_ptr& runtime, uint32_t abs_offset) { @@ -631,7 +631,8 @@ class UnionField : public Field { int32_t* discrim = GetRuntime()->template ToAddress( GetMessageBinaryStart() + static_cast<::toolbelt::BufferOffset>(relative_offset)); - if (*discrim != static_cast(field_numbers_[Id])) { + if (discrim == nullptr || + *discrim != static_cast(field_numbers_[Id])) { return; } std::get(value_).Print( @@ -709,6 +710,9 @@ class UnionField : public Field { int32_t* discrim = GetRuntime()->template ToAddress( GetMessageBinaryStart() + static_cast<::toolbelt::BufferOffset>(relative_offset)); + if (discrim == nullptr) { + return 0; + } return *discrim; } @@ -716,6 +720,9 @@ class UnionField : public Field { void Clear() { int32_t* discrim = GetRuntime()->template ToAddress( GetMessageBinaryStart() + relative_binary_offset_); + if (discrim == nullptr) { + return; + } int32_t field_number = static_cast(field_numbers_[Id]); if (*discrim != field_number) { return; @@ -811,7 +818,8 @@ class UnionField : public Field { int32_t* discrim = GetRuntime()->template ToAddress( GetMessageBinaryStart() + static_cast<::toolbelt::BufferOffset>(relative_offset)); - return *discrim == static_cast(field_numbers_[Id]); + return discrim != nullptr && + *discrim == static_cast(field_numbers_[Id]); } template diff --git a/phaser/runtime/vectors.h b/phaser/runtime/vectors.h index a83fbed..b018a64 100644 --- a/phaser/runtime/vectors.h +++ b/phaser/runtime/vectors.h @@ -272,17 +272,7 @@ class PrimitiveVectorField : public Field { bool empty() const { return size() == 0; } size_t capacity() const { - ::toolbelt::BufferOffset offset = BaseOffset(); - if (offset == 0) { - return 0; - } - ::toolbelt::BufferOffset* addr = - GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(offset); - if (addr == nullptr) { - return 0; - } - // Word before memory is size of memory in bytes. - return addr[-1] / sizeof(value_type); + return GetRuntime()->AllocatedCapacity(BaseOffset(), sizeof(value_type)); } ::toolbelt::BufferOffset BinaryEndOffset() const { @@ -470,7 +460,11 @@ class PrimitiveVectorField : public Field { if (offset < 0) { return 0; } - return Header(static_cast(offset))->data; + const toolbelt::VectorHeader* hdr = Header(static_cast(offset)); + if (hdr == nullptr) { + return 0; + } + return hdr->data; } size_t NumElements() const { @@ -646,13 +640,10 @@ class EnumVectorField : public Field { size_t capacity() const { toolbelt::VectorHeader* hdr = Header(); - ::toolbelt::BufferOffset* addr = - GetRuntime()->template ToAddress<::toolbelt::BufferOffset>(hdr->data); - if (addr == nullptr) { + if (hdr == nullptr) { return 0; } - // Word before memory is size of memory in bytes. - return addr[-1] / sizeof(T); + return GetRuntime()->AllocatedCapacity(hdr->data, sizeof(T)); } ::toolbelt::BufferOffset BinaryEndOffset() const { @@ -794,7 +785,11 @@ class EnumVectorField : public Field { if (offset < 0) { return 0; } - return Header(static_cast(offset))->data; + const toolbelt::VectorHeader* hdr = Header(static_cast(offset)); + if (hdr == nullptr) { + return 0; + } + return hdr->data; } size_t NumElements() const { @@ -1047,15 +1042,8 @@ class MessageVectorField : public Field { } size_t capacity() const { - ::toolbelt::BufferOffset* base = - GetRuntime()->template ToAddress<::toolbelt::BufferOffset>( - BaseOffset()); - - if (base == nullptr) { - return 0; - } - // Word before memory is size of memory in bytes. - return base[-1] / sizeof(::toolbelt::BufferOffset); + return GetRuntime()->AllocatedCapacity(BaseOffset(), + sizeof(::toolbelt::BufferOffset)); } void reserve(size_t n) { @@ -1206,7 +1194,11 @@ class MessageVectorField : public Field { if (offset < 0) { return 0; } - return Header(static_cast(offset))->data; + const toolbelt::VectorHeader* hdr = Header(static_cast(offset)); + if (hdr == nullptr) { + return 0; + } + return hdr->data; } size_t NumElements() const { @@ -1475,15 +1467,8 @@ class StringVectorField : public Field { } size_t capacity() const { - ::toolbelt::BufferOffset* base = - GetRuntime()->template ToAddress<::toolbelt::BufferOffset>( - BaseOffset()); - - if (base == nullptr) { - return 0; - } - // Word before memory is size of memory in bytes. - return base[-1] / sizeof(::toolbelt::BufferOffset); + return GetRuntime()->AllocatedCapacity(BaseOffset(), + sizeof(::toolbelt::BufferOffset)); } void reserve(size_t n) { @@ -1596,7 +1581,11 @@ class StringVectorField : public Field { if (offset < 0) { return 0; } - return Header(static_cast(offset))->data; + const toolbelt::VectorHeader* hdr = Header(static_cast(offset)); + if (hdr == nullptr) { + return 0; + } + return hdr->data; } size_t NumElements() const { diff --git a/phaser/runtime/wireformat.h b/phaser/runtime/wireformat.h index e6eba0f..68596e9 100644 --- a/phaser/runtime/wireformat.h +++ b/phaser/runtime/wireformat.h @@ -168,6 +168,7 @@ inline bool IsStructurallyValidPhaser(absl::Span data) { } if (free_list != 0 && (free_list < minimum_header || + free_list > data.size() - sizeof(::toolbelt::FreeBlockHeader) || free_list > full_size - sizeof(::toolbelt::FreeBlockHeader))) { return false; } From 3b85b3752bab69e2565acf91945616010b324e74 Mon Sep 17 00:00:00 2001 From: David Allison Date: Sat, 29 Aug 2026 14:44:01 -0700 Subject: [PATCH 3/6] Add receive-path fuzz harness and fix union Get bounds. libFuzzer found UnionInt64Field::Get SEGVing on hostile inflated full_size; route union primitives through MessageRuntime::ToAddress. Pin cpp_toolbelt to the ToAddress sizeof-fit fix and keep ASan crash inputs as regression corpus. --- .bazelrc | 20 +- MODULE.bazel | 2 + phaser/BUILD.bazel | 85 +++++++- phaser/receive_fuzz.cc | 204 ++++++++++++++++++ phaser/receive_fuzz.h | 12 ++ phaser/receive_fuzz_main.cc | 7 + phaser/receive_fuzz_seed_gen.cc | 101 +++++++++ phaser/receive_fuzz_test.cc | 123 +++++++++++ phaser/runtime/union.h | 28 ++- phaser/testdata/BUILD | 16 ++ phaser/testdata/fuzz_crash_bounded_string.bin | Bin 0 -> 2140 bytes phaser/testdata/fuzz_crash_union_int64.bin | Bin 0 -> 2137 bytes 12 files changed, 585 insertions(+), 13 deletions(-) create mode 100644 phaser/receive_fuzz.cc create mode 100644 phaser/receive_fuzz.h create mode 100644 phaser/receive_fuzz_main.cc create mode 100644 phaser/receive_fuzz_seed_gen.cc create mode 100644 phaser/receive_fuzz_test.cc create mode 100644 phaser/testdata/fuzz_crash_bounded_string.bin create mode 100644 phaser/testdata/fuzz_crash_union_int64.bin diff --git a/.bazelrc b/.bazelrc index 1290f59..512a800 100644 --- a/.bazelrc +++ b/.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 diff --git a/MODULE.bazel b/MODULE.bazel index 7402736..4718fa6 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -22,6 +22,8 @@ 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. +# NOTE: ToAddress sizeof(T) fit check is local on bounds_fixes and not yet +# pushed; bump this commit after landing that fix. git_override( module_name = "cpp_toolbelt", remote = "https://github.com/dallison/cpp_toolbelt.git", diff --git a/phaser/BUILD.bazel b/phaser/BUILD.bazel index 44ed55f..8b31c21 100644 --- a/phaser/BUILD.bazel +++ b/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,86 @@ 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", + "//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", + ], +) diff --git a/phaser/receive_fuzz.cc b/phaser/receive_fuzz.cc new file mode 100644 index 0000000..18aa48b --- /dev/null +++ b/phaser/receive_fuzz.cc @@ -0,0 +1,204 @@ +// Copyright 2024-2026 David Allison +// All Rights Reserved +// See LICENSE file for licensing information. + +// Fuzz target for the Phaser CreateReadonly receive path. +// +// Any byte string fed through PhaserReceiveFuzzOneInput, followed by typical +// receiver reads, must not crash or cause out-of-bounds access (ASan). +// +// Modes (first input byte): +// even: remaining bytes are a raw received buffer. +// odd: start from a valid TestMessage and overlay/mutate with remaining bytes. + +#include "phaser/receive_fuzz.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "phaser/testdata/TestMessage.phaser.h" + +namespace { + +std::vector MakeValidSeed() { + foo::bar::phaser::TestMessage msg; + msg.set_x(1234); + msg.set_y(5678); + msg.set_s("hello world"); + msg.mutable_m()->set_str("inner"); + msg.mutable_m()->set_f(0x1111); + msg.add_vi32(0x11111111); + msg.add_vi32(0x22222222); + msg.add_vi32(0x33333333); + msg.add_vstr("one"); + msg.add_vstr("two"); + msg.add_vstr("three"); + msg.add_vm().set_str("vm0"); + msg.set_u1b(4321); + msg.set_u2b("u2b"); + msg.mutable_u3b()->set_str("u3b"); + msg.set_buffer("buffer\ndata"); + msg.set_e(foo::bar::phaser::FOO); + msg.set_fl(1.5f); + msg.set_db(2.5); + const char* data = static_cast(msg.Data()); + return std::vector(data, data + msg.Size()); +} + +const std::vector& ValidSeed() { + static const std::vector seed = MakeValidSeed(); + return seed; +} + +void TouchInner(const foo::bar::phaser::InnerMessage& m) { + (void)m.has_str(); + (void)m.str(); + (void)m.has_f(); + (void)m.f(); + (void)m.has_e(); + (void)m.e(); + const int n = m.ev_size(); + for (int i = 0; i < n; ++i) { + (void)m.ev(i); + } + (void)m.has_uva(); + (void)m.uva(); + (void)m.has_uvb(); + (void)m.uvb(); +} + +void TouchMessage(const foo::bar::phaser::TestMessage& msg) { + (void)msg.has_x(); + (void)msg.x(); + (void)msg.has_y(); + (void)msg.y(); + (void)msg.has_s(); + { + std::string_view s = msg.s(); + (void)s.size(); + (void)s.data(); + } + (void)msg.has_m(); + TouchInner(msg.m()); + + const int vi = msg.vi32_size(); + for (int i = 0; i < vi; ++i) { + (void)msg.vi32(i); + } + for (int32_t v : msg.vi32()) { + (void)v; + } + (void)msg.vi32().capacity(); + + const int vs = msg.vstr_size(); + for (int i = 0; i < vs; ++i) { + std::string_view s = msg.vstr(i); + (void)s.size(); + } + + const int vm = msg.vm_size(); + for (int i = 0; i < vm; ++i) { + TouchInner(msg.vm(i)); + } + + (void)msg.has_u1a(); + (void)msg.u1a(); + (void)msg.has_u1b(); + (void)msg.u1b(); + (void)msg.has_u2a(); + (void)msg.u2a(); + (void)msg.has_u2b(); + { + std::string_view s = msg.u2b(); + (void)s.size(); + } + (void)msg.has_u3a(); + (void)msg.u3a(); + (void)msg.has_u3b(); + TouchInner(msg.u3b()); + + (void)msg.has_buffer(); + { + std::string_view b = msg.buffer(); + (void)b.size(); + } + (void)msg.has_e(); + (void)msg.e(); + (void)msg.has_fl(); + (void)msg.fl(); + (void)msg.has_db(); + (void)msg.db(); + + (void)msg.has_any(); + (void)msg.any().has_type_url(); + (void)msg.any().type_url(); + (void)msg.any().has_value(); + (void)msg.any().value().size(); + + { + std::ostringstream os; + os << msg; + } + (void)msg.DebugString(); + std::string serialized; + (void)msg.SerializeToString(&serialized); +} + +void FuzzBuffer(const char* data, size_t size) { + auto msg = foo::bar::phaser::TestMessage::CreateReadonly(data, size); + TouchMessage(msg); +} + +} // namespace + +int PhaserReceiveFuzzOneInput(const uint8_t* data, size_t size) { + if (size == 0) { + FuzzBuffer(nullptr, 0); + return 0; + } + + const uint8_t mode = data[0] & 1u; + const uint8_t* payload = data + 1; + const size_t payload_size = size - 1; + + try { + if (mode == 0) { + FuzzBuffer(reinterpret_cast(payload), payload_size); + } else { + std::vector buf = ValidSeed(); + if (payload_size == 0) { + FuzzBuffer(buf.data(), buf.size()); + return 0; + } + for (size_t i = 0; i < buf.size(); ++i) { + buf[i] = static_cast(static_cast(buf[i]) ^ + payload[i % payload_size]); + } + if (payload_size >= 8) { + uint32_t off = 0; + uint32_t val = 0; + std::memcpy(&off, payload, 4); + std::memcpy(&val, payload + 4, 4); + if (!buf.empty()) { + const size_t idx = off % buf.size(); + std::memcpy(buf.data() + idx, &val, + std::min(sizeof(val), buf.size() - idx)); + } + } + FuzzBuffer(buf.data(), buf.size()); + } + } catch (const std::exception&) { + } catch (...) { + } + return 0; +} + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + return PhaserReceiveFuzzOneInput(data, size); +} diff --git a/phaser/receive_fuzz.h b/phaser/receive_fuzz.h new file mode 100644 index 0000000..0de2fa5 --- /dev/null +++ b/phaser/receive_fuzz.h @@ -0,0 +1,12 @@ +// Copyright 2024-2026 David Allison +// All Rights Reserved +// See LICENSE file for licensing information. + +#pragma once + +#include +#include + +// Shared entry used by the libFuzzer binary and the corpus regression test. +// Returns 0 always; ASan/UBSan report memory errors. +int PhaserReceiveFuzzOneInput(const uint8_t* data, size_t size); diff --git a/phaser/receive_fuzz_main.cc b/phaser/receive_fuzz_main.cc new file mode 100644 index 0000000..6bbe839 --- /dev/null +++ b/phaser/receive_fuzz_main.cc @@ -0,0 +1,7 @@ +// Copyright 2024-2026 David Allison +// All Rights Reserved +// See LICENSE file for licensing information. + +// Empty translation unit so //phaser:receive_fuzz can be a cc_binary that +// depends on :receive_fuzz_lib (which defines LLVMFuzzerTestOneInput). +// Link with --config=fuzz (-fsanitize=fuzzer) to get libFuzzer's main. diff --git a/phaser/receive_fuzz_seed_gen.cc b/phaser/receive_fuzz_seed_gen.cc new file mode 100644 index 0000000..2be2653 --- /dev/null +++ b/phaser/receive_fuzz_seed_gen.cc @@ -0,0 +1,101 @@ +// Copyright 2024-2026 David Allison +// All Rights Reserved +// See LICENSE file for licensing information. + +// Writes a small seed corpus for receive_fuzz into the directory given as argv[1]. + +#include +#include +#include +#include +#include +#include + +#include "phaser/testdata/TestMessage.phaser.h" +#include "toolbelt/payload_buffer.h" + +namespace { + +void WriteFile(const std::string& path, const void* data, size_t size) { + std::ofstream out(path, std::ios::binary); + out.write(static_cast(data), + static_cast(size)); +} + +std::vector ValidPayload() { + foo::bar::phaser::TestMessage msg; + msg.set_x(42); + msg.set_s("seed"); + msg.add_vi32(1); + msg.add_vi32(2); + msg.add_vstr("a"); + msg.mutable_m()->set_str("inner"); + // Exercise oneof / union value slots in the seed corpus. + msg.set_u3a(0x1122334455667788LL); + const char* data = static_cast(msg.Data()); + return std::vector(data, data + msg.Size()); +} + +} // namespace + +int main(int argc, char** argv) { + if (argc < 2) { + std::fprintf(stderr, "usage: %s \n", argv[0]); + return 1; + } + const std::string dir = argv[1]; + + // Mode 0: empty / tiny / magic-only raw buffers. + WriteFile(dir + "/empty", "", 0); + const char tiny[] = {'\0'}; + WriteFile(dir + "/tiny", tiny, 1); + { + std::vector buf(sizeof(toolbelt::PayloadBuffer), '\0'); + uint32_t magic = toolbelt::kFixedBufferMagic; + std::memcpy(buf.data(), &magic, sizeof(magic)); + // Mode byte 0 = raw. + std::vector input; + input.push_back(0); + input.insert(input.end(), buf.begin(), buf.end()); + WriteFile(dir + "/magic_only", input.data(), input.size()); + } + + // Mode 0: a full valid payload as raw input. + { + auto payload = ValidPayload(); + std::vector input; + input.push_back(0); + input.insert(input.end(), payload.begin(), payload.end()); + WriteFile(dir + "/valid_raw", input.data(), input.size()); + } + + // Mode 1: structure-aware with empty overlay (just the seed message). + { + char mode = 1; + WriteFile(dir + "/mode1_empty", &mode, 1); + } + + // Mode 1: structure-aware with a few XOR bytes. + { + std::vector input; + input.push_back(1); + const uint8_t overlay[] = {0xff, 0x00, 0xaa, 0x55, 0x12, 0x34, 0x56, 0x78}; + input.insert(input.end(), overlay, overlay + sizeof(overlay)); + WriteFile(dir + "/mode1_xor", input.data(), input.size()); + } + + // Mode 0: inflate full_size on a valid payload. + { + auto payload = ValidPayload(); + if (payload.size() >= 16) { + uint32_t huge = 0xffffffffu; + std::memcpy(payload.data() + 12, &huge, sizeof(huge)); + } + std::vector input; + input.push_back(0); + input.insert(input.end(), payload.begin(), payload.end()); + WriteFile(dir + "/valid_inflated_full_size", input.data(), input.size()); + } + + return 0; +} diff --git a/phaser/receive_fuzz_test.cc b/phaser/receive_fuzz_test.cc new file mode 100644 index 0000000..d637153 --- /dev/null +++ b/phaser/receive_fuzz_test.cc @@ -0,0 +1,123 @@ +// Copyright 2024-2026 David Allison +// All Rights Reserved +// See LICENSE file for licensing information. + +#include +#include +#include +#include + +#include "gtest/gtest.h" +#include "phaser/receive_fuzz.h" + +namespace { + +std::vector> LoadCorpus(const std::string& dir) { + std::vector> inputs; + for (const auto& entry : std::filesystem::directory_iterator(dir)) { + if (!entry.is_regular_file()) { + continue; + } + const std::string name = entry.path().filename().string(); + // Only the generated corpus_* seeds (ignore other runfiles in the package). + if (name.rfind("corpus_", 0) != 0) { + continue; + } + std::ifstream in(entry.path(), std::ios::binary); + std::vector bytes((std::istreambuf_iterator(in)), + std::istreambuf_iterator()); + inputs.push_back(std::move(bytes)); + } + return inputs; +} + +} // namespace + +TEST(ReceiveFuzzTest, SeedCorpusDoesNotCrash) { + const char* corpus_rel = std::getenv("TEST_CORPUS"); + ASSERT_NE(corpus_rel, nullptr); + + std::string corpus_dir = corpus_rel; + if (const char* srcdir = std::getenv("TEST_SRCDIR")) { + std::string ws = "_main"; + if (const char* w = std::getenv("TEST_WORKSPACE")) { + ws = w; + } + corpus_dir = std::string(srcdir) + "/" + ws + "/" + corpus_rel; + } + if (!std::filesystem::exists(corpus_dir)) { + // Fallback: runfiles-relative path from the test cwd. + corpus_dir = corpus_rel; + } + ASSERT_TRUE(std::filesystem::exists(corpus_dir)) << corpus_dir; + + auto inputs = LoadCorpus(corpus_dir); + ASSERT_FALSE(inputs.empty()); + for (const auto& input : inputs) { + PhaserReceiveFuzzOneInput(input.data(), input.size()); + } +} + +TEST(ReceiveFuzzTest, RandomMutationsDoNotCrash) { + // Deterministic pseudo-random walk so CI catches crashes without libFuzzer. + std::vector buf(256); + uint32_t state = 0xC0FFEEu; + auto rnd = [&]() -> uint8_t { + state = state * 1664525u + 1013904223u; + return static_cast(state >> 24); + }; + + for (int iter = 0; iter < 2000; ++iter) { + const size_t n = 1 + (rnd() % buf.size()); + buf[0] = static_cast(iter & 1); // alternate modes + for (size_t i = 1; i < n; ++i) { + buf[i] = rnd(); + } + PhaserReceiveFuzzOneInput(buf.data(), n); + } +} + +TEST(ReceiveFuzzTest, KnownUnionCrashDoesNotSegfault) { + // Regression for ASan SEGV in UnionInt64Field::Get when a hostile payload + // inflates full_size and Get used PayloadBuffer::Get (no trusted size). + const char* rel = "phaser/testdata/fuzz_crash_union_int64.bin"; + std::string path = rel; + if (const char* srcdir = std::getenv("TEST_SRCDIR")) { + std::string ws = "_main"; + if (const char* w = std::getenv("TEST_WORKSPACE")) { + ws = w; + } + path = std::string(srcdir) + "/" + ws + "/" + rel; + } + ASSERT_TRUE(std::filesystem::exists(path)) << path; + std::ifstream in(path, std::ios::binary); + std::vector bytes((std::istreambuf_iterator(in)), + std::istreambuf_iterator()); + ASSERT_FALSE(bytes.empty()); + PhaserReceiveFuzzOneInput(bytes.data(), bytes.size()); +} + +TEST(ReceiveFuzzTest, KnownBoundedStringCrashDoesNotOverflow) { + // Regression for ASan heap-buffer-overflow in BoundedString when a string + // length word starts inside the buffer but sizeof(uint32_t) does not fit. + const char* rel = "phaser/testdata/fuzz_crash_bounded_string.bin"; + std::string path = rel; + if (const char* srcdir = std::getenv("TEST_SRCDIR")) { + std::string ws = "_main"; + if (const char* w = std::getenv("TEST_WORKSPACE")) { + ws = w; + } + path = std::string(srcdir) + "/" + ws + "/" + rel; + } + ASSERT_TRUE(std::filesystem::exists(path)) << path; + std::ifstream in(path, std::ios::binary); + std::vector bytes((std::istreambuf_iterator(in)), + std::istreambuf_iterator()); + ASSERT_FALSE(bytes.empty()); + PhaserReceiveFuzzOneInput(bytes.data(), bytes.size()); +} + +int main(int argc, char** argv) { + testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/phaser/runtime/union.h b/phaser/runtime/union.h index 6abc9ca..9e614ac 100644 --- a/phaser/runtime/union.h +++ b/phaser/runtime/union.h @@ -48,7 +48,15 @@ class UnionMemberField { if (runtime == nullptr) { \ return type(); \ } \ - return GetBuffer(runtime)->template Get(abs_offset); \ + /* Use MessageRuntime::ToAddress so CreateReadonly's trusted size \ + * clamps hostile offsets; PayloadBuffer::Get trusts inflated \ + * full_size and can SEGV on receive. */ \ + const type* addr = \ + runtime->template ToAddress(abs_offset); \ + if (addr == nullptr) { \ + return type(); \ + } \ + return *addr; \ } \ void Print(std::ostream& os, int /*indent*/, \ const std::shared_ptr& runtime, \ @@ -136,13 +144,7 @@ class UnionEnumField : public UnionMemberField { Enum Get(const std::shared_ptr& runtime, uint32_t abs_offset) const { - if (runtime == nullptr) { - return static_cast(0); - } - return static_cast( - GetBuffer(runtime) - ->template Get::type>( - abs_offset)); + return static_cast(GetUnderlying(runtime, abs_offset)); } void Print(std::ostream& os, int /*indent*/, @@ -153,8 +155,14 @@ class UnionEnumField : public UnionMemberField { T GetUnderlying(const std::shared_ptr& runtime, uint32_t abs_offset) const { - return GetBuffer(runtime) - ->template Get::type>(abs_offset); + if (runtime == nullptr) { + return T(); + } + const T* addr = runtime->template ToAddress(abs_offset); + if (addr == nullptr) { + return T(); + } + return *addr; } void SetOffset(const std::shared_ptr& /*runtime*/, diff --git a/phaser/testdata/BUILD b/phaser/testdata/BUILD index 0ead64a..a1919b4 100644 --- a/phaser/testdata/BUILD +++ b/phaser/testdata/BUILD @@ -6,6 +6,22 @@ load("//phaser:phaser_library.bzl", "phaser_library") package(default_visibility = ["//visibility:public"]) +# libFuzzer regression inputs from CreateReadonly ASan crashes. +exports_files([ + "fuzz_crash_union_int64.bin", + "fuzz_crash_bounded_string.bin", +]) + +filegroup( + name = "fuzz_crash_union_int64", + srcs = ["fuzz_crash_union_int64.bin"], +) + +filegroup( + name = "fuzz_crash_bounded_string", + srcs = ["fuzz_crash_bounded_string.bin"], +) + proto_library( name = "foo_proto", srcs = [ diff --git a/phaser/testdata/fuzz_crash_bounded_string.bin b/phaser/testdata/fuzz_crash_bounded_string.bin new file mode 100644 index 0000000000000000000000000000000000000000..e10e1ccefe41a634936241f4ea495a7f34595f74 GIT binary patch literal 2140 zcmeH|v2GJV5Qcx7B(g$51W0iap(`zlD4{?uD6U9hibxhp;ex`4Vh|EaP)tF21qCH# zDl3nWGA|H`l9Dp7kOm2wx)M-k+CFu+EXOExX$&RH@$i@*iJbF0Q{IVvk+J-eHaG?RdJo^#;r$E?Kc%L&U zfs9rND=5EsJ6GctloM<|eWZSG=UKVbIXLvU2uBRwLq0_QN}l(T>!OUYj(skDu-)cX zufGew3ppP9SV3&V*9fX3H8&w@*c>tjHQvCX#(Ox_Na0(|_eir(Dg$ZpGh+!f58lu4 z-d%D|tbm~pMdwZjU z$S|E*{uinz5LYT=D%J;dYEqcz(e12y2--=g+-+AbU0P6DRLP7(aqCgh4f$|Kh6VcT^RWnR?&f(fk$X zR_0uypX~qsKzt+_;l%pC*k#|T-OajeGMQA5yJ)oPV65HbpAfH~C+Btk2{_@#G&i6w gG(?ALT#N8e!2EiovuR&g!^ivDpXsS49j^xA63a4LcK`qY literal 0 HcmV?d00001 diff --git a/phaser/testdata/fuzz_crash_union_int64.bin b/phaser/testdata/fuzz_crash_union_int64.bin new file mode 100644 index 0000000000000000000000000000000000000000..411582d30e8bd03270775c59b0277d7f95adbe1f GIT binary patch literal 2137 zcmeHIJx{_=6g?mi!Xi-<8I0*j90)QP28Rv|bueiRLk9d z-&5B^gCL{I7Z!@ZBh4YZ3l^P>Wk~_^HX9D}A0=J6f20OJvF8O%G zqh6x2#-B3~mru!bNz=N+8XJ-4VCp|0TAE?cQ8hH#tLT&Rf<95Btovl99KTjHZ-Qp< NTEDKL2K0AF!WVz|L`eVu literal 0 HcmV?d00001 From 8aeb61d58a47dbc3093adbe73008ef6697b16eba Mon Sep 17 00:00:00 2001 From: David Allison Date: Sat, 29 Aug 2026 14:44:12 -0700 Subject: [PATCH 4/6] Bump cpp_toolbelt pin to ToAddress sizeof-fit fix. --- MODULE.bazel | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/MODULE.bazel b/MODULE.bazel index 4718fa6..73c6059 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -22,12 +22,10 @@ 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. -# NOTE: ToAddress sizeof(T) fit check is local on bounds_fixes and not yet -# pushed; bump this commit after landing that fix. git_override( module_name = "cpp_toolbelt", remote = "https://github.com/dallison/cpp_toolbelt.git", - commit = "f0ec6a4", + commit = "f69cb51", ) # protobuf pulls an older rules_go via gazelle; Bazel 9 needs a current rules_go. From ad851ca7f40d64b93a45d3e0207783d06ca90f4f Mon Sep 17 00:00:00 2001 From: David Allison Date: Sat, 29 Aug 2026 15:24:20 -0700 Subject: [PATCH 5/6] More vuln fixes --- phaser/receive_fuzz.cc | 50 +++++++++++++++++++++++++++++++++ phaser/receive_fuzz_seed_gen.cc | 6 ++++ phaser/runtime/message_test.cc | 13 +++++++++ phaser/runtime/vectors.h | 21 ++++++++++---- 4 files changed, 84 insertions(+), 6 deletions(-) diff --git a/phaser/receive_fuzz.cc b/phaser/receive_fuzz.cc index 18aa48b..3fa2cd9 100644 --- a/phaser/receive_fuzz.cc +++ b/phaser/receive_fuzz.cc @@ -33,6 +33,8 @@ std::vector MakeValidSeed() { msg.set_s("hello world"); msg.mutable_m()->set_str("inner"); msg.mutable_m()->set_f(0x1111); + msg.mutable_m()->add_ev(foo::bar::phaser::BAR); + msg.mutable_m()->add_ev(foo::bar::phaser::FOO); msg.add_vi32(0x11111111); msg.add_vi32(0x22222222); msg.add_vi32(0x33333333); @@ -47,6 +49,11 @@ std::vector MakeValidSeed() { msg.set_e(foo::bar::phaser::FOO); msg.set_fl(1.5f); msg.set_db(2.5); + { + auto entry = msg.add_values(); + entry.set_key("map key"); + entry.set_value(99); + } const char* data = static_cast(msg.Data()); return std::vector(data, data + msg.Size()); } @@ -67,6 +74,21 @@ void TouchInner(const foo::bar::phaser::InnerMessage& m) { for (int i = 0; i < n; ++i) { (void)m.ev(i); } + // Aggregate getter and range iteration exercise the whole-vector code path, + // which is distinct from the per-index accessor above. + for (auto v : m.ev()) { + (void)v; + } + { + auto all = m.ev().Get(); + (void)all.size(); + } + { + absl::Span span = m.ev().AsSpan(); + for (auto v : span) { + (void)v; + } + } (void)m.has_uva(); (void)m.uva(); (void)m.has_uvb(); @@ -95,17 +117,30 @@ void TouchMessage(const foo::bar::phaser::TestMessage& msg) { (void)v; } (void)msg.vi32().capacity(); + { + auto all = msg.vi32().Get(); + (void)all.size(); + } const int vs = msg.vstr_size(); for (int i = 0; i < vs; ++i) { std::string_view s = msg.vstr(i); (void)s.size(); } + for (std::string_view s : msg.vstr()) { + (void)s.size(); + } const int vm = msg.vm_size(); for (int i = 0; i < vm; ++i) { TouchInner(msg.vm(i)); } + { + auto all = msg.vm().Get(); + for (const auto& inner : all) { + TouchInner(inner); + } + } (void)msg.has_u1a(); (void)msg.u1a(); @@ -135,6 +170,21 @@ void TouchMessage(const foo::bar::phaser::TestMessage& msg) { (void)msg.has_db(); (void)msg.db(); + // map is compiled to a repeated ValuesEntry message. + const int nvals = msg.values_size(); + for (int i = 0; i < nvals; ++i) { + auto entry = msg.values(i); + (void)entry.has_key(); + std::string_view k = entry.key(); + (void)k.size(); + (void)entry.has_value(); + (void)entry.value(); + } + for (auto entry : msg.values()) { + (void)entry.key().size(); + (void)entry.value(); + } + (void)msg.has_any(); (void)msg.any().has_type_url(); (void)msg.any().type_url(); diff --git a/phaser/receive_fuzz_seed_gen.cc b/phaser/receive_fuzz_seed_gen.cc index 2be2653..0f1b377 100644 --- a/phaser/receive_fuzz_seed_gen.cc +++ b/phaser/receive_fuzz_seed_gen.cc @@ -30,8 +30,14 @@ std::vector ValidPayload() { msg.add_vi32(2); msg.add_vstr("a"); msg.mutable_m()->set_str("inner"); + msg.mutable_m()->add_ev(foo::bar::phaser::FOO); // Exercise oneof / union value slots in the seed corpus. msg.set_u3a(0x1122334455667788LL); + { + auto entry = msg.add_values(); + entry.set_key("k"); + entry.set_value(7); + } const char* data = static_cast(msg.Data()); return std::vector(data, data + msg.Size()); } diff --git a/phaser/runtime/message_test.cc b/phaser/runtime/message_test.cc index 5a03024..016a098 100644 --- a/phaser/runtime/message_test.cc +++ b/phaser/runtime/message_test.cc @@ -2022,6 +2022,19 @@ TEST(MessageTest, Dynamic) { ASSERT_EQ("Hello, world!", s); } +TEST(MessageTest, EnumVectorGetReturnsAllElements) { + TestMessage msg; + InnerMessage* inner = msg.m_.Mutable(); + inner->ev_.Add(EnumTest::FOO); + inner->ev_.Add(EnumTest::BAR); + ASSERT_EQ(2u, inner->ev_.size()); + + const std::vector got = inner->ev_.Get(); + ASSERT_EQ(2u, got.size()); + EXPECT_EQ(EnumTest::FOO, got[0]); + EXPECT_EQ(EnumTest::BAR, got[1]); +} + TEST(MessageTest, Print) { TestMessage msg; msg.x_.Set(1234); diff --git a/phaser/runtime/vectors.h b/phaser/runtime/vectors.h index b018a64..c18ca84 100644 --- a/phaser/runtime/vectors.h +++ b/phaser/runtime/vectors.h @@ -546,8 +546,9 @@ class EnumVectorField : public Field { const std::vector Get() const { size_t n = size(); std::vector r; + r.reserve(n); for (size_t i = 0; i < n; i++) { - r[i] = (*this)[i]; + r.push_back((*this)[i]); } return r; } @@ -569,26 +570,34 @@ class EnumVectorField : public Field { absl::Span AsMutableSpan() { toolbelt::VectorHeader* hdr = Header(relative_binary_offset_); + if (hdr == nullptr) { + return absl::Span(); + } Enum* base = GetRuntime()->template ToAddress(hdr->data); if (base == nullptr) { return absl::Span(); } - - return absl::Span(base, hdr->num_elements); + const size_t n = GetRuntime()->ClampElementCount( + hdr->data, hdr->num_elements, sizeof(T)); + return absl::Span(base, n); } absl::Span AsSpan() const { int32_t offset = FindFieldOffset(source_offset_); if (offset < 0) { - return absl::Span(); + return absl::Span(); } toolbelt::VectorHeader* hdr = Header(static_cast(offset)); + if (hdr == nullptr) { + return absl::Span(); + } const Enum* base = GetRuntime()->template ToAddress(hdr->data); if (base == nullptr) { return absl::Span(); } - - return absl::Span(base, hdr->num_elements); + const size_t n = GetRuntime()->ClampElementCount( + hdr->data, hdr->num_elements, sizeof(T)); + return absl::Span(base, n); } void push_back(const Enum& v) { From 4e6fc978ef4d3e8c9f54d2bfb14c0498141026bc Mon Sep 17 00:00:00 2001 From: Dave Allison Date: Wed, 2 Sep 2026 12:54:27 -0700 Subject: [PATCH 6/6] Add valgrind suppression for fuzzer --- phaser/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/phaser/BUILD.bazel b/phaser/BUILD.bazel index 8b31c21..8ae718a 100644 --- a/phaser/BUILD.bazel +++ b/phaser/BUILD.bazel @@ -317,6 +317,7 @@ cc_test( copts = PHASER_COPTS, data = [ ":receive_fuzz_corpus", + "valgrind.supp", "//phaser/testdata:fuzz_crash_union_int64", "//phaser/testdata:fuzz_crash_bounded_string", ],