From 9759abde4473f7b696ac08c2f53501c80087a30f Mon Sep 17 00:00:00 2001 From: omigamedev Date: Tue, 2 Jun 2026 17:35:00 +0200 Subject: [PATCH] Harden binary stream overlapping writes --- docs/modernization/build-inventory.md | 2 +- docs/modernization/roadmap.md | 2 +- src/foundation/binary_stream.cpp | 27 ++++++++++++++++++++++++ tests/foundation/binary_stream_tests.cpp | 21 ++++++++++++++++++ 4 files changed, 50 insertions(+), 2 deletions(-) diff --git a/docs/modernization/build-inventory.md b/docs/modernization/build-inventory.md index de52a62..0ac3c83 100644 --- a/docs/modernization/build-inventory.md +++ b/docs/modernization/build-inventory.md @@ -91,7 +91,7 @@ Known local toolchain state: `pp_paint`, `pp_document`, `pp_renderer_api`, `pp_renderer_gl`, `pp_paint_renderer`, `pp_ui_core`, `pano_cli`, and their current headless test binaries, - including foundation event/logging/task queue coverage, PNG metadata and + including foundation binary-stream/event/logging/task queue coverage, PNG metadata and decode, PPI header/layout, settings document, document snapshot/per-layer-frame/move/duration/face-pixel/PPI export coverage, snapshot-embedded face-payload rejection, paint brush/final-blend/ diff --git a/docs/modernization/roadmap.md b/docs/modernization/roadmap.md index 8c2a003..c5667a4 100644 --- a/docs/modernization/roadmap.md +++ b/docs/modernization/roadmap.md @@ -306,7 +306,7 @@ Gate: Goal: split libraries while keeping current app behavior. Status: started. `pp_foundation` exists with binary stream utilities and -boundary/overread tests. It also owns strict decimal `uint32` parsing used by +boundary/overread/overlapping-write tests. It also owns strict decimal `uint32` parsing used by `pano_cli`, with rejection tests for empty, signed, mixed, and overflowing input. A synchronous event dispatcher, structured logging facade, bounded FIFO task queue, and deterministic `TraceRecorder` now record diff --git a/src/foundation/binary_stream.cpp b/src/foundation/binary_stream.cpp index 2dc44d8..0a64e6f 100644 --- a/src/foundation/binary_stream.cpp +++ b/src/foundation/binary_stream.cpp @@ -1,7 +1,28 @@ #include "foundation/binary_stream.h" +#include + namespace pp::foundation { +namespace { + +[[nodiscard]] bool overlaps_backing_storage( + const std::vector& backing, + std::span bytes) noexcept +{ + if (backing.empty() || bytes.empty()) { + return false; + } + + const auto backing_begin = reinterpret_cast(backing.data()); + const auto backing_end = backing_begin + backing.size(); + const auto bytes_begin = reinterpret_cast(bytes.data()); + const auto bytes_end = bytes_begin + bytes.size(); + return bytes_begin < backing_end && backing_begin < bytes_end; +} + +} + ByteReader::ByteReader(std::span bytes) noexcept : bytes_(bytes) { @@ -135,6 +156,12 @@ Status ByteWriter::write_bytes(std::span bytes) return Status::invalid_argument("writer has no backing storage"); } + if (overlaps_backing_storage(*bytes_, bytes)) { + const std::vector copy(bytes.begin(), bytes.end()); + bytes_->insert(bytes_->end(), copy.begin(), copy.end()); + return Status::success(); + } + bytes_->insert(bytes_->end(), bytes.begin(), bytes.end()); return Status::success(); } diff --git a/tests/foundation/binary_stream_tests.cpp b/tests/foundation/binary_stream_tests.cpp index 38c578e..57720bf 100644 --- a/tests/foundation/binary_stream_tests.cpp +++ b/tests/foundation/binary_stream_tests.cpp @@ -87,6 +87,26 @@ void boundary_reads_are_consistent(pp::tests::Harness& h) } } +void appends_overlapping_source_bytes_deterministically(pp::tests::Harness& h) +{ + std::vector bytes { + std::byte { 0x10 }, + std::byte { 0x20 }, + std::byte { 0x30 }, + }; + ByteWriter writer(bytes); + + const auto status = writer.write_bytes(std::span(bytes.data() + 1, 2)); + + PP_EXPECT(h, status.ok()); + PP_EXPECT(h, writer.size() == 5U); + PP_EXPECT(h, bytes[0] == std::byte { 0x10 }); + PP_EXPECT(h, bytes[1] == std::byte { 0x20 }); + PP_EXPECT(h, bytes[2] == std::byte { 0x30 }); + PP_EXPECT(h, bytes[3] == std::byte { 0x20 }); + PP_EXPECT(h, bytes[4] == std::byte { 0x30 }); +} + } int main() @@ -96,5 +116,6 @@ int main() harness.run("rejects_overread_without_moving_cursor", rejects_overread_without_moving_cursor); harness.run("rejects_out_of_range_seek", rejects_out_of_range_seek); harness.run("boundary_reads_are_consistent", boundary_reads_are_consistent); + harness.run("appends_overlapping_source_bytes_deterministically", appends_overlapping_source_bytes_deterministically); return harness.finish(); }