Copilot commented on code in PR #3856:
URL: https://github.com/apache/avro/pull/3856#discussion_r3564679785
##########
lang/php/lib/DataFile/AvroDataIOReader.php:
##########
@@ -276,17 +344,57 @@ 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. A
+ * varint longer than five bytes is malformed and is rejected as well.
+ *
+ * @throws AvroException if the declared length exceeds the limit or is
malformed
+ */
+ 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) {
+ // A Snappy uncompressed length is a uint32, encoded in at most
+ // five varint bytes; a longer encoding is malformed.
+ throw new AvroException('snappy uncompression failed -
malformed length header.');
+ }
+ }
+ }
Review Comment:
`ensureSnappyWithinLimit()` silently succeeds if the varint never terminates
(e.g., the input ends with all continuation bits set). That means a
malformed/truncated length header can bypass the declared-length guard
entirely. If the loop completes without hitting the terminating byte, this
should be treated as malformed and rejected.
##########
lang/php/lib/DataFile/AvroDataIOReader.php:
##########
@@ -276,17 +344,57 @@ 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));
Review Comment:
`snappyUncompress()` assumes the compressed block ends with a 4-byte CRC,
but if the block is truncated (< 4 bytes) `unpack()` returns false and `[1]`
will raise a warning/fatal ("Trying to access array offset on bool"), instead
of a clean `AvroException`. Add an explicit length/`unpack()` check and reuse
the payload slice for both the declared-length precheck and
`snappy_uncompress()`.
--
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]
