Ismaël Mejía created AVRO-4343:
----------------------------------
Summary: [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
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)
