php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #71506 inflate_add() does not detect truncated data
Submitted: 2016-02-03 10:54 UTC Modified: 2016-08-02 15:18 UTC
Votes:2
Avg. Score:3.5 ± 0.5
Reproduced:1 of 1 (100.0%)
Same Version:0 (0.0%)
Same OS:0 (0.0%)
From: salsi at icosaedro dot it Assigned:
Status: Analyzed Package: Zlib related
PHP Version: Irrelevant OS: Slackware 14.1
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: salsi at icosaedro dot it
New email:
PHP Version: OS:

 

 [2016-02-03 10:54 UTC] 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).


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-08-01 13:38 UTC] cmb@php.net
-Assigned To: +Assigned To: rdlowrey
 [2016-08-01 13:38 UTC] 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-08-01 13:49 UTC] cmb@php.net
-Assigned To: rdlowrey +Assigned To: cmb
 [2016-08-01 13:49 UTC] 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-02 11:44 UTC] cmb@php.net
-Summary: inflate_add() does not detect corrupted compressed data +Summary: inflate_add() does not detect truncated data
 [2016-08-02 15:18 UTC] cmb@php.net
-Status: Assigned +Status: Analyzed -Assigned To: cmb +Assigned To:
 [2016-08-02 15:18 UTC] cmb@php.net
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>
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Mon Nov 25 07:01:31 2024 UTC