diff --git a/NEWS b/NEWS index 3c7f104a24d3..310d8f91136a 100644 --- a/NEWS +++ b/NEWS @@ -56,6 +56,7 @@ PHP NEWS . Fixed a bypass of the magic ".phar" directory protection in Phar::addEmptyDir() for paths starting with "/.phar", while allowing non-magic directory names that merely share the ".phar" prefix. (Weilin Du) + . Fixed an integer underflow when parsing ZIP extra fields. (Weilin Du) - Reflection: . Preserve class-name case in ReflectionClass::getProperty() error messages diff --git a/ext/phar/tests/zip/zip_extra_underflow.phpt b/ext/phar/tests/zip/zip_extra_underflow.phpt new file mode 100644 index 000000000000..e37a3493b663 --- /dev/null +++ b/ext/phar/tests/zip/zip_extra_underflow.phpt @@ -0,0 +1,91 @@ +--TEST-- +Phar: ZIP extra field length must not underflow +--EXTENSIONS-- +phar +--FILE-- +getMTime(), "\n"; +} catch (Exception $e) { + echo $e->getMessage(), "\n"; +} +?> +--CLEAN-- + +--EXPECTF-- +phar error: Unable to process extra field header for file in central directory in zip-based phar "%szip_extra_underflow.zip" diff --git a/ext/phar/zip.c b/ext/phar/zip.c index 339e45e73088..f051c35a09a7 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -41,19 +41,30 @@ static inline void phar_write_16(char buffer[2], uint32_t value) # define PHAR_SET_32(var, value) phar_write_32(var, (uint32_t) (value)); # define PHAR_SET_16(var, value) phar_write_16(var, (uint16_t) (value)); -static int phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16_t len) /* {{{ */ +static int phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16_t extra_len) /* {{{ */ { union { phar_zip_extra_field_header header; phar_zip_unix3 unix3; phar_zip_unix_time time; } h; + size_t len = extra_len; size_t read; - do { + while (len) { + size_t header_size; + + if (len < sizeof(h.header)) { + return FAILURE; + } if (sizeof(h.header) != php_stream_read(fp, (char *) &h.header, sizeof(h.header))) { return FAILURE; } + len -= sizeof(h.header); + header_size = PHAR_GET_16(h.header.size); + if (header_size > len) { + return FAILURE; + } if (h.header.tag[0] == 'U' && h.header.tag[1] == 'T') { /* Unix timestamp header found. @@ -62,7 +73,6 @@ static int phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16 * We only store the modification time in the entry, so only read that. */ const size_t min_size = 5; - uint16_t header_size = PHAR_GET_16(h.header.size); if (header_size >= min_size) { read = php_stream_read(fp, &h.time.flags, min_size); if (read != min_size) { @@ -73,12 +83,11 @@ static int phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16 entry->timestamp = PHAR_GET_32(h.time.time); } - len -= header_size + 4; - /* Consume remaining bytes */ - if (header_size != read) { - php_stream_seek(fp, header_size - read, SEEK_CUR); + if (header_size != read && -1 == php_stream_seek(fp, header_size - read, SEEK_CUR)) { + return FAILURE; } + len -= header_size; continue; } /* Fallthrough to next if to skip header */ @@ -86,23 +95,36 @@ static int phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16 if (h.header.tag[0] != 'n' || h.header.tag[1] != 'u') { /* skip to next header */ - php_stream_seek(fp, PHAR_GET_16(h.header.size), SEEK_CUR); - len -= PHAR_GET_16(h.header.size) + 4; + if (header_size && -1 == php_stream_seek(fp, header_size, SEEK_CUR)) { + return FAILURE; + } + len -= header_size; continue; } /* unix3 header found */ - read = php_stream_read(fp, (char *) &(h.unix3.crc32), sizeof(h.unix3) - sizeof(h.header)); - len -= read + 4; + size_t unix3_size = sizeof(h.unix3) - sizeof(h.header); + size_t field_size = header_size; + if (field_size == unix3_size - sizeof(h.unix3.crc32)) { + /* Some archives omit the CRC32 from the unix3 size field. */ + field_size = unix3_size; + } + if (field_size < unix3_size || field_size > len) { + return FAILURE; + } - if (sizeof(h.unix3) - sizeof(h.header) != read) { + read = php_stream_read(fp, (char *) &(h.unix3.crc32), unix3_size); + if (unix3_size != read) { return FAILURE; } - if (PHAR_GET_16(h.unix3.size) > sizeof(h.unix3) - 4) { + if (field_size > unix3_size) { /* skip symlink filename - we may add this support in later */ - php_stream_seek(fp, PHAR_GET_16(h.unix3.size) - sizeof(h.unix3.size), SEEK_CUR); + if (-1 == php_stream_seek(fp, field_size - unix3_size, SEEK_CUR)) { + return FAILURE; + } } + len -= field_size; /* set permissions */ entry->flags &= PHAR_ENT_COMPRESSION_MASK; @@ -113,7 +135,7 @@ static int phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16 entry->flags |= PHAR_GET_16(h.unix3.perms) & PHAR_ENT_PERM_MASK; } - } while (len); + } return SUCCESS; }