php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #40189 endless loop in zlib.inflate stream filter
Submitted: 2007-01-22 02:25 UTC Modified: 2008-01-12 21:37 UTC
From: cellog@php.net Assigned: cellog (profile)
Status: Closed Package: Zlib related
PHP Version: 5CVS-2007-01-22 (CVS) OS: kubuntu linux
Private report: No CVE-ID: None
View Developer Edit
 [2007-01-22 02:25 UTC] cellog@php.net 
Description:
------------
run the test pecl/phar/tests/phar_ctx_001.phpt or pecl/phar/tests/phar_oo_compressed_001.phpt

and you will see an endless loop.  The loop happens in ext/zlib/zlib_filter.c at line 83:

		while (bin < bucket->buflen) {
			desired = bucket->buflen - bin;
			if (desired > data->inbuf_len) {
				desired = data->inbuf_len;
			}
			memcpy(data->strm.next_in, bucket->buf + bin, desired);
			data->strm.avail_in = desired;

			status = inflate(&(data->strm), flags & PSFS_FLAG_FLUSH_CLOSE ? Z_FINISH : Z_SYNC_FLUSH);
			if (status != Z_OK && status != Z_STREAM_END) {
				/* Something bad happened */
				php_stream_bucket_delref(bucket TSRMLS_CC);
				return PSFS_ERR_FATAL;
			}
			desired -= data->strm.avail_in; /* desired becomes what we consumed this round through */
			data->strm.next_in = data->inbuf;
			data->strm.avail_in = 0;
			consumed += desired;
			bin += desired;

			if (data->strm.avail_out < data->outbuf_len) {
				php_stream_bucket *out_bucket;
				size_t bucketlen = data->outbuf_len - data->strm.avail_out;
				out_bucket = php_stream_bucket_new(stream, estrndup(data->outbuf, bucketlen), bucketlen, 1, 0 TSRMLS_CC);
				php_stream_bucket_append(buckets_out, out_bucket TSRMLS_CC);
				data->strm.avail_out = data->outbuf_len;
				data->strm.next_out = data->outbuf;
				exit_status = PSFS_PASS_ON;
			}
		}

the loop should exit when desired is set to 0, but doesn't.

The attached patch fixes this issue.

Reproduce code:
---------------
Index: ext/zlib/zlib_filter.c
===================================================================
RCS file: /repository/php-src/ext/zlib/zlib_filter.c,v
retrieving revision 1.6.2.2.2.3
diff -u -r1.6.2.2.2.3 zlib_filter.c
--- ext/zlib/zlib_filter.c      1 Jan 2007 09:36:10 -0000       1.6.2.2.2.3
+++ ext/zlib/zlib_filter.c      22 Jan 2007 02:23:39 -0000
@@ -99,6 +99,10 @@
                        data->strm.avail_in = 0;
                        consumed += desired;
                        bin += desired;
+                       if (!desired) {
+                               flags |= PSFS_FLAG_FLUSH_CLOSE;
+                               break;
+                       }

                        if (data->strm.avail_out < data->outbuf_len) {
                                php_stream_bucket *out_bucket;

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2007-01-22 07:17 UTC] tony2001@php.net 
Sara, could you look into this?
Btw, I can see the very same code in ext/bz2.
 [2007-01-22 08:55 UTC] mike@php.net 
Seems to work fine here... (incidentally same OS and PHP version):

mike@honeybadger:~/cvs/pecl/phar/tests$ php -d"phar.require_hash=0" -d"phar.readonly=0" phar_oo_compressed_001.phpt
--TEST--
Phar context
--SKIPIF--
--INI--
phar.require_hash=0
phar.readonly=0
--FILE--
string(1) "a"
bool(false)
string(1) "b"
bool(false)
string(1) "c"
bool(false)
string(5) "new a"
bool(false)
string(5) "new b"
bool(true)
string(1) "c"
bool(false)
string(5) "new d"
bool(false)
===DONE===
--CLEAN--
--EXPECTF--
string(1) "a"
bool(false)
string(1) "b"
bool(false)
string(1) "c"
bool(false)
string(5) "new a"
bool(false)
string(5) "new b"
bool(true)
string(1) "c"
bool(false)
string(5) "new d"
bool(false)
===DONE===
mike@honeybadger:~/cvs/pecl/phar/tests$ php -d"phar.require_hash=0" -d"phar.readonly=0" phar_ctx_001.phpt
--TEST--
Phar context
--SKIPIF--
--INI--
phar.require_hash=0
phar.readonly=0
--FILE--
string(1) "a"
bool(false)
string(1) "b"
bool(false)
string(1) "c"
bool(false)
string(5) "new a"
bool(false)
string(5) "new b"
bool(true)
string(1) "c"
bool(false)
string(5) "new d"
bool(false)
===DONE===
--CLEAN--
--EXPECTF--
string(1) "a"
bool(false)
string(1) "b"
bool(false)
string(1) "c"
bool(false)
string(5) "new a"
bool(false)
string(5) "new b"
bool(true)
string(1) "c"
bool(false)
string(5) "new d"
bool(false)
===DONE===

Tests that fail here:
FAILED TEST SUMMARY
---------------------------------------------------------------------
Phar::mapPhar valid file (bzip2) [/home/mike/cvs/pecl/phar/tests/015b.phpt]
Phar::mapPhar invalid file (gzipped file length is too short) [/home/mike/cvs/pecl/phar/tests/016.phpt]
Phar::mapPhar invalid file (gzipped file length is too short) [/home/mike/cvs/pecl/phar/tests/016b.phpt]
Phar: test that refcounting avoids problems with deleting a file [/home/mike/cvs/pecl/phar/tests/refcount1.phpt]
 [2007-01-22 09:46 UTC] tony2001@php.net 
I have to say, I can't reproduce it on my Suse either.
4 tests fail, but the ones you mentioned work fine.

Btw, phar_oo_compressed_001.phpt fails because of several leaks:

016+ /local/dev/php/5_2/main/streams/streams.c(386) : Stream of type 'MEMORY' 0x40203094 (path:(null)) was not closed
017+ /local/dev/php/5_2/main/streams/streams.c(386) : Stream of type 'TEMP' 0x402004f8 (path:(null)) was not closed

You need to close those streams when destroying the object.
 [2007-01-22 14:44 UTC] cellog@php.net 
cvs update -f "Jan 22 02:07:44 2007 UTC"

I committed a workaround for the failure, completely 
forgetting you needed the file for this bug, sorry.  This 
is the patch that needs to be reversed:

http://cvs.php.net/viewvc.cgi/pecl/phar/phar.c?r1=1.135&r2=1.136
 [2007-01-23 23:57 UTC] cellog@php.net 
not sure what this cvs update -f business was.  Forget 
that.  Use this to reproduce the bug:

cd pecl/phar
cvs update -D "Jan 22 01:00:00 2007 UTC"
ln -s ~/php5/ext/phar ~/pecl/phar
cd ~/php5
./configure --enable-debug --enable-phar --with-zlib
make cli
cd ext/phar/tests
~/php5/sapi/cli/php phar_ctx_001.phpt
~/php5/sapi/cli/php phar_ctx_001.phpt

or

~/php5/sapi/cli/php phar_oo_compressed_001.phpt
~/php5/sapi/cli/php phar_oo_compressed_001.phpt

(sometimes it only happened on the 2nd run for me)
 [2007-01-23 23:58 UTC] cellog@php.net 
last detail: I have a 64-bit machine, not sure if that 
matters or not, but there you go
 [2007-01-25 12:22 UTC] tony2001@php.net 
Patch committed (+ I ported the same patch to ext/bz2).
 [2008-01-12 20:03 UTC] cellog@php.net 
the fix for this bug unfortunately is incorrect, and can result in truncation of valid gzipped data.  The correct fix is to look for Z_STREAM_END and to flush if it is returned.  Currently, Z_OK and Z_STREAM_END are treated identically.  I need to look at bz2 and see if the same fix needs to happen there.
 [2008-01-12 21:37 UTC] cellog@php.net 
This bug has been fixed in CVS.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.

fixed in HEAD, PHP_5_2, PHP_5_3
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Thu Nov 21 13:01:29 2024 UTC