iemejia commented on code in PR #3864:
URL: https://github.com/apache/avro/pull/3864#discussion_r3565200087
##########
lang/perl/lib/Avro/BinaryDecoder.pm:
##########
@@ -132,10 +132,107 @@ sub decode_bytes {
my $class = shift;
my $reader = pop;
my $size = decode_long($class, undef, undef, $reader);
+ if ($size < 0) {
+ throw Avro::Schema::Error::Parse(
+ "Invalid negative bytes/string length: $size");
+ }
+ _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;
+
+ # Attempt to seek to the end. Readers that cannot (e.g. a streaming
+ # decompressor) leave the position unchanged; skip the check for them.
+ my $moved = eval { $reader->seek(0, 2) }; # SEEK_END
+ return if !$moved;
+
+ # We are now at the end. Record the end offset, then ALWAYS restore the
+ # original position. A restore failure leaves the reader corrupted for
+ # subsequent decoding, so treat it as fatal rather than continuing.
+ my $end = eval { $reader->tell };
+ unless (eval { $reader->seek($current, 0) }) { # SEEK_SET
+ throw Avro::Schema::Error::Parse(
+ "Failed to restore reader position after size check");
+ }
+ return if !defined $end || $end < 0;
+ return $end - $current;
+}
+
+sub _ensure_available {
+ my ($reader, $size) = @_;
+ return if $size <= _MAX_UNCHECKED_READ;
+ my $remaining = _bytes_remaining($reader);
+ return unless defined $remaining;
+ if ($size > $remaining) {
+ throw Avro::Schema::Error::Parse(
+ "Cannot read $size bytes, only $remaining remaining");
+ }
+ return;
+}
+
+## Minimum number of bytes a single value of the given schema can occupy on the
+## wire. Used to reject an array/map block count that could not be backed by
the
+## bytes remaining. A type that can encode to zero bytes (null) returns 0,
which
+## disables the collection check for it (so an array of nulls is not falsely
+## rejected).
+sub _min_bytes_per_element {
+ my ($schema, $visited) = @_;
+ $visited ||= {};
+ my $type = $schema->type;
+ return 0 if $type eq 'null';
+ return 4 if $type eq 'float';
+ return 8 if $type eq 'double';
+ return $schema->size if $type eq 'fixed';
+ if ($type eq 'record' || $type eq 'error') {
+ my $id = "$schema"; # stringified reference as identity
+ return 0 if $visited->{$id};
+ $visited->{$id} = 1;
+ my $total = 0;
+ for my $field (@{ $schema->fields }) {
+ $total += _min_bytes_per_element($field->{type}, $visited);
+ }
+ delete $visited->{$id};
+ return $total;
+ }
+ # boolean, int, long, bytes, string, enum, union, array, map: >= 1 byte
+ # (a union encodes at least a 1-byte branch index).
+ return 1;
+}
+
+## Reject a collection (array or map) block whose declared element count could
+## not be backed by the bytes actually remaining, before iterating. Skipped
when
+## the per-element minimum is zero or when the reader cannot report how many
+## bytes remain.
+sub _ensure_collection_available {
+ my ($reader, $count, $min_bytes) = @_;
+ return if $count <= 0 || $min_bytes <= 0;
+ my $remaining = _bytes_remaining($reader);
+ return unless defined $remaining;
+ if ($count * $min_bytes > $remaining) {
Review Comment:
Fixed in 98aaeb6: _ensure_collection_available compares count against
int(remaining / min_bytes) instead of multiplying, avoiding overflow/wrap on
32-bit builds.
--
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]
