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

go to bug id or search bugs for

Bug #75540 Segfault with libzip 1.3.1
Submitted: 2017-11-20 06:15 UTC Modified: 2017-11-20 08:26 UTC
From: remi@php.net Assigned: remi (profile)
Status: Closed Package: zip (PECL)
PHP Version: 7.0.25 OS: n/a
Private report: No CVE-ID: None
View Developer Edit
 [2017-11-20 06:15 UTC] remi@php.net 
Description:
------------
When building against newly released libzip 1.3.1

segfault during the test suite

(gdb) bt
#0  zip_source_free (src=0x7478742e) at zip_source_free.c:46
#1  0x00007fffec9212fe in zip_source_free (src=0x555555c9d530) at zip_source_free.c:68
#2  0x00007fffec919b97 in zip_discard (za=0x555555c9bef0) at zip_discard.c:54
#3  0x00007fffecb34271 in c_ziparchive_close (execute_data=<optimized out>, return_value=0x7ffff3814150) at /work/GIT/php_zip/php7/php_zip.c:1540
#4  0x0000555555827bce in ZEND_DO_FCALL_SPEC_HANDLER ()
#5  0x00005555557e242b in execute_ex ()
#6  0x0000555555836c67 in zend_execute ()
#7  0x00005555557a1913 in zend_execute_scripts ()
#8  0x00005555557404d0 in php_execute_script ()
#9  0x000055555583891c in do_cli ()
#10 0x000055555561ff29 in main ()

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-11-20 06:16 UTC] remi@php.net 
Changes in libzip

diff -ru libzip-1.3.0/lib/zip_close.c libzip-1.3.1/lib/zip_close.c
--- libzip-1.3.0/lib/zip_close.c        2017-05-17 19:41:59.000000000 +0200
+++ libzip-1.3.1/lib/zip_close.c        2017-11-13 10:59:12.000000000 +0100
@@ -218,14 +218,13 @@
 
     if (error) {
        zip_source_rollback_write(za->src);
-       return -1;
     }
 
     _zip_progress_end(za->progress);
 
     zip_discard(za);
     
-    return 0;
+    return error ? -1 : 0;
 }
 
 
In short, this mean return value from zip_close cannot be used to know if za need to be free.

In extension sources

	if ((err = zip_close(intern))) {
		php_error_docref(NULL, E_WARNING, "%s", zip_strerror(intern));
		zip_discard(intern);
	}

If we drop the zip_discard call, we fix the segfault but introduce a memory leak.

Need investigation with upstream.
 [2017-11-20 06:40 UTC] remi@php.net 
> In short, this mean return value from zip_close cannot be used to know if za need to be free.

Bad analysis.

The memory leak is fixed in libzip 1.3.1, so the zip_discard call is no more needed.
 [2017-11-20 06:45 UTC] remi@php.net 
Possible solution:

https://github.com/pierrejoye/php_zip/commit/82cf37939ca15fb9ec0f568e4e7f7fe6a5203651
 [2017-11-20 07:52 UTC] remi@php.net 
Automatic comment on behalf of remi
Revision: http://git.php.net/?p=php-src.git;a=commit;h=de47d4792fdefbcea023877c931cbdc15dd38963
Log: fix bug #75540 Segfault with libzip 1.3.1
 [2017-11-20 07:52 UTC] remi@php.net
-Status: Open +Status: Closed
 [2017-11-20 08:26 UTC] remi@php.net
-Status: Closed +Status: Assigned -Assigned To: +Assigned To: remi
 [2017-11-20 08:26 UTC] remi@php.net 
Re-open, as the fix is not ok (php_error_docref will raise a use after free)

libzip upstream is aware of this issue, and if looking at it
 [2017-11-20 08:43 UTC] remi@php.net 
Automatic comment on behalf of remi
Revision: http://git.php.net/?p=php-src.git;a=commit;h=702ef2736479af85ce17aec83c6cfc1fecccf326
Log: Better fix bug #75540 Segfault with libzip 1.3.1 - only 1.3.1 is affected - fix use after free
 [2017-11-20 08:43 UTC] remi@php.net
-Status: Assigned +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Sat Dec 21 15:01:29 2024 UTC