iemejia commented on code in PR #3856:
URL: https://github.com/apache/avro/pull/3856#discussion_r3564495392
##########
lang/php/lib/autoload.php:
##########
@@ -27,6 +27,7 @@
include __DIR__.'/AvroUtil.php';
include __DIR__.'/DataFile/AvroDataIO.php';
+include __DIR__.'/DataFile/AvroDataIODecompressionSizeException.php';
include __DIR__.'/DataFile/AvroDataIOException.php';
include __DIR__.'/DataFile/AvroDataIOReader.php';
Review Comment:
Fixed in fc78dc1a22 — autoload.php now includes AvroDataIOException before
its subclass AvroDataIODecompressionSizeException, avoiding the fatal error
under the include-based loader.
##########
lang/php/lib/DataFile/AvroDataIOReader.php:
##########
@@ -220,17 +231,47 @@ private function readBlockHeader(): string|int
return $this->decoder->readLong();
}
+ /**
+ * The maximum number of bytes a single block is allowed to decompress to.
+ */
+ private static function maxDecompressLength(): int
+ {
+ $value = getenv(self::MAX_DECOMPRESS_LENGTH_ENV);
+ if (false !== $value && ctype_digit($value) && (int) $value > 0) {
+ return (int) $value;
+ }
+
+ return self::DEFAULT_MAX_DECOMPRESS_LENGTH;
+ }
+
+ /**
+ * @throws AvroDataIODecompressionSizeException if the length exceeds the
limit
+ */
+ private static function checkDecompressLength(int $length, int
$maxLength): void
+ {
+ if ($length > $maxLength) {
+ throw new AvroDataIODecompressionSizeException($maxLength);
+ }
+ }
+
/**
* @throws AvroException
*/
private function gzUncompress(string $compressed): string
{
- $datum = gzinflate($compressed);
+ $maxLength = self::maxDecompressLength();
+ // gzinflate caps its output at the given length: a block that would
+ // decompress to more than the limit yields false here without
+ // materializing the full (potentially huge) output. The '@' suppresses
+ // the "insufficient memory" notice zlib emits when the cap is hit.
+ $datum = @gzinflate($compressed, $maxLength + 1);
Review Comment:
Fixed in fc78dc1a22 — gzUncompress now streams via inflate_add in chunks:
genuine inflate errors (inflate_add === false) are reported as a gzip failure,
while only an actual over-limit output raises
AvroDataIODecompressionSizeException. No more misclassifying corrupt data.
##########
lang/php/lib/DataFile/AvroDataIOReader.php:
##########
@@ -220,17 +231,47 @@ private function readBlockHeader(): string|int
return $this->decoder->readLong();
}
+ /**
+ * The maximum number of bytes a single block is allowed to decompress to.
+ */
+ private static function maxDecompressLength(): int
+ {
+ $value = getenv(self::MAX_DECOMPRESS_LENGTH_ENV);
+ if (false !== $value && ctype_digit($value) && (int) $value > 0) {
+ return (int) $value;
+ }
Review Comment:
Fixed in fc78dc1a22 — switching to streaming inflate_add removes the
`$maxLength + 1` arithmetic entirely, so there is no float overflow near
PHP_INT_MAX.
##########
lang/php/lib/DataFile/AvroDataIOReader.php:
##########
@@ -276,17 +321,51 @@ private function snappyUncompress(string $compressed):
string
if (!extension_loaded('snappy')) {
throw new AvroException('Please install ext-snappy to use snappy
compression.');
}
+ $maxLength = self::maxDecompressLength();
+ // The Snappy block header declares the uncompressed length as a
varint;
+ // reject an over-large block before allocating for it.
+ $declared = self::snappyDeclaredLength(substr((string) $compressed, 0,
-4));
Review Comment:
Fixed in fc78dc1a22 — the snappy declared-length parse (now
ensureSnappyWithinLimit) caps the running value against the limit and treats
any 32-bit wrap to negative as over-limit, so the guard holds on 32-bit builds.
##########
lang/php/lib/DataFile/AvroDataIOReader.php:
##########
@@ -276,17 +321,51 @@ private function snappyUncompress(string $compressed):
string
if (!extension_loaded('snappy')) {
throw new AvroException('Please install ext-snappy to use snappy
compression.');
}
+ $maxLength = self::maxDecompressLength();
+ // The Snappy block header declares the uncompressed length as a
varint;
+ // reject an over-large block before allocating for it.
+ $declared = self::snappyDeclaredLength(substr((string) $compressed, 0,
-4));
+ if (null !== $declared) {
+ self::checkDecompressLength($declared, $maxLength);
+ }
$crc32 = unpack('N', substr((string) $compressed, -4))[1];
$datum = snappy_uncompress(substr((string) $compressed, 0, -4));
if (false === $datum) {
throw new AvroException('snappy uncompression failed.');
}
+ self::checkDecompressLength(strlen($datum), $maxLength);
+
if ($crc32 !== crc32($datum)) {
throw new AvroException('snappy uncompression failed - crc32
mismatch.');
}
return $datum;
}
+
+ /**
+ * Return the uncompressed length declared in a raw Snappy block header,
+ * which prefixes the data as a little-endian base-128 varint. Returns null
+ * if the header cannot be parsed.
+ */
+ private static function snappyDeclaredLength(string $data): ?int
+ {
+ $result = 0;
+ $shift = 0;
+ $length = strlen($data);
+ for ($i = 0; $i < $length; $i++) {
+ $byte = ord($data[$i]);
+ $result |= ($byte & 0x7F) << $shift;
+ if (0 === ($byte & 0x80)) {
+ return $result;
+ }
+ $shift += 7;
+ if ($shift > 63) {
+ break;
+ }
+ }
+
+ return null;
+ }
Review Comment:
Fixed in fc78dc1a22 — see above: ensureSnappyWithinLimit parses with
overflow protection and an early cap at the configured limit.
##########
lang/php/test/DataFileTest.php:
##########
@@ -386,6 +388,66 @@ public function
test_differing_schemas_with_complex_objects(): void
}
}
+ /**
+ * A block with a very high compression ratio can expand to far more memory
+ * than its compressed size; reading such a block must be rejected once its
+ * decompressed size would exceed the configured maximum.
+ */
+ public function test_deflate_block_decompression_limit(): void
Review Comment:
Fixed in fc78dc1a22 — added snappy/zstandard/bzip2 decompression-limit tests
(skipped when the extension is unavailable).
--
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]
