Fix #81490: ZipArchive::extractTo() may leak memory

cmb69 · cmb69 · commit 4d442714155a · 2021-09-30T15:41:05.000+02:00
We always need to free the CWD state. Closes GH-7536.
diff --git a/NEWS b/NEWS
@@ -32,6 +32,9 @@ PHP                                                                        NEWS
   . Fixed bug #70962 (XML_OPTION_SKIP_WHITE strips embedded whitespace).
     (Aliaksandr Bystry, cmb)
 
+- Zip:
+  . Fixed bug #81490 (ZipArchive::extractTo() may leak memory). (cmb, Remi)
+
 23 Sep 2021, PHP 7.4.24
 
 - Core:
diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c
@@ -161,11 +161,13 @@ static int php_zip_extract_file(struct zip * za, char *dest, char *file, size_t
 	virtual_file_ex(&new_state, file, NULL, CWD_EXPAND);
 	path_cleaned =  php_zip_make_relative_path(new_state.cwd, new_state.cwd_length);
 	if(!path_cleaned) {
+		CWD_STATE_FREE(new_state.cwd);
 		return 0;
 	}
 	path_cleaned_len = strlen(path_cleaned);
 
 	if (path_cleaned_len >= MAXPATHLEN || zip_stat(za, file, 0, &sb) != 0) {
+		CWD_STATE_FREE(new_state.cwd);
 		return 0;
 	}
 
@@ -200,8 +202,8 @@ static int php_zip_extract_file(struct zip * za, char *dest, char *file, size_t
 			efree(file_dirname_fullpath);
 			if (!is_dir_only) {
 				zend_string_release_ex(file_basename, 0);
-				CWD_STATE_FREE(new_state.cwd);
 			}
+			CWD_STATE_FREE(new_state.cwd);
 			return 0;
 		}
 	}
diff --git a/ext/zip/tests/bug81490.phpt b/ext/zip/tests/bug81490.phpt
@@ -0,0 +1,21 @@
+--TEST--
+Bug #81490 (ZipArchive::extractTo() may leak memory)
+--SKIPIF--
+<?php
+if (!extension_loaded("zip")) die("skip zip extension not available");
+?>
+--FILE--
+<?php
+$zip = new ZipArchive();
+$zip->open(__DIR__ . "/bug81490.zip", ZipArchive::CREATE|ZipArchive::OVERWRITE);
+$zip->addFromString("", "yada yada");
+mkdir(__DIR__ . "/bug81490");
+$zip->open(__DIR__ . "/bug81490.zip");
+$zip->extractTo(__DIR__ . "/bug81490", "");
+?>
+--EXPECT--
+--CLEAN--
+<?php
+@unlink(__DIR__ . "/bug81490.zip");
+@rmdir(__DIR__ . "/bug81490");
+?>

Original file line number	Diff line number	Diff line change
`@@ -161,11 +161,13 @@ static int php_zip_extract_file(struct zip * za, char dest, char file, size_t`
`161`	`161`	`virtual_file_ex(&new_state, file, NULL, CWD_EXPAND);`
`162`	`162`	`path_cleaned = php_zip_make_relative_path(new_state.cwd, new_state.cwd_length);`
`163`	`163`	`if(!path_cleaned) {`
	`164`	`+ CWD_STATE_FREE(new_state.cwd);`
`164`	`165`	`return 0;`
`165`	`166`	`}`
`166`	`167`	`path_cleaned_len = strlen(path_cleaned);`
`167`	`168`
`168`	`169`	`if (path_cleaned_len >= MAXPATHLEN \|\| zip_stat(za, file, 0, &sb) != 0) {`
	`170`	`+ CWD_STATE_FREE(new_state.cwd);`
`169`	`171`	`return 0;`
`170`	`172`	`}`
`171`	`173`
`@@ -200,8 +202,8 @@ static int php_zip_extract_file(struct zip * za, char dest, char file, size_t`
`200`	`202`	`efree(file_dirname_fullpath);`
`201`	`203`	`if (!is_dir_only) {`
`202`	`204`	`zend_string_release_ex(file_basename, 0);`
`203`		`- CWD_STATE_FREE(new_state.cwd);`
`204`	`205`	`}`
	`206`	`+ CWD_STATE_FREE(new_state.cwd);`
`205`	`207`	`return 0;`
`206`	`208`	`}`
`207`	`209`	`}`