Copilot commented on code in PR #3855:
URL: https://github.com/apache/avro/pull/3855#discussion_r3567557706
##########
lang/perl/lib/Avro/DataFileReader.pm:
##########
@@ -203,36 +208,172 @@ sub read_block_header {
$datafile->{block_size} = Avro::BinaryDecoder->decode_long(
undef, undef, $fh,
);
+ ## Both are Avro long (zigzag) values, so a malformed/truncated file can
+ ## yield negatives. A negative block_size would flow into $want and a
+ ## negative-length read; a negative object_count is equally nonsensical.
+ ## Reject both before first use.
+ if ($datafile->{object_count} < 0) {
+ croak "Invalid negative object count: $datafile->{object_count}";
+ }
+ if ($datafile->{block_size} < 0) {
+ croak "Invalid negative block size: $datafile->{block_size}";
+ }
$datafile->{block_start} = tell $fh;
return if $codec eq 'null';
- ## we need to read the entire block into memory, to inflate it
- my $nread = read $fh, my $block, $datafile->{block_size} + MARKER_SIZE
- or croak "Error reading from file: $!";
+ ## Guard against an attacker-controlled block_size triggering a huge
+ ## allocation for the compressed block itself, before any decompression
+ ## happens. When the reader is configured with block_max_size, reject a
+ ## block whose declared compressed size exceeds that bound up front.
+ my $block_max = $datafile->{block_max_size};
+ my $block_size = $datafile->{block_size};
+ if (defined $block_max && $block_size > $block_max) {
+ Avro::DataFile::Error::CompressedBlockSize->throw(
+ "Compressed block size $block_size exceeds the configured
block_max_size of $block_max bytes"
+ );
+ }
+
+ ## we need to read the entire block into memory, to inflate it. Verify the
+ ## exact byte count: a short read (truncated/malformed file) would
otherwise
+ ## slip through and surface later as a confusing marker/decompressor error.
+ my $want = $datafile->{block_size} + MARKER_SIZE;
+ my $block;
+ my $nread = read $fh, $block, $want;
+ if (!defined $nread) {
+ croak "Error reading from file: $!";
+ }
+ if ($nread != $want) {
+ croak "Short read: expected $want bytes for the block, got $nread
(truncated file?)";
+ }
Review Comment:
`read $fh, $block, $want` can allocate based on the attacker-controlled
`$want` (block_size + MARKER_SIZE) before you detect a short read. On malformed
files with an enormous declared block_size but little/no remaining data, this
can still trigger an OOM/DoS even though you later croak on `Short read`.
Consider reading the compressed block in bounded chunks until `$want` bytes are
accumulated (or EOF), so truncated files fail without preallocating `$want`
bytes.
--
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]
