diff options
-rw-r--r-- | NEWS | 3 | ||||
-rw-r--r-- | ext/phar/tests/bug69441.phpt | 2 | ||||
-rw-r--r-- | ext/phar/tests/bug77565.phpt | 13 | ||||
-rw-r--r-- | ext/phar/tests/bug77565.zip | bin | 0 -> 318 bytes | |||
-rw-r--r-- | ext/phar/tests/zip/corrupt_003.phpt | 2 | ||||
-rw-r--r-- | ext/phar/zip.c | 97 |
6 files changed, 77 insertions, 40 deletions
@@ -20,6 +20,9 @@ PHP NEWS . Fixed bug #77935 (Crash in mysqlnd_fetch_stmt_row_cursor when calling an SP with a cursor). (Nikita) +- Phar: + . Fixed bug #77565 (Incorrect locator detection in ZIP-based phars). (cmb) + 07 Jan 2021, PHP 7.4.14 - Core: diff --git a/ext/phar/tests/bug69441.phpt b/ext/phar/tests/bug69441.phpt index 91e2dcf146..7150af0f34 100644 --- a/ext/phar/tests/bug69441.phpt +++ b/ext/phar/tests/bug69441.phpt @@ -14,7 +14,7 @@ $r = new Phar($fname, 0); ==DONE== --EXPECTF-- -UnexpectedValueException: phar error: corrupted central directory entry, no magic signature in zip-based phar "%sbug69441.phar" in %sbug69441.php:%d +UnexpectedValueException: phar error: end of central directory not found in zip-based phar "%sbug69441.phar" in %sbug69441.php:%d Stack trace: #0 %s%ebug69441.php(%d): Phar->__construct('%s', 0) #1 {main} diff --git a/ext/phar/tests/bug77565.phpt b/ext/phar/tests/bug77565.phpt new file mode 100644 index 0000000000..e15e7ae647 --- /dev/null +++ b/ext/phar/tests/bug77565.phpt @@ -0,0 +1,13 @@ +--TEST-- +Bug #77565 (Incorrect locator detection in ZIP-based phars) +--SKIPIF-- +<?php +if (!extension_loaded('phar')) die('skip phar extension not available'); +?> +--FILE-- +<?php +$phar = new PharData(__DIR__ . '/bug77565.zip'); +var_dump($phar['1.zip']->getFilename()); +?> +--EXPECT-- +string(5) "1.zip" diff --git a/ext/phar/tests/bug77565.zip b/ext/phar/tests/bug77565.zip Binary files differnew file mode 100644 index 0000000000..134d571d7c --- /dev/null +++ b/ext/phar/tests/bug77565.zip diff --git a/ext/phar/tests/zip/corrupt_003.phpt b/ext/phar/tests/zip/corrupt_003.phpt index e076a4eb8f..9f5bcceeec 100644 --- a/ext/phar/tests/zip/corrupt_003.phpt +++ b/ext/phar/tests/zip/corrupt_003.phpt @@ -12,5 +12,5 @@ try { ?> ===DONE=== --EXPECTF-- -phar error: corrupt zip archive, zip file comment truncated in zip-based phar "%sfilecomment.zip" +phar error: end of central directory not found in zip-based phar "%sfilecomment.zip" ===DONE=== diff --git a/ext/phar/zip.c b/ext/phar/zip.c index 0bf873aff1..c52e87647d 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -161,6 +161,29 @@ static void phar_zip_u2d_time(time_t time, char *dtime, char *ddate) /* {{{ */ } /* }}} */ +static char *phar_find_eocd(const char *s, size_t n) +{ + const char *end = s + n + sizeof("PK\5\6") - 1 - sizeof(phar_zip_dir_end); + + /* search backwards for end of central directory signatures */ + do { + uint16_t comment_len; + const char *eocd_start = zend_memnrstr(s, "PK\5\6", sizeof("PK\5\6") - 1, end); + + if (eocd_start == NULL) { + return NULL; + } + ZEND_ASSERT(eocd_start + sizeof(phar_zip_dir_end) <= s + n); + comment_len = PHAR_GET_16(((phar_zip_dir_end *) eocd_start)->comment_len); + if (eocd_start + sizeof(phar_zip_dir_end) + comment_len == s + n) { + /* we can't be sure, but this looks like the proper EOCD signature */ + return (char *) eocd_start; + } + end = eocd_start; + } while (end > s); + return NULL; +} + /** * Does not check for a previously opened phar in the cache. * @@ -205,57 +228,55 @@ int phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, char *alia return FAILURE; } - while ((p=(char *) memchr(p + 1, 'P', (size_t) (size - (p + 1 - buf)))) != NULL) { - if ((p - buf) + sizeof(locator) <= (size_t)size && !memcmp(p + 1, "K\5\6", 3)) { - memcpy((void *)&locator, (void *) p, sizeof(locator)); - if (PHAR_GET_16(locator.centraldisk) != 0 || PHAR_GET_16(locator.disknumber) != 0) { - /* split archives not handled */ - php_stream_close(fp); - if (error) { - spprintf(error, 4096, "phar error: split archives spanning multiple zips cannot be processed in zip-based phar \"%s\"", fname); - } - return FAILURE; + if ((p = phar_find_eocd(buf, size)) != NULL) { + memcpy((void *)&locator, (void *) p, sizeof(locator)); + if (PHAR_GET_16(locator.centraldisk) != 0 || PHAR_GET_16(locator.disknumber) != 0) { + /* split archives not handled */ + php_stream_close(fp); + if (error) { + spprintf(error, 4096, "phar error: split archives spanning multiple zips cannot be processed in zip-based phar \"%s\"", fname); } + return FAILURE; + } - if (PHAR_GET_16(locator.counthere) != PHAR_GET_16(locator.count)) { - if (error) { - spprintf(error, 4096, "phar error: corrupt zip archive, conflicting file count in end of central directory record in zip-based phar \"%s\"", fname); - } - php_stream_close(fp); - return FAILURE; + if (PHAR_GET_16(locator.counthere) != PHAR_GET_16(locator.count)) { + if (error) { + spprintf(error, 4096, "phar error: corrupt zip archive, conflicting file count in end of central directory record in zip-based phar \"%s\"", fname); } + php_stream_close(fp); + return FAILURE; + } - mydata = pecalloc(1, sizeof(phar_archive_data), PHAR_G(persist)); - mydata->is_persistent = PHAR_G(persist); + mydata = pecalloc(1, sizeof(phar_archive_data), PHAR_G(persist)); + mydata->is_persistent = PHAR_G(persist); - /* read in archive comment, if any */ - if (PHAR_GET_16(locator.comment_len)) { + /* read in archive comment, if any */ + if (PHAR_GET_16(locator.comment_len)) { - metadata = p + sizeof(locator); + metadata = p + sizeof(locator); - if (PHAR_GET_16(locator.comment_len) != size - (metadata - buf)) { - if (error) { - spprintf(error, 4096, "phar error: corrupt zip archive, zip file comment truncated in zip-based phar \"%s\"", fname); - } - php_stream_close(fp); - pefree(mydata, mydata->is_persistent); - return FAILURE; + if (PHAR_GET_16(locator.comment_len) != size - (metadata - buf)) { + if (error) { + spprintf(error, 4096, "phar error: corrupt zip archive, zip file comment truncated in zip-based phar \"%s\"", fname); } + php_stream_close(fp); + pefree(mydata, mydata->is_persistent); + return FAILURE; + } - mydata->metadata_len = PHAR_GET_16(locator.comment_len); + mydata->metadata_len = PHAR_GET_16(locator.comment_len); - if (phar_parse_metadata(&metadata, &mydata->metadata, PHAR_GET_16(locator.comment_len)) == FAILURE) { - mydata->metadata_len = 0; - /* if not valid serialized data, it is a regular string */ + if (phar_parse_metadata(&metadata, &mydata->metadata, PHAR_GET_16(locator.comment_len)) == FAILURE) { + mydata->metadata_len = 0; + /* if not valid serialized data, it is a regular string */ - ZVAL_NEW_STR(&mydata->metadata, zend_string_init(metadata, PHAR_GET_16(locator.comment_len), mydata->is_persistent)); - } - } else { - ZVAL_UNDEF(&mydata->metadata); + ZVAL_NEW_STR(&mydata->metadata, zend_string_init(metadata, PHAR_GET_16(locator.comment_len), mydata->is_persistent)); } - - goto foundit; + } else { + ZVAL_UNDEF(&mydata->metadata); } + + goto foundit; } php_stream_close(fp); |