iemejia commented on code in PR #3856:
URL: https://github.com/apache/avro/pull/3856#discussion_r3564714264
##########
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:
Fixed. snappyUncompress() now rejects a block shorter than the 4-byte CRC
trailer and checks unpack()'s result before indexing, instead of slicing with
negative offsets and reading [1] off a false return. The payload slice is
computed once and reused for the precheck and snappy_uncompress(). Pushed in
1fe9871.
##########
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:
Fixed. ensureSnappyWithinLimit() now throws when the loop ends without a
terminating byte (input ends with all continuation bits set), so a
truncated/never-terminating length header is rejected as malformed rather than
silently succeeding. Pushed in 1fe9871.
--
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]
