[
https://issues.apache.org/jira/browse/AVRO-4343?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated AVRO-4343:
---------------------------------
Labels: pull-request-available (was: )
> [perl] BinaryDecoder skip path broken: absolute seek corrupts decoding and
> skip_block infinite-loops on negative block count
> ----------------------------------------------------------------------------------------------------------------------------
>
> Key: AVRO-4343
> URL: https://issues.apache.org/jira/browse/AVRO-4343
> Project: Apache Avro
> Issue Type: Bug
> Components: perl
> Reporter: Ismaël Mejía
> Priority: Major
> Labels: pull-request-available
> Time Spent: 10m
> Remaining Estimate: 0h
>
> The Perl BinaryDecoder skip path (used during schema resolution when the
> writer's record contains a field absent from the reader's schema -- see
> decode_record -> skip) is broken in two ways. Both are reachable with ordinary
> schema evolution and one is a denial-of-service on crafted input. The code has
> been present since the initial Perl implementation (AVRO-974).
> *1. Absolute seek instead of relative in skip_bytes / skip_string /
> skip_fixed*
> {code:perl}
> sub skip_bytes {
> my $size = decode_long(...);
> $reader->seek($size, 0); # whence 0 = SEEK_SET (absolute!)
> }
> {code}
> To skip a value the decoder must advance the cursor by $size bytes
> (SEEK_CUR, whence 1). Using whence 0 seeks to *absolute* offset $size from the
> start of the stream, so any bytes/string/fixed field that is skipped
> mid-record
> repositions the reader and corrupts the decoding of every following field.
> skip_fixed (seek($schema->size, 0)) has the same defect.
> This is silently masked by t/03_bin_decode.t because the only skipped field in
> the test ("bonus") is the *last* writer field, so the wrong position is never
> observed. Reorder it (or skip a non-trailing field) and decoding breaks.
> *2. Infinite loop in skip_block on a negative block count (DoS)*
> {code:perl}
> sub skip_block {
> my $block_count = decode_long(...);
> while ($block_count) {
> if ($block_count < 0) {
> $reader->seek($block_count, 0); # negative absolute seek;
> block-size never read
> next; # re-loops WITHOUT re-reading
> $block_count
> }
> ...
> $block_count = decode_long(...);
> }
> }
> {code}
> Per the Avro spec, a negative array/map block count is followed by a long
> block-size (in bytes) to permit skipping. skip_block never reads that
> block-size,
> seeks to a negative absolute offset (a no-op / failure on a scalar or file
> handle), and then does `next` without re-reading $block_count -- so
> $block_count
> stays negative and the loop spins forever. An attacker who controls the
> serialized data (and knows the reader drops an array/map field) can encode a
> negative block count to hang the decoder -> CPU denial of service. This is the
> skip-path analogue of the Java issue fixed in AVRO-3635.
> *Reachability*: both paths are hit by decode_record whenever the writer schema
> has a field not present in the reader schema (a common schema-evolution case).
> *Suggested fix*:
> - skip_bytes/skip_string/skip_fixed: use a relative skip (seek($size, 1) /
> SEEK_CUR, or read-and-discard) and reject negative lengths.
> - skip_block: when block_count < 0, read the following block-size long and
> skip
> forward by that many bytes (or decode/skip abs(count) items), reject
> negative
> block-size, and always re-read block_count each iteration (drop the bare
> `next`).
> - Add resolution tests that skip a *non-trailing* bytes field and skip an
> array/map encoded with a negative block count.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
