From 283e1d647823524d94303889377a3e1a68d3b2ec Mon Sep 17 00:00:00 2001 From: jrd Date: Tue, 21 Jul 2026 20:43:14 +0000 Subject: [PATCH] Guard GetValFromStream against out-of-bounds reads in release builds GetValFromStream validated its arguments only with Q_ASSERT, which is compiled out in release builds (the shipped configuration). CVector uses std::vector's unchecked operator[], so a truncated or malformed network message could drive a read past the end of the buffer with no protection. Add a runtime guard alongside the existing asserts: return 0 when iNumOfBytes is out of the 1..4 range or when fewer than iNumOfBytes bytes remain. The asserts are kept so developer errors still fail loudly in debug builds, while release builds no longer perform the out-of-bounds read. Fixes #3819 --- src/protocol.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/protocol.cpp b/src/protocol.cpp index 335856ad93..aab521c522 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -2832,6 +2832,14 @@ uint32_t CProtocol::GetValFromStream ( const CVector& vecIn, int& iPos, Q_ASSERT ( ( iNumOfBytes > 0 ) && ( iNumOfBytes <= 4 ) ); Q_ASSERT ( vecIn.Size() >= iPos + iNumOfBytes ); + // The asserts above are compiled out in release builds, so also guard at + // runtime: CVector uses unchecked operator[], and reading past the end of a + // truncated/malformed network message would be an out-of-bounds access. + if ( ( iNumOfBytes < 1 ) || ( iNumOfBytes > 4 ) || ( vecIn.Size() < iPos + iNumOfBytes ) ) + { + return 0; + } + uint32_t iRet = 0; for ( int i = 0; i < iNumOfBytes; i++ )