-
Notifications
You must be signed in to change notification settings - Fork 1.8k
AVRO-4281: [php] Bound collection size when decoding arrays and maps #3846
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
de74b31
3bd1337
7bd2e3a
175b42f
3062721
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| <?php | ||
|
|
||
| /** | ||
| * 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. | ||
| */ | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Apache\Avro\Datum; | ||
|
|
||
| use Apache\Avro\AvroException; | ||
|
|
||
| /** | ||
| * Raised when an array or map declares more items than the configured maximum. | ||
| * | ||
| * The block count of an array or map is read from the (potentially untrusted or | ||
| * truncated) input and drives allocation of the resulting collection. This | ||
| * exception guards against unbounded memory allocation from a very large or | ||
| * malformed block count. | ||
| */ | ||
| class AvroIOCollectionSizeException extends AvroException | ||
| { | ||
| public function __construct(int $maxItems) | ||
| { | ||
| parent::__construct( | ||
| sprintf('Cannot read collections larger than %d items.', $maxItems) | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,10 +43,44 @@ | |
| */ | ||
| class AvroIODatumReader | ||
| { | ||
| /** | ||
| * Default upper bound on the number of items in a single decoded array or | ||
| * map. The block count of an array or map is read from the (potentially | ||
| * untrusted or truncated) input and drives allocation of the resulting | ||
| * collection. Reading a collection that declares more items than this limit | ||
| * throws an {@see AvroIOCollectionSizeException} instead of attempting a | ||
| * potentially huge allocation. Mirrors the Java SDK's collection item limit. | ||
| */ | ||
| public const DEFAULT_MAX_COLLECTION_ITEMS = 2147483639; // Integer.MAX_VALUE - 8, matching the Java SDK | ||
|
|
||
| /** | ||
| * Name of the environment variable used to override the default maximum | ||
| * number of items permitted in a single decoded array or map. | ||
| */ | ||
| public const MAX_COLLECTION_ITEMS_ENV = 'AVRO_MAX_COLLECTION_ITEMS'; | ||
|
|
||
| private int $maxCollectionItems; | ||
|
|
||
| public function __construct( | ||
| private ?AvroSchema $writersSchema = null, | ||
| private ?AvroSchema $readersSchema = null | ||
| ) { | ||
| $this->maxCollectionItems = self::defaultMaxCollectionItems(); | ||
| } | ||
|
|
||
| /** | ||
| * Set the maximum number of items permitted in a single decoded array or map. | ||
| * | ||
| * @throws \InvalidArgumentException if $maxCollectionItems is negative | ||
| */ | ||
| public function setMaxCollectionItems(int $maxCollectionItems): void | ||
| { | ||
| if ($maxCollectionItems < 0) { | ||
| throw new \InvalidArgumentException( | ||
| sprintf('maxCollectionItems must not be negative, got %d.', $maxCollectionItems) | ||
| ); | ||
| } | ||
| $this->maxCollectionItems = $maxCollectionItems; | ||
| } | ||
|
|
||
| public function setWritersSchema(AvroSchema $schema): void | ||
|
|
@@ -285,11 +319,14 @@ public function readArray( | |
| ): array { | ||
| $items = []; | ||
| $blockCount = $decoder->readLong(); | ||
| while (0 !== $blockCount) { | ||
| // Loose comparison: on 32-bit builds with GMP, readLong() may return a | ||
| // numeric string, so a strict check against 0 would never terminate. | ||
| while (0 != $blockCount) { | ||
| if ($blockCount < 0) { | ||
| $blockCount = -$blockCount; | ||
| $decoder->readLong(); // Read (and ignore) block size | ||
| } | ||
| $this->checkCollectionItems(count($items), $blockCount); | ||
| for ($i = 0; $i < $blockCount; $i++) { | ||
| $items[] = $this->readData( | ||
| $writersSchema->items(), | ||
|
|
@@ -312,6 +349,7 @@ public function readMap( | |
| AvroIOBinaryDecoder $decoder | ||
| ): array { | ||
| $items = []; | ||
| $itemsRead = 0; | ||
| $pair_count = $decoder->readLong(); | ||
| while (0 != $pair_count) { | ||
| if ($pair_count < 0) { | ||
|
|
@@ -320,6 +358,10 @@ public function readMap( | |
| $decoder->readLong(); | ||
| } | ||
|
|
||
| // Track the number of decoded pairs rather than count($items): repeated | ||
| // keys collapse in the result and would otherwise let the cumulative | ||
| // check be bypassed by a stream that keeps rewriting the same key. | ||
| $this->checkCollectionItems($itemsRead, $pair_count); | ||
| for ($i = 0; $i < $pair_count; $i++) { | ||
|
Comment on lines
358
to
365
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch — fixed in 7bd2e3a. |
||
| $key = $decoder->readString(); | ||
| $items[$key] = $this->readData( | ||
|
|
@@ -328,6 +370,7 @@ public function readMap( | |
| $decoder | ||
| ); | ||
| } | ||
| $itemsRead += $pair_count; | ||
| $pair_count = $decoder->readLong(); | ||
| } | ||
|
|
||
|
|
@@ -530,6 +573,35 @@ public function readDefaultValue(AvroSchema $fieldSchema, mixed $defaultValue): | |
| } | ||
| } | ||
|
|
||
| private static function defaultMaxCollectionItems(): int | ||
| { | ||
| $env = getenv(self::MAX_COLLECTION_ITEMS_ENV); | ||
| if (false !== $env && ctype_digit($env)) { | ||
| return (int) $env; | ||
| } | ||
|
|
||
| return self::DEFAULT_MAX_COLLECTION_ITEMS; | ||
| } | ||
|
|
||
| /** | ||
| * Guard against unbounded allocation when decoding an array or map. | ||
| * | ||
| * @param int $existing the number of items already decoded for the collection | ||
| * @param int $blockCount the number of items in the next block; this is the | ||
| * normalized (non-negative) count | ||
| * | ||
| * @throws AvroIOCollectionSizeException if the block count is negative or the | ||
| * running total would exceed the maximum | ||
| */ | ||
| private function checkCollectionItems(int $existing, int $blockCount): void | ||
| { | ||
| // Use subtraction rather than $existing + $blockCount so the comparison | ||
| // cannot overflow when the limit is configured close to PHP_INT_MAX. | ||
| if ($blockCount < 0 || $blockCount > $this->maxCollectionItems - $existing) { | ||
| throw new AvroIOCollectionSizeException($this->maxCollectionItems); | ||
| } | ||
| } | ||
|
|
||
| private function readDecimal(string $bytes, int $scale): string | ||
| { | ||
| $mostSignificantBit = ord($bytes[0]) & 0x80; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch — fixed in 175b42f.
readArraynow uses a loose comparison (0 != $blockCount) so the loop terminates whenreadLong()returns a numeric string under GMP on 32-bit, matchingreadMap.