php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
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
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: remi@php.net
New email:
PHP Version: OS:

 

 [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 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: Mon Nov 11 07:01:30 2024 UTC