From 9faaee66fa493372c7340b1ab05f8fd115131a42 Mon Sep 17 00:00:00 2001
From: Stanislav Malyshev <stas@php.net>
Date: Sun, 5 Apr 2015 15:07:36 -0700
Subject: [PATCH] Fixed bug #69324 (Buffer Over-read in unserialize when
parsing Phar)
---
ext/phar/phar.c | 65 ++++++++++++++++++++-----------------------
ext/phar/phar_internal.h | 2 +-
ext/phar/tests/bug69324.phar | Bin 0 -> 269 bytes
ext/phar/tests/bug69324.phpt | 17 +++++++++++
4 files changed, 48 insertions(+), 36 deletions(-)
create mode 100644 ext/phar/tests/bug69324.phar
create mode 100644 ext/phar/tests/bug69324.phpt
diff --git a/ext/phar/phar.c b/ext/phar/phar.c
index ec82351..bf0c985 100644
--- a/ext/phar/phar.c
+++ b/ext/phar/phar.c
@@ -601,25 +601,18 @@ int phar_open_parsed_phar(char *fname, int fname_len, char *alias, int alias_len
*
* data is the serialized zval
*/
-int phar_parse_metadata(char **buffer, zval **metadata, int zip_metadata_len TSRMLS_DC) /* {{{ */
+int phar_parse_metadata(char **buffer, zval **metadata, php_uint32 zip_metadata_len TSRMLS_DC) /* {{{ */
{
const unsigned char *p;
- php_uint32 buf_len;
php_unserialize_data_t var_hash;
- if (!zip_metadata_len) {
- PHAR_GET_32(*buffer, buf_len);
- } else {
- buf_len = zip_metadata_len;
- }
-
- if (buf_len) {
+ if (zip_metadata_len) {
ALLOC_ZVAL(*metadata);
INIT_ZVAL(**metadata);
p = (const unsigned char*) *buffer;
PHP_VAR_UNSERIALIZE_INIT(var_hash);
- if (!php_var_unserialize(metadata, &p, p + buf_len, &var_hash TSRMLS_CC)) {
+ if (!php_var_unserialize(metadata, &p, p + zip_metadata_len, &var_hash TSRMLS_CC)) {
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
zval_ptr_dtor(metadata);
*metadata = NULL;
@@ -631,19 +624,14 @@ int phar_parse_metadata(char **buffer, zval **metadata, int zip_metadata_len TSR
if (PHAR_G(persist)) {
/* lazy init metadata */
zval_ptr_dtor(metadata);
- *metadata = (zval *) pemalloc(buf_len, 1);
- memcpy(*metadata, *buffer, buf_len);
- *buffer += buf_len;
+ *metadata = (zval *) pemalloc(zip_metadata_len, 1);
+ memcpy(*metadata, *buffer, zip_metadata_len);
return SUCCESS;
}
} else {
*metadata = NULL;
}
- if (!zip_metadata_len) {
- *buffer += buf_len;
- }
-
return SUCCESS;
}
/* }}}*/
@@ -664,6 +652,7 @@ static int phar_parse_pharfile(php_stream *fp, char *fname, int fname_len, char
phar_entry_info entry;
php_uint32 manifest_len, manifest_count, manifest_flags, manifest_index, tmp_len, sig_flags;
php_uint16 manifest_ver;
+ php_uint32 len;
long offset;
int sig_len, register_alias = 0, temp_alias = 0;
char *signature = NULL;
@@ -1029,16 +1018,21 @@ static int phar_parse_pharfile(php_stream *fp, char *fname, int fname_len, char
mydata->is_persistent = PHAR_G(persist);
/* check whether we have meta data, zero check works regardless of byte order */
+ PHAR_GET_32(buffer, len);
if (mydata->is_persistent) {
- PHAR_GET_32(buffer, mydata->metadata_len);
- if (phar_parse_metadata(&buffer, &mydata->metadata, mydata->metadata_len TSRMLS_CC) == FAILURE) {
- MAPPHAR_FAIL("unable to read phar metadata in .phar file \"%s\"");
- }
- } else {
- if (phar_parse_metadata(&buffer, &mydata->metadata, 0 TSRMLS_CC) == FAILURE) {
- MAPPHAR_FAIL("unable to read phar metadata in .phar file \"%s\"");
+ mydata->metadata_len = len;
+ if(!len) {
+ /* FIXME: not sure why this is needed but removing it breaks tests */
+ PHAR_GET_32(buffer, len);
}
}
+ if(len > endbuffer - buffer) {
+ MAPPHAR_FAIL("internal corruption of phar \"%s\" (trying to read past buffer end)");
+ }
+ if (phar_parse_metadata(&buffer, &mydata->metadata, len TSRMLS_CC) == FAILURE) {
+ MAPPHAR_FAIL("unable to read phar metadata in .phar file \"%s\"");
+ }
+ buffer += len;
/* set up our manifest */
zend_hash_init(&mydata->manifest, manifest_count,
@@ -1073,7 +1067,7 @@ static int phar_parse_pharfile(php_stream *fp, char *fname, int fname_len, char
entry.manifest_pos = manifest_index;
}
- if (buffer + entry.filename_len + 20 > endbuffer) {
+ if (entry.filename_len + 20 > endbuffer - buffer) {
MAPPHAR_FAIL("internal corruption of phar \"%s\" (truncated manifest entry)");
}
@@ -1109,19 +1103,20 @@ static int phar_parse_pharfile(php_stream *fp, char *fname, int fname_len, char
entry.flags |= PHAR_ENT_PERM_DEF_DIR;
}
+ PHAR_GET_32(buffer, len);
if (entry.is_persistent) {
- PHAR_GET_32(buffer, entry.metadata_len);
- if (!entry.metadata_len) buffer -= 4;
- if (phar_parse_metadata(&buffer, &entry.metadata, entry.metadata_len TSRMLS_CC) == FAILURE) {
- pefree(entry.filename, entry.is_persistent);
- MAPPHAR_FAIL("unable to read file metadata in .phar file \"%s\"");
- }
+ entry.metadata_len = len;
} else {
- if (phar_parse_metadata(&buffer, &entry.metadata, 0 TSRMLS_CC) == FAILURE) {
- pefree(entry.filename, entry.is_persistent);
- MAPPHAR_FAIL("unable to read file metadata in .phar file \"%s\"");
- }
+ entry.metadata_len = 0;
+ }
+ if (len > endbuffer - buffer) {
+ MAPPHAR_FAIL("internal corruption of phar \"%s\" (truncated manifest entry)");
+ }
+ if (phar_parse_metadata(&buffer, &entry.metadata, len TSRMLS_CC) == FAILURE) {
+ pefree(entry.filename, entry.is_persistent);
+ MAPPHAR_FAIL("unable to read file metadata in .phar file \"%s\"");
}
+ buffer += len;
entry.offset = entry.offset_abs = offset;
offset += entry.compressed_filesize;
diff --git a/ext/phar/phar_internal.h b/ext/phar/phar_internal.h
index c9306c1..fcfc864 100644
--- a/ext/phar/phar_internal.h
+++ b/ext/phar/phar_internal.h
@@ -570,7 +570,7 @@ int phar_mount_entry(phar_archive_data *phar, char *filename, int filename_len,
char *phar_find_in_include_path(char *file, int file_len, phar_archive_data **pphar TSRMLS_DC);
char *phar_fix_filepath(char *path, int *new_len, int use_cwd TSRMLS_DC);
phar_entry_info * phar_open_jit(phar_archive_data *phar, phar_entry_info *entry, char **error TSRMLS_DC);
-int phar_parse_metadata(char **buffer, zval **metadata, int zip_metadata_len TSRMLS_DC);
+int phar_parse_metadata(char **buffer, zval **metadata, php_uint32 zip_metadata_len TSRMLS_DC);
void destroy_phar_manifest_entry(void *pDest);
int phar_seek_efp(phar_entry_info *entry, off_t offset, int whence, off_t position, int follow_links TSRMLS_DC);
php_stream *phar_get_efp(phar_entry_info *entry, int follow_links TSRMLS_DC);
--
2.1.4
From 12d3bdee3dfa6605024a72080d8a17c165c5ed24 Mon Sep 17 00:00:00 2001
From: Stanislav Malyshev <stas@php.net>
Date: Sat, 11 Apr 2015 16:42:16 -0700
Subject: [PATCH] Additional fix for bug #69324
Not so happy about duplication but needed due to bug #69429
---
ext/phar/phar.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/ext/phar/phar.c b/ext/phar/phar.c
index bf0c985..c5c8b46 100644
--- a/ext/phar/phar.c
+++ b/ext/phar/phar.c
@@ -598,27 +598,28 @@ int phar_open_parsed_phar(char *fname, int fname_len, char *alias, int alias_len
*
* Meta-data is in this format:
* [len32][data...]
- *
+ *
* data is the serialized zval
*/
int phar_parse_metadata(char **buffer, zval **metadata, php_uint32 zip_metadata_len TSRMLS_DC) /* {{{ */
{
- const unsigned char *p;
php_unserialize_data_t var_hash;
if (zip_metadata_len) {
+ const unsigned char *p, *p_buff = estrndup(*buffer, zip_metadata_len);
+ p = p_buff;
ALLOC_ZVAL(*metadata);
INIT_ZVAL(**metadata);
- p = (const unsigned char*) *buffer;
PHP_VAR_UNSERIALIZE_INIT(var_hash);
if (!php_var_unserialize(metadata, &p, p + zip_metadata_len, &var_hash TSRMLS_CC)) {
+ efree(p_buff);
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
zval_ptr_dtor(metadata);
*metadata = NULL;
return FAILURE;
}
-
+ efree(p_buff);
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
if (PHAR_G(persist)) {
@@ -641,7 +642,7 @@ int phar_parse_metadata(char **buffer, zval **metadata, php_uint32 zip_metadata_
*
* Parse a new one and add it to the cache, returning either SUCCESS or
* FAILURE, and setting pphar to the pointer to the manifest entry
- *
+ *
* This is used by phar_open_from_filename to process the manifest, but can be called
* directly.
*/
@@ -2212,7 +2213,7 @@ last_time:
/**
* Process a phar stream name, ensuring we can handle any of:
- *
+ *
* - whatever.phar
* - whatever.phar.gz
* - whatever.phar.bz2
--
2.1.4
From cee97220285fd7b955a58617b3e0300ec104ed87 Mon Sep 17 00:00:00 2001
From: Dmitry Stogov <dmitry@zend.com>
Date: Tue, 14 Apr 2015 15:47:26 +0300
Subject: [PATCH] Fixed recently introduced memory leak
---
ext/phar/phar.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/ext/phar/phar.c b/ext/phar/phar.c
index c5c8b46..223bfe8 100644
--- a/ext/phar/phar.c
+++ b/ext/phar/phar.c
@@ -1111,6 +1111,7 @@ static int phar_parse_pharfile(php_stream *fp, char *fname, int fname_len, char
entry.metadata_len = 0;
}
if (len > endbuffer - buffer) {
+ pefree(entry.filename, entry.is_persistent);
MAPPHAR_FAIL("internal corruption of phar \"%s\" (truncated manifest entry)");
}
if (phar_parse_metadata(&buffer, &entry.metadata, len TSRMLS_CC) == FAILURE) {
--
2.1.4