From e1163f76c48b9ed7876ebf8ff9555a3cb4fbb1ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Tue, 19 Jan 2021 13:43:33 +0100 Subject: [PATCH 1/3] Bug #69477 ext/zip: Allow extracting to paths with dirs ending with dot The function `php_zip_make_relative_path()` used to stop on the first right-most occurrence of './' in the ZIP entry path. As a result, the `extractTo()` method didn't extract entries like `foo/bar./baz/file.txt` into correct location. --- ext/zip/php_zip.c | 14 ++++++-- ext/zip/tests/bug69477.phpt | 68 +++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 ext/zip/tests/bug69477.phpt diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index 21182068d1d7f..a49ca36b65e68 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -117,11 +117,21 @@ 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 >= 1 && path[i - 1] == ':') { path_begin = path + i + 1; break; } + + if (i == 2 && path[i - 2] == '.' && path[i - 1] == '.') { + path_begin = path + 3; + break; + } + + if (i >= 3 && IS_SLASH(path[i - 3]) && path[i - 2] == '.' && path[i - 1] == '.') { + path_begin = path + i + 1; + break; + } + i--; } diff --git a/ext/zip/tests/bug69477.phpt b/ext/zip/tests/bug69477.phpt new file mode 100644 index 0000000000000..a2ff7ffe0a49b --- /dev/null +++ b/ext/zip/tests/bug69477.phpt @@ -0,0 +1,68 @@ +--TEST-- +Bug #69477 (ZipArchive::extractTo() truncates path segments ending with dot) +--SKIPIF-- + +--FILE-- +open($zipfile, ZipArchive::CREATE)) { + exit('failed: unable to create archive'); +} + +// (string) Entry path in the ZIP => (string) Expected actual target path +$paths = [ + '.a/b/c/file01.txt' => '.a/b/c/file01.txt', + 'a./b/c/file02.txt' => 'a./b/c/file02.txt', + 'a/.b/c/file03.txt' => 'a/.b/c/file03.txt', + 'a/b./c/file04.txt' => 'a/b./c/file04.txt', + 'a/b../c/file05.txt' => 'a/b../c/file05.txt', + 'a/b.../c/file06.txt' => 'a/b.../c/file06.txt', + 'a/..b/c/file07.txt' => 'a/..b/c/file07.txt', + 'a/...b/c/file08.txt' => 'a/...b/c/file08.txt', + 'a/../b./c./file09.txt' => 'b./c./file09.txt', + '//../b./c./file10.txt' => 'b./c./file10.txt', + '/../b./c./file11.txt' => 'b./c./file11.txt', + 'C:/a./b./file12.txt' => 'a./b./file12.txt', + 'a/b:/c/file13.txt' => 'c/file13.txt', +]; + +foreach ($paths as $zippath => $realpath) { + $archive->addFromString($zippath, $zippath . ' => ' . $realpath); +} + +$archive->close(); + +$archive2 = new ZipArchive(); + +if (!$archive2->open($zipfile)) { + exit('failed: unable to open archive2'); +} + +$archive2->extractTo($dir); +$archive2->close(); + +foreach ($paths as $zippath => $realpath) { + if (!is_readable($dir . '/' . $realpath) || file_get_contents($dir . '/' . $realpath) !== $zippath . ' => ' . $realpath) { + exit('failed: ' . $zippath); + } +} + +echo 'ok'; +?> +--CLEAN-- + +--EXPECT-- +ok From 585e63851ba4191b015922084bd0f8daf8bcff15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Wed, 20 Jan 2021 20:04:04 +0100 Subject: [PATCH 2/3] Bug #69477 ext/zip: Do not extract files that can't be opened on Windows There are known issues with folder and files ending with dot on Windows. Similarly to how 7-zip deals with the situation, replace the trailing dot with underscore for such files. --- ext/zip/php_zip.c | 23 ++++++++++++++--- ext/zip/tests/bug69477.phpt | 50 ++++++++++++++++++++++++++----------- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index a49ca36b65e68..1bc8cb64f3ffa 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -102,8 +102,8 @@ static char * php_zip_make_relative_path(char *path, size_t path_len) /* {{{ */ return NULL; } - if (IS_SLASH(path[0])) { - return path + 1; + if (path_len == 1 && (path[0] == '.' || IS_SLASH(path[0]))) { + return NULL; } i = path_len; @@ -114,7 +114,12 @@ static char * php_zip_make_relative_path(char *path, size_t path_len) /* {{{ */ } if (!i) { - return path; + if (IS_SLASH(path[0])) { + path_begin = path + 1; + } else { + path_begin = path; + } + break; } if (i >= 1 && path[i - 1] == ':') { @@ -135,6 +140,18 @@ static char * php_zip_make_relative_path(char *path, size_t path_len) /* {{{ */ i--; } +#ifdef PHP_WIN32 + if (path[path_len - 1] == '.') { + path[path_len - 1] = '_'; + } + + for (i = 1; i < path_len; i++) { + if (IS_SLASH(path[i]) && path[i - 1] == '.') { + path[i - 1] = '_'; + } + } +#endif + return path_begin; } /* }}} */ diff --git a/ext/zip/tests/bug69477.phpt b/ext/zip/tests/bug69477.phpt index a2ff7ffe0a49b..c1ca2f509e460 100644 --- a/ext/zip/tests/bug69477.phpt +++ b/ext/zip/tests/bug69477.phpt @@ -19,21 +19,41 @@ if (!$archive->open($zipfile, ZipArchive::CREATE)) { } // (string) Entry path in the ZIP => (string) Expected actual target path -$paths = [ - '.a/b/c/file01.txt' => '.a/b/c/file01.txt', - 'a./b/c/file02.txt' => 'a./b/c/file02.txt', - 'a/.b/c/file03.txt' => 'a/.b/c/file03.txt', - 'a/b./c/file04.txt' => 'a/b./c/file04.txt', - 'a/b../c/file05.txt' => 'a/b../c/file05.txt', - 'a/b.../c/file06.txt' => 'a/b.../c/file06.txt', - 'a/..b/c/file07.txt' => 'a/..b/c/file07.txt', - 'a/...b/c/file08.txt' => 'a/...b/c/file08.txt', - 'a/../b./c./file09.txt' => 'b./c./file09.txt', - '//../b./c./file10.txt' => 'b./c./file10.txt', - '/../b./c./file11.txt' => 'b./c./file11.txt', - 'C:/a./b./file12.txt' => 'a./b./file12.txt', - 'a/b:/c/file13.txt' => 'c/file13.txt', -]; +if (PHP_OS_FAMILY === 'Windows') { + $paths = [ + '.a/b/c/file01.txt' => '.a/b/c/file01.txt', + 'a./b/c/file02.txt' => 'a_/b/c/file02.txt', + 'a/.b/c/file03.txt' => 'a/.b/c/file03.txt', + 'a/b./c/file04.txt' => 'a/b_/c/file04.txt', + 'a/b../c/file05.txt' => 'a/b._/c/file05.txt', + 'a/b.../c/file06.txt' => 'a/b.._/c/file06.txt', + 'a/..b/c/file07.txt' => 'a/..b/c/file07.txt', + 'a/...b/c/file08.txt' => 'a/...b/c/file08.txt', + 'a/../b./c./file09.txt' => 'b_/c_/file09.txt', + '//../b./c./file10.txt' => 'b_/c_/file10.txt', + '/../b./c./file11.txt' => 'b_/c_/file11.txt', + 'C:/a./b./file12.txt' => 'a_/b_/file12.txt', + 'a/b:/c/file13.txt' => 'c/file13.txt', + 'a/b/c/file14.' => 'a/b/c/file14_', + ]; +} else { + $paths = [ + '.a/b/c/file01.txt' => '.a/b/c/file01.txt', + 'a./b/c/file02.txt' => 'a./b/c/file02.txt', + 'a/.b/c/file03.txt' => 'a/.b/c/file03.txt', + 'a/b./c/file04.txt' => 'a/b./c/file04.txt', + 'a/b../c/file05.txt' => 'a/b../c/file05.txt', + 'a/b.../c/file06.txt' => 'a/b.../c/file06.txt', + 'a/..b/c/file07.txt' => 'a/..b/c/file07.txt', + 'a/...b/c/file08.txt' => 'a/...b/c/file08.txt', + 'a/../b./c./file09.txt' => 'b./c./file09.txt', + '//../b./c./file10.txt' => 'b./c./file10.txt', + '/../b./c./file11.txt' => 'b./c./file11.txt', + 'C:/a./b./file12.txt' => 'a./b./file12.txt', + 'a/b:/c/file13.txt' => 'c/file13.txt', + 'a/b/c/file14.' => 'a/b/c/file14.', + ]; +} foreach ($paths as $zippath => $realpath) { $archive->addFromString($zippath, $zippath . ' => ' . $realpath); From d172596e47c1488085d657066ddddae48d340826 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Fri, 12 Feb 2021 22:02:54 +0100 Subject: [PATCH 3/3] Bug #69477 ext/zip: Fix handling of file names with colon sign If the colon sign is used in a drive relative path (such as C:temp/file.txt), only the part following the colon should be used as the filename. --- ext/zip/php_zip.c | 11 ++++++++--- ext/zip/tests/bug69477.phpt | 2 ++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index 1bc8cb64f3ffa..212138f1d52b2 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -102,19 +102,19 @@ static char * php_zip_make_relative_path(char *path, size_t path_len) /* {{{ */ return NULL; } - if (path_len == 1 && (path[0] == '.' || IS_SLASH(path[0]))) { + if (path_len == 1 && (path[0] == '.' || IS_SLASH(path[0]) || path[0] == ':')) { return NULL; } i = path_len; while (1) { - while (i > 0 && !IS_SLASH(path[i])) { + while (i > 0 && !(IS_SLASH(path[i]) || path[i] == ':')) { i--; } if (!i) { - if (IS_SLASH(path[0])) { + if (IS_SLASH(path[0]) || path[0] == ':') { path_begin = path + 1; } else { path_begin = path; @@ -122,6 +122,11 @@ static char * php_zip_make_relative_path(char *path, size_t path_len) /* {{{ */ break; } + if (i == 1 && path[i] == ':') { + path_begin = path + i + 1; + break; + } + if (i >= 1 && path[i - 1] == ':') { path_begin = path + i + 1; break; diff --git a/ext/zip/tests/bug69477.phpt b/ext/zip/tests/bug69477.phpt index c1ca2f509e460..5839311681610 100644 --- a/ext/zip/tests/bug69477.phpt +++ b/ext/zip/tests/bug69477.phpt @@ -35,6 +35,7 @@ if (PHP_OS_FAMILY === 'Windows') { 'C:/a./b./file12.txt' => 'a_/b_/file12.txt', 'a/b:/c/file13.txt' => 'c/file13.txt', 'a/b/c/file14.' => 'a/b/c/file14_', + 'C:a./b./file15.txt' => 'a_/b_/file15.txt', ]; } else { $paths = [ @@ -52,6 +53,7 @@ if (PHP_OS_FAMILY === 'Windows') { 'C:/a./b./file12.txt' => 'a./b./file12.txt', 'a/b:/c/file13.txt' => 'c/file13.txt', 'a/b/c/file14.' => 'a/b/c/file14.', + 'C:a./b./file15.txt' => 'a./b./file15.txt', ]; }