From 138573c8ce044e5b7b8f429dd8953c88dae7c8ab Mon Sep 17 00:00:00 2001 From: Rhett Creighton Date: Fri, 5 Jun 2026 23:40:25 +0000 Subject: [PATCH] =?UTF-8?q?beta7:=20beta7:=20memory-safety=20=E2=80=94=20p?= =?UTF-8?q?revector=20capacity=20overflow=20guard=20+=20stream=20read=20si?= =?UTF-8?q?ze=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- src/prevector.h | 7 +++++++ src/streams.h | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/src/prevector.h b/src/prevector.h index aad4c27174f..82066cd24cb 100644 --- a/src/prevector.h +++ b/src/prevector.h @@ -172,10 +172,17 @@ class prevector { /* FIXME: Because malloc/realloc here won't call new_handler if allocation fails, assert success. These should instead use an allocator or new/delete so that handlers are called as necessary, but performance would be slightly degraded by doing so. */ + // Guard against sizeof(T)*new_capacity overflowing size_t (would wrap to a + // small allocation followed by an out-of-bounds write). Compared in size_t + // to match the multiplication below; only rejects sizes that can never + // represent a valid object. + if ((size_t)new_capacity > SIZE_MAX / sizeof(T)) { new_handler_terminate(); } _union.indirect = static_cast(realloc(_union.indirect, ((size_t)sizeof(T)) * new_capacity)); if (!_union.indirect) { new_handler_terminate(); } _union.capacity = new_capacity; } else { + // Guard against sizeof(T)*new_capacity overflowing size_t (see above). + if ((size_t)new_capacity > SIZE_MAX / sizeof(T)) { new_handler_terminate(); } char* new_indirect = static_cast(malloc(((size_t)sizeof(T)) * new_capacity)); if (!new_indirect) { new_handler_terminate(); } T* src = direct_ptr(0); diff --git a/src/streams.h b/src/streams.h index 9d4a2e39e04..d73a2908d78 100644 --- a/src/streams.h +++ b/src/streams.h @@ -281,6 +281,15 @@ class CBaseDataStream throw std::ios_base::failure("CBaseDataStream::read(): cannot read from null pointer"); } + // Bounds-check in full-width size_t before the (unsigned int) math below, which + // would otherwise truncate nReadPos + nSize on 64-bit and could wrap past the + // buffer. (vch.size() - nReadPos) is safe: nReadPos <= vch.size() is an invariant. + // This only rejects reads that already run past the end of data; valid reads are + // unaffected and still take the existing path below. + if (nSize > vch.size() - nReadPos) { + throw std::ios_base::failure("CBaseDataStream::read(): end of data"); + } + // Read from the beginning of the buffer unsigned int nReadPosNext = nReadPos + nSize; if (nReadPosNext >= vch.size())