From 4d70e48b4e8b76d6c887e8df3c3093180f5b2d91 Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Thu, 12 Aug 2021 07:56:35 +0200 Subject: [PATCH 1/4] Fix #80833 ZipArchive::getStream doesn't use setPassword --- ext/zip/php_zip.c | 5 +---- ext/zip/php_zip.h | 2 +- ext/zip/tests/bug80833.phpt | 36 ++++++++++++++++++++++++++++++++++++ ext/zip/zip_stream.c | 31 ++++++------------------------- 4 files changed, 44 insertions(+), 30 deletions(-) create mode 100644 ext/zip/tests/bug80833.phpt diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index cf8506fa389a2..c3ad51ec67e2d 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -2887,7 +2887,6 @@ PHP_METHOD(ZipArchive, getStream) char *mode = "rb"; zend_string *filename; php_stream *stream; - ze_zip_object *obj; if (zend_parse_parameters(ZEND_NUM_ARGS(), "P", &filename) == FAILURE) { RETURN_THROWS(); @@ -2899,9 +2898,7 @@ PHP_METHOD(ZipArchive, getStream) RETURN_FALSE; } - obj = Z_ZIP_P(self); - - stream = php_stream_zip_open(obj->filename, ZSTR_VAL(filename), mode STREAMS_CC); + stream = php_stream_zip_open(intern, ZSTR_VAL(filename), mode STREAMS_CC); if (stream) { php_stream_to_zval(stream, return_value); } else { diff --git a/ext/zip/php_zip.h b/ext/zip/php_zip.h index e58bceba43ee4..9f44aa7db2860 100644 --- a/ext/zip/php_zip.h +++ b/ext/zip/php_zip.h @@ -75,7 +75,7 @@ static inline ze_zip_object *php_zip_fetch_object(zend_object *obj) { #define Z_ZIP_P(zv) php_zip_fetch_object(Z_OBJ_P((zv))) php_stream *php_stream_zip_opener(php_stream_wrapper *wrapper, const char *path, const char *mode, int options, zend_string **opened_path, php_stream_context *context STREAMS_DC); -php_stream *php_stream_zip_open(const char *filename, const char *path, const char *mode STREAMS_DC); +php_stream *php_stream_zip_open(struct zip *arch, const char *path, const char *mode STREAMS_DC); extern const php_stream_wrapper php_stream_zip_wrapper; diff --git a/ext/zip/tests/bug80833.phpt b/ext/zip/tests/bug80833.phpt new file mode 100644 index 0000000000000..08ff3c3896bd2 --- /dev/null +++ b/ext/zip/tests/bug80833.phpt @@ -0,0 +1,36 @@ +--TEST-- +Bug #80833 (ZipArchive::getStream doesn't use setPassword) +--SKIPIF-- + +--FILE-- +open(__DIR__ . "/80833.zip", ZipArchive::CREATE); +$create_zip->setPassword("default_password"); +$create_zip->addFromString("test.txt", "This is a test string."); +$create_zip->setEncryptionName("test.txt", ZipArchive::EM_AES_256, "first_password"); +$create_zip->addFromString("test2.txt", "This is another test string."); +$create_zip->setEncryptionName("test2.txt", ZipArchive::EM_AES_256, "second_password"); +$create_zip->close(); + +$extract_zip = new ZipArchive(); +$extract_zip->open(__DIR__ . "/80833.zip", ZipArchive::RDONLY); +$extract_zip->setPassword("first_password"); +$file_stream = $extract_zip->getStream("test.txt"); +var_dump(stream_get_contents($file_stream)); +fclose($file_stream); +$extract_zip->setPassword("second_password"); +$file_stream = $extract_zip->getStream("test2.txt"); +var_dump(stream_get_contents($file_stream)); +fclose($file_stream); +$extract_zip->close(); +?> +--CLEAN-- + +--EXPECT-- +string(22) "This is a test string." +string(28) "This is another test string." diff --git a/ext/zip/zip_stream.c b/ext/zip/zip_stream.c index 6c6e08918cddd..3ee314457a9ba 100644 --- a/ext/zip/zip_stream.c +++ b/ext/zip/zip_stream.c @@ -48,7 +48,7 @@ static ssize_t php_zip_ops_read(php_stream *stream, char *buf, size_t count) ssize_t n = 0; STREAM_DATA_FROM_STREAM(); - if (self->za && self->zf) { + if (self->zf) { n = zip_fread(self->zf, buf, count); if (n < 0) { #if LIBZIP_VERSION_MAJOR < 1 @@ -208,51 +208,32 @@ const php_stream_ops php_stream_zipio_ops = { }; /* {{{ php_stream_zip_open */ -php_stream *php_stream_zip_open(const char *filename, const char *path, const char *mode STREAMS_DC) +php_stream *php_stream_zip_open(struct zip *arch, const char *path, const char *mode STREAMS_DC) { struct zip_file *zf = NULL; - int err = 0; php_stream *stream = NULL; struct php_zip_stream_data_t *self; - struct zip *stream_za; if (strncmp(mode,"r", strlen("r")) != 0) { return NULL; } - if (filename) { - if (ZIP_OPENBASEDIR_CHECKPATH(filename)) { - return NULL; - } - - /* duplicate to make the stream za independent (esp. for MSHUTDOWN) */ - stream_za = zip_open(filename, ZIP_CREATE, &err); - if (!stream_za) { - return NULL; - } - - zf = zip_fopen(stream_za, path, 0); + if (arch) { + zf = zip_fopen(arch, path, 0); if (zf) { self = emalloc(sizeof(*self)); - self->za = stream_za; + self->za = NULL; /* to keep it open on stream close */ self->zf = zf; self->stream = NULL; self->cursor = 0; stream = php_stream_alloc(&php_stream_zipio_ops, self, NULL, mode); stream->orig_path = estrdup(path); - } else { - zip_close(stream_za); } } - if (!stream) { - return NULL; - } else { - return stream; - } - + return stream; } /* }}} */ From 0dad6b50b8a0e21488be13502b2433bd8eb4d6e9 Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Thu, 12 Aug 2021 08:25:15 +0200 Subject: [PATCH 2/4] also test using stream API --- ext/zip/tests/bug80833.phpt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ext/zip/tests/bug80833.phpt b/ext/zip/tests/bug80833.phpt index 08ff3c3896bd2..00974390f3053 100644 --- a/ext/zip/tests/bug80833.phpt +++ b/ext/zip/tests/bug80833.phpt @@ -15,6 +15,16 @@ $create_zip->addFromString("test2.txt", "This is another test string."); $create_zip->setEncryptionName("test2.txt", ZipArchive::EM_AES_256, "second_password"); $create_zip->close(); +// Stream API +$o = [ + 'zip' => [ + 'password' => "first_password", + ], +]; +$c = stream_context_create($o); +var_dump(file_get_contents("zip://" . __DIR__ . "/80833.zip#test.txt", false, $c)); + +// getStream method $extract_zip = new ZipArchive(); $extract_zip->open(__DIR__ . "/80833.zip", ZipArchive::RDONLY); $extract_zip->setPassword("first_password"); @@ -33,4 +43,5 @@ $extract_zip->close(); ?> --EXPECT-- string(22) "This is a test string." +string(22) "This is a test string." string(28) "This is another test string." From 1c5969865810690f9d7aa5ccea09f7e49ab8a8f8 Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Thu, 12 Aug 2021 08:46:23 +0200 Subject: [PATCH 3/4] skip with old libzip --- ext/zip/tests/bug80833.phpt | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/zip/tests/bug80833.phpt b/ext/zip/tests/bug80833.phpt index 00974390f3053..214efe23f3d9c 100644 --- a/ext/zip/tests/bug80833.phpt +++ b/ext/zip/tests/bug80833.phpt @@ -3,6 +3,7 @@ Bug #80833 (ZipArchive::getStream doesn't use setPassword) --SKIPIF-- --FILE-- Date: Thu, 12 Aug 2021 14:32:19 +0200 Subject: [PATCH 4/4] check error --- ext/zip/tests/bug80833.phpt | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ext/zip/tests/bug80833.phpt b/ext/zip/tests/bug80833.phpt index 214efe23f3d9c..56cabf816bb8a 100644 --- a/ext/zip/tests/bug80833.phpt +++ b/ext/zip/tests/bug80833.phpt @@ -36,13 +36,22 @@ $extract_zip->setPassword("second_password"); $file_stream = $extract_zip->getStream("test2.txt"); var_dump(stream_get_contents($file_stream)); fclose($file_stream); + +// Archive close before the stream +$extract_zip->setPassword("first_password"); +$file_stream = $extract_zip->getStream("test.txt"); $extract_zip->close(); +var_dump(stream_get_contents($file_stream)); +fclose($file_stream); ?> --CLEAN-- ---EXPECT-- +--EXPECTF-- string(22) "This is a test string." string(22) "This is a test string." string(28) "This is another test string." + +Warning: stream_get_contents(): Zip stream error: Containing zip archive was closed in %s +string(0) ""