iemejia commented on code in PR #3856:
URL: https://github.com/apache/avro/pull/3856#discussion_r3564664569
##########
lang/php/lib/DataFile/AvroDataIOReader.php:
##########
@@ -276,17 +340,54 @@ 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. Parsed with an
+ // early cap so it stays correct even if a 32-bit int would overflow.
+ self::ensureSnappyWithinLimit(substr((string) $compressed, 0, -4),
$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;
}
+
+ /**
+ * Reject a Snappy block whose declared uncompressed length (a
little-endian
+ * base-128 varint at the start of the block) exceeds $maxLength, before
+ * allocating for it. The running length is compared against the cap after
+ * every group, and any wrap to a negative value (32-bit int overflow) is
+ * treated as over the limit, so the guard holds on 32-bit builds too.
+ *
+ * @throws AvroDataIODecompressionSizeException if the declared length
exceeds the limit
+ */
+ private static function ensureSnappyWithinLimit(string $data, int
$maxLength): void
+ {
+ $result = 0;
+ $shift = 0;
+ $length = strlen($data);
+ for ($i = 0; $i < $length; $i++) {
+ $byte = ord($data[$i]);
+ $result += ($byte & 0x7F) << $shift;
+ if ($result < 0 || $result > $maxLength) {
+ throw new AvroDataIODecompressionSizeException($maxLength);
+ }
+ if (0 === ($byte & 0x80)) {
+ return; // declared length is within the limit
+ }
+ $shift += 7;
+ if ($shift > 28) {
+ return; // more than 5 bytes: malformed; the post-decompress
check will catch it
+ }
+ }
+ }
Review Comment:
Fixed. `ensureSnappyWithinLimit()` now throws `AvroException('snappy
uncompression failed - malformed length header.')` when the varint exceeds five
bytes, instead of silently returning; a uint32 Snappy length never needs more
than five bytes. Pushed in 0d74370.
##########
lang/php/lib/DataFile/AvroDataIOReader.php:
##########
@@ -220,16 +234,62 @@ 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();
+ $context = inflate_init(ZLIB_ENCODING_RAW);
+ if (false === $context) {
+ throw new AvroException('gzip uncompression failed.');
+ }
- if (false === $datum) {
+ // Inflate in chunks and check the running length after each step so an
+ // over-large (or malicious) block is rejected without materializing
the
+ // full output, while genuine decompression errors (inflate_add ===
false)
+ // are reported distinctly.
+ $datum = '';
+ $length = strlen($compressed);
+ for ($offset = 0; $offset < $length; $offset +=
self::INFLATE_CHUNK_SIZE) {
+ $piece = substr($compressed, $offset, self::INFLATE_CHUNK_SIZE);
+ $out = @inflate_add($context, $piece);
+ if (false === $out) {
+ throw new AvroException('gzip uncompression failed.');
+ }
+ $datum .= $out;
+ self::checkDecompressLength(strlen($datum), $maxLength);
+ }
+
+ $out = @inflate_add($context, '', ZLIB_FINISH);
+ if (false === $out) {
throw new AvroException('gzip uncompression failed.');
}
+ $datum .= $out;
+ self::checkDecompressLength(strlen($datum), $maxLength);
return $datum;
}
Review Comment:
Fixed. `gzUncompress()` now collects inflated chunks into an array and
`implode()`s once at the end, tracking the running total separately for the
per-chunk size check, avoiding repeated reallocation of a growing result
string. Pushed in 0d74370.
--
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]
