Copilot commented on code in PR #3856:
URL: https://github.com/apache/avro/pull/3856#discussion_r3567429177
##########
lang/php/lib/DataFile/AvroDataIOReader.php:
##########
@@ -220,18 +234,69 @@ 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('deflate uncompression failed.');
+ }
- if (false === $datum) {
- throw new AvroException('gzip uncompression failed.');
+ // Inflate in chunks and check the running total after each step so an
+ // over-large (or malicious) block is rejected before its whole output
is
+ // accumulated (each inflated chunk is still materialized), while
genuine
+ // decompression errors (inflate_add === false) are reported
distinctly.
+ // Pieces are collected and joined once at the end to avoid repeatedly
+ // reallocating a growing result string.
+ $pieces = [];
+ $total = 0;
+ $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('deflate uncompression failed.');
Review Comment:
Avoid using the error-suppression operator (@) on inflate_add(); it can hide
unexpected zlib warnings and is inconsistent with the rest of the PHP code here
(other codec helpers call into extensions without suppression and then handle
false returns). Let inflate_add() run normally and keep the explicit false
check/exception handling.
##########
lang/php/lib/DataFile/AvroDataIOReader.php:
##########
@@ -220,18 +234,69 @@ 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('deflate uncompression failed.');
+ }
- if (false === $datum) {
- throw new AvroException('gzip uncompression failed.');
+ // Inflate in chunks and check the running total after each step so an
+ // over-large (or malicious) block is rejected before its whole output
is
+ // accumulated (each inflated chunk is still materialized), while
genuine
+ // decompression errors (inflate_add === false) are reported
distinctly.
+ // Pieces are collected and joined once at the end to avoid repeatedly
+ // reallocating a growing result string.
+ $pieces = [];
+ $total = 0;
+ $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('deflate uncompression failed.');
+ }
+ $pieces[] = $out;
+ $total += strlen($out);
+ self::checkDecompressLength($total, $maxLength);
}
- return $datum;
+ $out = @inflate_add($context, '', ZLIB_FINISH);
+ if (false === $out) {
+ throw new AvroException('deflate uncompression failed.');
Review Comment:
Avoid using the error-suppression operator (@) on inflate_add(); it can mask
warnings that would otherwise help diagnose corrupt input or runtime issues.
Since this path already checks for false and throws an AvroException, the
suppression isn’t needed.
--
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]
