[
https://issues.apache.org/jira/browse/AVRO-4228?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18095520#comment-18095520
]
ASF subversion and git services commented on AVRO-4228:
-------------------------------------------------------
Commit 271de632302fc4cf25d82204a252b4e19bc474d2 in avro's branch
refs/heads/avro-cpp-issue from Ismaël Mejía
[ https://gitbox.apache.org/repos/asf?p=avro.git;h=271de63230 ]
AVRO-4275: [C] Fix signed integer overflow in block_count negation
read_array_value() and read_map_value() negate block_count via
multiplication by -1, which is undefined behavior for values that
cannot be represented after negation. Replace with the overflow-safe
-(x+1)+1 idiom (consistent with the C++ fix in AVRO-4228) and reject
invalid results early.
Add test_avro_4275 covering the overflow case and verifying that
legitimate negative block counts still decode correctly.
Assisted-by: OpenCode:claude-opus-4.6
> BinaryDecoder::arrayNext() does not handle negative block counts
> ----------------------------------------------------------------
>
> Key: AVRO-4228
> URL: https://issues.apache.org/jira/browse/AVRO-4228
> Project: Apache Avro
> Issue Type: Bug
> Reporter: Gabriel Feyer
> Assignee: Martin Tzvetanov Grigorov
> Priority: Minor
> Labels: pull-request-available
> Fix For: 1.13.0
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> BinaryDecoder::arrayNext() in lang/c++/impl/BinaryDecoder.cc calls
> doDecodeLong() directly instead of doDecodeItemCount(), causing it to
> mishandle negative array block counts.
> Per the Avro spec, a negative block count indicates the absolute value is the
> item count, followed by an additional long representing the byte-size of the
> block. The helper method doDecodeItemCount() handles this correctly, and is
> already used by arrayStart(), mapStart(), and mapNext(). Only arrayNext()
> bypasses it.
> When arrayNext() reads a negative count (e.g., -100),
> static_cast<size_t>(-100) produces a huge value, the byte-size long is left
> unconsumed, and the stream position becomes corrupted. Subsequent reads
> produce garbage, typically resulting in a vector::_M_range_check crash or
> (with the validating decoder) a "Wrong number of items" error.
> This affects any array large enough to be encoded in multiple blocks. Arrays
> that fit in a single block are unaffected because arrayNext() only sees the
> terminal zero.
> Fix:
> {code:java}
> // BinaryDecoder.cc — from:
> size_t BinaryDecoder::arrayNext() {
> return static_cast<size_t>(doDecodeLong());
> }
> // to:
> size_t BinaryDecoder::arrayNext() {
> return doDecodeItemCount();
> } {code}
> This is consistent with how arrayStart(), mapStart(), and mapNext() are
> implemented.
> Note: ClickHouse independently encountered this bug
> (https://github.com/ClickHouse/ClickHouse/issues/60438) and fixed it in their
> fork (https://github.com/ClickHouse/avro/pull/23) by inlining the
> negative-count logic. The fix above is simpler as it reuses the existing
> helper.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
