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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: cellog@php.net
New email:
PHP Version: OS:

 

 [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