Fix bug #76524 - ZipArchive memory leak

timurib · weltling · commit 08f08858f3bc · 2018-07-10T11:18:37.000+02:00
Bugfix #76524: Free up zip internal state and adjust the tests for Windows

Bugfix #76524: Fix possible use after free for libzip 1.3.1

Bugfix #76524: Make the test independent of platform
diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c
@@ -1003,10 +1003,13 @@ static void php_zip_object_free_storage(zend_object *object) /* {{{ */
 	}
 	if (intern->za) {
 		if (zip_close(intern->za) != 0) {
+#if LIBZIP_VERSION_MAJOR == 1 && LIBZIP_VERSION_MINOR == 3 && LIBZIP_VERSION_MICRO == 1
+			php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", "zip_close have failed");
+#else
 			php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(intern->za));
-			return;
+			zip_discard(intern->za);
+#endif
 		}
-		intern->za = NULL;
 	}
 
 	if (intern->buffers_cnt>0) {
diff --git a/ext/zip/tests/bug76524.phpt b/ext/zip/tests/bug76524.phpt
@@ -0,0 +1,21 @@
+--TEST--
+ZipArchive Bug #76524 (memory leak with ZipArchive::OVERWRITE flag and empty archive)
+--SKIPIF--
+<?php
+if(!extension_loaded('zip')) die('skip');
+?>
+--FILE--
+<?php
+
+$filename = __DIR__ . '/nonexistent.zip';
+
+$zip = new ZipArchive();
+$zip->open($filename, ZipArchive::CREATE | ZipArchive::OVERWRITE);
+echo 'ok';
+
+/* Zip-related error messages depend on platform and libzip version,
+   so the regex is used to check that Zend MM does NOT show warnings
+   about leaks: */
+?>
+--EXPECTREGEX--
+ok((?!memory leaks detected).)*

Original file line number	Diff line number	Diff line change
`@@ -1003,10 +1003,13 @@ static void php_zip_object_free_storage(zend_object object) / {{{ */`
`1003`	`1003`	`}`
`1004`	`1004`	`if (intern->za) {`
`1005`	`1005`	`if (zip_close(intern->za) != 0) {`
	`1006`	`+#if LIBZIP_VERSION_MAJOR == 1 && LIBZIP_VERSION_MINOR == 3 && LIBZIP_VERSION_MICRO == 1`
	`1007`	`+ php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", "zip_close have failed");`
	`1008`	`+#else`
`1006`	`1009`	`php_error_docref(NULL, E_WARNING, "Cannot destroy the zip context: %s", zip_strerror(intern->za));`
`1007`		`- return;`
	`1010`	`+ zip_discard(intern->za);`
	`1011`	`+#endif`
`1008`	`1012`	`}`
`1009`		`- intern->za = NULL;`
`1010`	`1013`	`}`
`1011`	`1014`
`1012`	`1015`	`if (intern->buffers_cnt>0) {`