iemejia commented on code in PR #3847:
URL: https://github.com/apache/avro/pull/3847#discussion_r3564260904
##########
lang/perl/lib/Avro/BinaryDecoder.pm:
##########
@@ -32,6 +32,31 @@ unless ($Config{use64bitint}) {
$complement = Math::BigInt->new("0b" . ("1" x 57) . ("0" x 7));
}
+## The block count of an array or map is read from the (potentially untrusted
or
+## truncated) input and drives allocation of the resulting collection. To guard
+## against unbounded memory allocation from a very large or malformed block
+## count, the number of items in a single decoded array or map is capped. This
+## mirrors the Java SDK's collection item limit. The default can be overridden
+## with the AVRO_MAX_COLLECTION_ITEMS environment variable, or by setting
+## $Avro::BinaryDecoder::MAX_COLLECTION_ITEMS directly.
+our $DEFAULT_MAX_COLLECTION_ITEMS = (2 ** 31) - 8;
+our $MAX_COLLECTION_ITEMS =
+ ( defined $ENV{AVRO_MAX_COLLECTION_ITEMS} &&
$ENV{AVRO_MAX_COLLECTION_ITEMS} =~ /\A[0-9]+\z/ )
+ ? $ENV{AVRO_MAX_COLLECTION_ITEMS} + 0
+ : $DEFAULT_MAX_COLLECTION_ITEMS;
+
+## Ensure that decoding the next block of $block_count items would not grow the
+## collection beyond $MAX_COLLECTION_ITEMS. Throws on a negative block count or
+## when the running total would exceed the limit.
+sub _check_collection_items {
+ my ($existing, $block_count) = @_;
Review Comment:
Fixed in 9499118f2f. Reworded the comment to clarify that callers pass the
normalized (non-negative) count — in the Avro encoding a negative count merely
signals a following block-size long whose absolute value is the count — and
that the negative check here is a defensive guard against malformed input.
##########
lang/perl/t/06_bin_decode_limits.t:
##########
@@ -0,0 +1,77 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#!/usr/bin/env perl
+
+# Decoding arrays and maps must bound the block count read from the input. The
+# block count drives allocation of the resulting collection, so a pathological
+# or truncated input declaring a very large block count must raise an error
+# instead of attempting an unbounded allocation.
+
+use strict;
+use warnings;
+use Avro::Schema;
+use Test::More;
+use Test::Exception;
+
+use_ok 'Avro::BinaryDecoder';
Review Comment:
Fixed in 9499118f2f. The test now clears `AVRO_MAX_COLLECTION_ITEMS` in a
`BEGIN` block before any Avro module is loaded, so the default-limit assertion
is independent of the runner environment. Verified by running the suite with
`AVRO_MAX_COLLECTION_ITEMS=5` set.
--
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]
