iemejia commented on code in PR #3864:
URL: https://github.com/apache/avro/pull/3864#discussion_r3565150811
##########
lang/perl/lib/Avro/BinaryDecoder.pm:
##########
@@ -132,10 +132,95 @@ sub decode_bytes {
my $class = shift;
my $reader = pop;
my $size = decode_long($class, undef, undef, $reader);
+ _ensure_available($reader, $size);
$reader->read(my $buf, $size);
Review Comment:
Fixed in 92df248: decode_bytes now rejects a negative decoded length before
reading.
##########
lang/perl/lib/Avro/BinaryDecoder.pm:
##########
@@ -132,10 +132,95 @@ sub decode_bytes {
my $class = shift;
my $reader = pop;
my $size = decode_long($class, undef, undef, $reader);
+ _ensure_available($reader, $size);
$reader->read(my $buf, $size);
return $buf;
}
+## Reject a declared length that exceeds the bytes actually remaining before
+## allocating for it, to guard against an out-of-memory attack from a malicious
+## or truncated input. Only enforced for larger reads and when the reader can
+## report its size via seek/tell; smaller reads and non-seekable readers fall
+## through to a direct read.
+use constant _MAX_UNCHECKED_READ => 1024 * 1024;
+
+## Number of bytes still available to read, or undef when the reader cannot
+## report its size. Used to reject a declared length or collection block count
+## that exceeds the data actually available before allocating for it. Readers
+## that do not support seeking to the end (e.g. a streaming decompressor)
return
+## undef, so the check is simply skipped for them.
+sub _bytes_remaining {
+ my ($reader) = @_;
+ my $current = eval { $reader->tell };
+ return if !defined $current || $current < 0;
+ return eval {
+ $reader->seek(0, 2) # SEEK_END
+ or die "seek to end failed\n";
+ my $end = $reader->tell;
+ $reader->seek($current, 0); # SEEK_SET, restore position
+ die "unknown end position\n" if !defined $end || $end < 0;
+ $end - $current;
+ };
+}
Review Comment:
Fixed in 92df248: _bytes_remaining now always restores the original position
after seeking to the end (even if reading the end offset fails) and treats a
restore failure as fatal, so the reader is never silently left at EOF.
##########
lang/perl/lib/Avro/BinaryDecoder.pm:
##########
@@ -296,13 +383,16 @@ sub decode_map {
my $block_count = decode_long($class, @_);
my $writer_values = $writer_schema->values;
my $reader_values = $reader_schema->values;
+ # Map keys are strings (>= 1 byte length prefix) plus the value.
+ my $min_bytes = 1 + _min_bytes_per_element($writer_values);
Review Comment:
Fixed in 92df248: decode_map now uses 2 bytes as the minimum key size
(1-byte length prefix + at least 1 byte of data), since empty keys are
rejected, making the pre-check slightly tighter.
--
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]
