summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--NEWS3
-rw-r--r--ext/phar/tests/bug69441.phpt2
-rw-r--r--ext/phar/tests/bug77565.phpt13
-rw-r--r--ext/phar/tests/bug77565.zipbin0 -> 318 bytes
-rw-r--r--ext/phar/tests/zip/corrupt_003.phpt2
-rw-r--r--ext/phar/zip.c97
6 files changed, 77 insertions, 40 deletions
diff --git a/NEWS b/NEWS
index 60741d463a..6a12fe4a88 100644
--- a/NEWS
+++ b/NEWS
@@ -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
new file mode 100644
index 0000000000..134d571d7c
--- /dev/null
+++ b/ext/phar/tests/bug77565.zip
Binary files differ
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);