From 313cbec470237a0e6c404c3ee5e52c00af84799e Mon Sep 17 00:00:00 2001 From: igedeon Date: Sun, 20 Oct 2019 06:11:30 -0500 Subject: [PATCH] Fixed bug #77978 Wrong relative path on unpacking zip archives with ".","..",":" character sequences on different platforms. The fix tries to separate Windows absolute paths from Unix paths that might contain a colon just before an slash relaying on the fact that the semicolon on windows absolute paths is always in the same position. There might be a case for a relative Unix path that might look like a windows absolute path (x:\foo\bar) that might result in chopping the leading path (x:). I think this is acceptable and an improvement over the current bug in order to avoid using different conditions per operating system. This could result in different extraction paths depending on the OS, what might generally be undesirable, and could cause BC issues. This is partially based on PR #4160 and its comments. --- ext/zip/php_zip.c | 8 +- ext/zip/tests/bug77978_unix.phpt | 177 ++++++++++++++++++++++++++++ ext/zip/tests/bug77978_windows.phpt | 177 ++++++++++++++++++++++++++++ 3 files changed, 360 insertions(+), 2 deletions(-) create mode 100644 ext/zip/tests/bug77978_unix.phpt create mode 100644 ext/zip/tests/bug77978_windows.phpt diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index 6b29266d9b211..5dc5d21aab25a 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -110,6 +110,10 @@ static char * php_zip_make_relative_path(char *path, size_t path_len) /* {{{ */ return path + 1; } + if (path_len < 1 && path[0] == '.' && IS_SLASH(path[1])) { + return path + 2; + } + i = path_len; while (1) { @@ -121,8 +125,8 @@ static char * php_zip_make_relative_path(char *path, size_t path_len) /* {{{ */ return path; } - if (i >= 2 && (path[i -1] == '.' || path[i -1] == ':')) { - /* i is the position of . or :, add 1 for / */ + if ( (i >= 2 && path[i -1] == '.') || (i == 2 && path[i -1] == ':') ) { + /* i is the position of . or : (in Windows absolute paths), add 1 for / */ path_begin = path + i + 1; break; } diff --git a/ext/zip/tests/bug77978_unix.phpt b/ext/zip/tests/bug77978_unix.phpt new file mode 100644 index 0000000000000..f3cd051102fbd --- /dev/null +++ b/ext/zip/tests/bug77978_unix.phpt @@ -0,0 +1,177 @@ +--TEST-- +Bug #77978 Wrong relative path for :/ Unix tests +--SKIPIF-- + +--FILE-- +open($file, ZIPARCHIVE::CREATE); +foreach($pathList as $path) { + $zipWriter->addFromString($path, "contents"); +} +$zipWriter->close(); + + +$zipReader = new ZipArchive(); +$i = 0; +while($zipReader->open($file) !== true && $i < 30) { + ++$i; + if($i == 30) { + die("Can't open zip file {$file} for read."); + } + sleep(1); +} + +for ($i=0; $i < $zipReader->count(); $i++) { + $stat = $zipReader->statIndex( $i ); + $path=$stat['name']; + $dir=dirname($path); + + try{ + $zipReader->extractTo($target, $path); + } catch(Exception $e) { + //nothing + } +} +$zipReader->close(); + + +$resultList = [ + "dir/test:/filename1.txt", + "dir/test:a/filename2.txt", + "dir./test/filename3.txt", + "test/filename3.txt", + "dir../test/filename4.txt", + "test/filename4.txt", + "dir/test/filename5.txt", + "../abc/filename6.txt", + "abc/filename6.txt", + "abc/filename7.txt", + "/abc/filename8.txt", + "abc/filename9.txt", + ":abc/filename10.txt", + "ab:c/filename11.txt", + "abc:/filename12.txt", + "abc/.filename13.txt", + "abc/..filename14.txt", + "abc/../filename15.txt", + "filename15.txt", + "abc/../../filename16.txt", + "filename16.txt", + "abc/../../dir/filename17.txt", + "dir/filename17.txt", + "abc/./filename18.txt", + "abc/file:name19.txt", + "abc/file.name20.txt", + "abc//filename21.txt", + "C:abc/filename22.txt", + "abc/filename22.txt", + "C:\abc/filename23.txt", + "abc/filename23.txt", + "C:/abc/filename24.txt", + "abc/filename24.txt" +]; + +foreach ($resultList as $path) { + $result = file_exists($target . DIRECTORY_SEPARATOR . $path) ? 'found' : 'not found'; + printf("%s %s%s", $path, $result, PHP_EOL); +} + +$error_reporting = $error_reporting; +?> + +--CLEAN-- +isDir()){ + rmdir($file->getRealPath()); + } else { + unlink($file->getRealPath()); + } + } + rmdir($target); +} +?> +--EXPECT-- +dir/test:/filename1.txt found +dir/test:a/filename2.txt found +dir./test/filename3.txt not found +test/filename3.txt found +dir../test/filename4.txt not found +test/filename4.txt found +dir/test/filename5.txt found +../abc/filename6.txt not found +abc/filename6.txt found +abc/filename7.txt found +/abc/filename8.txt found +abc/filename9.txt found +:abc/filename10.txt found +ab:c/filename11.txt found +abc:/filename12.txt found +abc/.filename13.txt found +abc/..filename14.txt found +abc/../filename15.txt found +filename15.txt found +abc/../../filename16.txt not found +filename16.txt found +abc/../../dir/filename17.txt not found +dir/filename17.txt found +abc/./filename18.txt found +abc/file:name19.txt not found +abc/file.name20.txt found +abc//filename21.txt found +C:abc/filename22.txt found +abc/filename22.txt not found +C:\abc/filename23.txt found +abc/filename23.txt not found +C:/abc/filename24.txt found +abc/filename24.txt not found diff --git a/ext/zip/tests/bug77978_windows.phpt b/ext/zip/tests/bug77978_windows.phpt new file mode 100644 index 0000000000000..2da56675873ea --- /dev/null +++ b/ext/zip/tests/bug77978_windows.phpt @@ -0,0 +1,177 @@ +--TEST-- +Bug #77978 Wrong relative path for :/ Windows tests +--SKIPIF-- + +--FILE-- +open($file, ZIPARCHIVE::CREATE); +foreach($pathList as $path) { + $zipWriter->addFromString($path, "contents"); +} +$zipWriter->close(); + + +$zipReader = new ZipArchive(); +$i = 0; +while($zipReader->open($file) !== true && $i < 30) { + ++$i; + if($i == 30) { + die("Can't open zip file {$file} for read."); + } + sleep(1); +} + +for ($i=0; $i < $zipReader->count(); $i++) { + $stat = $zipReader->statIndex( $i ); + $path=$stat['name']; + $dir=dirname($path); + + try{ + $zipReader->extractTo($target, $path); + } catch(Exception $e) { + //nothing + } +} +$zipReader->close(); + + +$resultList = [ + "dir/test:/filename1.txt", + "dir/test:a/filename2.txt", + "dir./test/filename3.txt", + "test/filename3.txt", + "dir../test/filename4.txt", + "test/filename4.txt", + "dir/test/filename5.txt", + "../abc/filename6.txt", + "abc/filename6.txt", + "abc/filename7.txt", + "/abc/filename8.txt", + "abc/filename9.txt", + ":abc/filename10.txt", + "ab:c/filename11.txt", + "abc:/filename12.txt", + "abc/.filename13.txt", + "abc/..filename14.txt", + "abc/../filename15.txt", + "filename15.txt", + "abc/../../filename16.txt", + "filename16.txt", + "abc/../../dir/filename17.txt", + "dir/filename17.txt", + "abc/./filename18.txt", + "abc/file:name19.txt", + "abc/file.name20.txt", + "abc//filename21.txt", + "C:abc/filename22.txt", + "abc/filename22.txt", + "C:\abc/filename23.txt", + "abc/filename23.txt", + "C:/abc/filename24.txt", + "abc/filename24.txt" +]; + +foreach ($resultList as $path) { + $result = file_exists($target . DIRECTORY_SEPARATOR . $path) ? 'found' : 'not found'; + printf("%s %s%s", $path, $result, PHP_EOL); +} + +$error_reporting = $error_reporting; +?> + +--CLEAN-- +isDir()){ + rmdir($file->getRealPath()); + } else { + unlink($file->getRealPath()); + } + } + rmdir($target); +} +?> +--EXPECT-- +dir/test:/filename1.txt not found +dir/test:a/filename2.txt not found +dir./test/filename3.txt not found +test/filename3.txt found +dir../test/filename4.txt not found +test/filename4.txt found +dir/test/filename5.txt found +../abc/filename6.txt not found +abc/filename6.txt found +abc/filename7.txt found +/abc/filename8.txt found +abc/filename9.txt found +:abc/filename10.txt not found +ab:c/filename11.txt not found +abc:/filename12.txt not found +abc/.filename13.txt found +abc/..filename14.txt found +abc/../filename15.txt found +filename15.txt found +abc/../../filename16.txt not found +filename16.txt found +abc/../../dir/filename17.txt not found +dir/filename17.txt found +abc/./filename18.txt found +abc/file:name19.txt not found +abc/file.name20.txt found +abc//filename21.txt found +C:abc/filename22.txt not found +abc/filename22.txt found +C:\abc/filename23.txt not found +abc/filename23.txt found +C:/abc/filename24.txt not found +abc/filename24.txt found