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

We always need to free the CWD state.
pierrejoye · Oct 1, 2021 · 80bcec9 · 80bcec9
1 parent 7353897
commit 80bcec9
Show file tree

Hide file tree

Showing 8 changed files with 41 additions and 7 deletions.
diff --git a/package.xml b/package.xml
@@ -27,7 +27,7 @@
   </stability>
   <license>PHP 3.01</license>
   <notes>
--
+- Fix #81490 ZipArchive::extractTo() may leak memory. (cmb, Remi)
   </notes>
   <contents>
     <dir name="/">
@@ -79,6 +79,7 @@
         <file name="bug80863.phpt" role="test"/>
         <file name="bug81420.phpt" role="test"/>
         <file name="bug81420.zip" role="test"/>
+        <file name="bug81490.phpt" role="test"/>
         <file name="compression_methods.phpt" role="test"/>
         <file name="compression_methods.zip" role="test"/>
         <file name="doubleclose.phpt" role="test"/>

diff --git a/php5/php_zip.c b/php5/php_zip.c
@@ -183,11 +183,13 @@ static int php_zip_extract_file(struct zip * za, char *dest, char *file, int fil
 	virtual_file_ex(&new_state, file, NULL, CWD_EXPAND TSRMLS_CC);
 	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;
 	}
 
@@ -223,8 +225,8 @@ static int php_zip_extract_file(struct zip * za, char *dest, char *file, int fil
 			efree(file_dirname_fullpath);
 			if (!is_dir_only) {
 				efree(file_basename);
-				CWD_STATE_FREE(new_state.cwd);
 			}
+			CWD_STATE_FREE(new_state.cwd);
 			return 0;
 		}
 	}

diff --git a/php7/php_zip.c b/php7/php_zip.c
@@ -165,11 +165,13 @@ static int php_zip_extract_file(struct zip * za, char *dest, char *file, int fil
 	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;
 	}
 
@@ -204,8 +206,8 @@ static int php_zip_extract_file(struct zip * za, char *dest, char *file, int fil
 			efree(file_dirname_fullpath);
 			if (!is_dir_only) {
 				zend_string_release(file_basename);
-				CWD_STATE_FREE(new_state.cwd);
 			}
+			CWD_STATE_FREE(new_state.cwd);
 			return 0;
 		}
 	}

diff --git a/php73/php_zip.c b/php73/php_zip.c
@@ -165,11 +165,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;
 	}
 
@@ -204,8 +206,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/php74/php_zip.c b/php74/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/php8/php_zip.c b/php8/php_zip.c
@@ -149,11 +149,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;
 	}
 
@@ -188,8 +190,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/php81/php_zip.c b/php81/php_zip.c
@@ -149,11 +149,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;
 	}
 
@@ -188,8 +190,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/tests/bug81490.phpt b/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");
+?>