iemejia commented on code in PR #3855:
URL: https://github.com/apache/avro/pull/3855#discussion_r3567451770
##########
lang/perl/lib/Avro/DataFileReader.pm:
##########
@@ -207,32 +212,157 @@ sub read_block_header {
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"
+ );
+ }
Review Comment:
Fixed — object_count and block_size are now validated as >= 0 immediately
after decoding, so a negative block_size can't flow into $want / a
negative-length read.
##########
lang/perl/lib/Avro/DataFileReader.pm:
##########
@@ -207,32 +212,157 @@ sub read_block_header {
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?)";
+ }
## remove the marker
my $marker = substr $block, -(MARKER_SIZE), MARKER_SIZE, '';
$datafile->{block_marker} = $marker;
+ ## The decompressed size of a block is capped to guard against a block with
+ ## a very high compression ratio expanding to far more memory than its
+ ## compressed size. The limit is the AVRO_MAX_DECOMPRESS_LENGTH environment
+ ## variable, or DEFAULT_MAX_DECOMPRESS_LENGTH. (Note: block_max_size is a
+ ## writer-side flush threshold measured in compressed bytes and is not
reused
+ ## here to avoid conflating the two units.)
Review Comment:
Fixed — reworded to clarify that block_max_size IS used above as a
compressed-size pre-read guard, but is not reused here for the
decompressed-size cap (to avoid conflating the two units).
--
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]
