Bug #71506 [Asn->Ana]: inflate_add() does not detect truncated data

Edit report at https://bugs.php.net/bug.php?id=71506&edit=1 ID: 71506 Updated by: cmb@php.net Reported by: salsi at icosaedro dot it Summary: inflate_add() does not detect truncated data -Status: Assigned +Status: Analyzed Type: Bug Package: Zlib related Operating System: Slackware 14.1 PHP Version: Irrelevant -Assigned To: cmb +Assigned To: Block user comment: N Private report: N New Comment: Z(LIB)_FINISH is not meant to signal the end of compression; from the zlib manual[1] (emphasis mine): | However if all decompression is to be performed in a *single* | *step* (a single call of inflate), the parameter flush should be | set to Z_FINISH. Actually, zlib signals the end of compression by returning E_STREAM_END: | inflate() should normally be called until it returns | Z_STREAM_END or an error. However, the current API simply resets the stream when Z_STREAM_END is encountered and calls it a day[2]. One potential solution would be to store the latest result of calling inflate() in the php_zlib_context[3] and to verify that when the zlib.inflate resource is destroyed[4], and to report an error otherwise. However, that would break code using the seemingly common inflate_add($ctx, '', ZLIB_FINISH) at the end, because that would report Z_BUF_ERROR (as obviously no progress can be made if no input is added). Furthermore, unless the resource would be explicitly unassigned, it would be freed at the end of the request only, so the warning message would not be very helpful. Maybe even worse, calling inflate_add() with some unfinished input would result in a warning, even if the result wouldn't even be used. All in all, I think the incremental inflate API needs a redesign. [1] <http://www.zlib.net/manual.html> [2] <https://github.com/php/php-src/blob/PHP-7.0.9/ext/zlib/zlib.c#L967-L969> [3} <https://github.com/php/php-src/blob/PHP-7.0.9/ext/zlib/php_zlib.h#L48-L53> [4] <https://github.com/php/php-src/blob/PHP-7.0.9/ext/zlib/zlib.c#L76-L84> Previous Comments: ------------------------------------------------------------------------ [2016-08-01 13:49:36] cmb@php.net > It might be necessary to add something like inflate_close() […] Ah, there is alread ZLIB_FINISH; should have read the report more carefully. Will have a look at this issue. ------------------------------------------------------------------------ [2016-08-01 13:38:37] cmb@php.net It might be necessary to add something like inflate_close() to detect the end of the input, so the data could be verified. ------------------------------------------------------------------------ [2016-02-03 10:54:24] salsi at icosaedro dot it Description: ------------ The inflate_add() function is available as of PHP 7.0.0 and it incrementally decompresses DEFLATE, ZLIB and GZIP compressed data passed chunk by chunk. When GZIP compressed data are feed to this function, it succeeds detecting corrupted data, but it fails to detect trunked data, as the following test script proves. Test script: --------------- <?php error_reporting(-1); // $deflateContext = deflate_init(ZLIB_ENCODING_GZIP); // $compressed = deflate_add($deflateContext, "Data to compress", ZLIB_NO_FLUSH); // $compressed .= deflate_add($deflateContext, ", more data", ZLIB_NO_FLUSH); // $compressed .= deflate_add($deflateContext, ", and even more data!", ZLIB_FINISH); // Creates GZIP compressed data: $plain = "Data to compress, more data, and even more data!"; $compressed = gzencode($plain); echo "Compressed: ", wordwrap(bin2hex($compressed), 2, " ", TRUE), "\n"; // These tests succeeded with error, as expected: // Invalid checksum: // $compressed[strlen($compressed)-5] = 'x'; // --> E_WARNING: inflate_add(): data error // Invalid length: // $compressed[strlen($compressed)-2] = 'x'; // --> E_WARNING: inflate_add(): data error // Corrupted compressed data: // $compressed[strlen($compressed)/2] = 'x'; // --> E_WARNING: inflate_add(): data error // Following test FAILED to detect corrupted data: // Trunked lenght field: // $compressed = substr($compressed, 0, strlen($compressed)-4); // --> Data to compress, more data, and even more data! // NO ERROR SIGNALED // Trunked Adler32 and lenght fields: // $compressed = substr($compressed, 0, strlen($compressed)-8); // --> Data to compress, more data, and even more data! // NO ERROR SIGNALED // Trunked some data, Adler32 and lenght fields: $compressed = substr($compressed, 0, strlen($compressed)-12); // --> Data to compress, more data, and eve // TRUNKED RESULTING DATA, NO ERROR SIGNALED echo "Corrupted : ", wordwrap(bin2hex($compressed), 2, " ", TRUE), "\n"; $inflateContext = inflate_init(ZLIB_ENCODING_GZIP); $uncompressed = inflate_add($inflateContext, $compressed, ZLIB_NO_FLUSH); $uncompressed .= inflate_add($inflateContext, NULL, ZLIB_FINISH); echo $uncompressed; ?> Expected result: ---------------- E_WARNING: inflate_add(): data error Actual result: -------------- Data to compress, more data, and eve (note how the original plain string has been trunked, but no error is detected). ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=71506&edit=1