Copilot commented on code in PR #3859:
URL: https://github.com/apache/avro/pull/3859#discussion_r3567458249
##########
lang/c++/impl/BinaryDecoder.cc:
##########
@@ -177,9 +242,27 @@ size_t BinaryDecoder::skipArray() {
for (;;) {
auto r = doDecodeLong();
if (r < 0) {
- auto n = static_cast<size_t>(doDecodeLong());
- in_.skipBytes(n);
+ auto byteSize = doDecodeLong();
+ if (byteSize < 0) {
+ // A negative block byte-size would convert to a huge size_t
and
+ // drive an unbounded skip; reject it.
+ throw Exception("Invalid negative block size: {}", byteSize);
+ }
+ in_.skipBytes(static_cast<size_t>(byteSize));
Review Comment:
In BinaryDecoder::skipArray(), the negative-block path casts the decoded
block byte-size to size_t without checking it fits. On 32-bit builds (or any
build where size_t is narrower), a large byteSize can truncate, causing the
decoder to skip the wrong amount. Also, StreamReader::skipBytes ultimately
calls InputStream::skip(), and the memory-backed skip() implementations clamp
at EOF rather than throwing, so an oversized byteSize can be silently accepted
when the remaining size is known. Consider rejecting byte sizes that don’t fit
in size_t and using checkAvailableBytes() before skipping, consistent with
skipString()/skipBytes().
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
