From 8f897f1040f00210f4a5cdd82a88a1fe3e558955 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 26 Aug 2019 12:37:29 +0200 Subject: [PATCH] Replace deprecated libzip functions We replace all deprecated libzip functions with their recommended substitutes, and add proper comment length checks including a test case. --- ext/zip/php_zip.c | 65 +++++++++++++++++--------- ext/zip/php_zip.h | 4 +- ext/zip/tests/oo_setcomment_error.phpt | 43 +++++++++++++++++ 3 files changed, 87 insertions(+), 25 deletions(-) create mode 100644 ext/zip/tests/oo_setcomment_error.phpt diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index 0af6f24240..e6918f93b1 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -80,10 +80,10 @@ static int le_zip_entry; #define PHP_ZIP_SET_FILE_COMMENT(za, index, comment, comment_len) \ if (comment_len == 0) { \ /* Passing NULL remove the existing comment */ \ - if (zip_set_file_comment(za, index, NULL, 0) < 0) { \ + if (zip_file_set_comment(za, index, NULL, 0, 0) < 0) { \ RETURN_FALSE; \ } \ - } else if (zip_set_file_comment(za, index, comment, comment_len) < 0) { \ + } else if (zip_file_set_comment(za, index, comment, comment_len, 0) < 0) { \ RETURN_FALSE; \ } \ RETURN_TRUE; @@ -408,7 +408,7 @@ static int php_zip_parse_options(zval *options, zend_long *remove_all_path, char #endif /* }}} */ -static int php_zip_status(struct zip *za) /* {{{ */ +static zend_long php_zip_status(struct zip *za) /* {{{ */ { #if LIBZIP_VERSION_MAJOR < 1 int zep, syp; @@ -426,7 +426,7 @@ static int php_zip_status(struct zip *za) /* {{{ */ } /* }}} */ -static int php_zip_status_sys(struct zip *za) /* {{{ */ +static zend_long php_zip_status_sys(struct zip *za) /* {{{ */ { #if LIBZIP_VERSION_MAJOR < 1 int zep, syp; @@ -444,9 +444,10 @@ static int php_zip_status_sys(struct zip *za) /* {{{ */ } /* }}} */ -static int php_zip_get_num_files(struct zip *za) /* {{{ */ +static zend_long php_zip_get_num_files(struct zip *za) /* {{{ */ { - return zip_get_num_files(za); + zip_int64_t num = zip_get_num_entries(za, 0); + return MIN(num, ZEND_LONG_MAX); } /* }}} */ @@ -784,7 +785,7 @@ static zend_object_handlers zip_object_handlers; static HashTable zip_prop_handlers; -typedef int (*zip_read_int_t)(struct zip *za); +typedef zend_long (*zip_read_int_t)(struct zip *za); typedef char *(*zip_read_const_char_t)(struct zip *za, int *len); typedef char *(*zip_read_const_char_from_ze_t)(ze_zip_object *obj); @@ -1177,7 +1178,7 @@ static PHP_NAMED_FUNCTION(zif_zip_open) } rsrc_int->index_current = 0; - rsrc_int->num_files = zip_get_num_files(rsrc_int->za); + rsrc_int->num_files = zip_get_num_entries(rsrc_int->za, 0); RETURN_RES(zend_register_resource(rsrc_int, le_zip_dir)); } @@ -1553,10 +1554,12 @@ static ZIPARCHIVE_METHOD(count) { struct zip *intern; zval *self = ZEND_THIS; + zip_int64_t num; ZIP_FROM_OBJECT(intern, self); - RETVAL_LONG(zip_get_num_files(intern)); + num = zip_get_num_entries(intern, 0); + RETVAL_LONG(MIN(num, ZEND_LONG_MAX)); } /* }}} */ @@ -1840,7 +1843,7 @@ static ZIPARCHIVE_METHOD(addFromString) } } - if (zip_add(intern, name, zs) == -1) { + if (zip_file_add(intern, name, zs, 0) == -1) { zip_source_free(zs); RETURN_FALSE; } else { @@ -1966,7 +1969,13 @@ static ZIPARCHIVE_METHOD(setArchiveComment) if (zend_parse_parameters(ZEND_NUM_ARGS(), "s", &comment, &comment_len) == FAILURE) { return; } - if (zip_set_archive_comment(intern, (const char *)comment, (int)comment_len)) { + + if (comment_len > 0xffff) { + php_error_docref(NULL, E_WARNING, "Comment must not exceed 65535 bytes"); + RETURN_FALSE; + } + + if (zip_set_archive_comment(intern, (const char *)comment, comment_len)) { RETURN_FALSE; } else { RETURN_TRUE; @@ -2019,6 +2028,11 @@ static ZIPARCHIVE_METHOD(setCommentName) php_error_docref(NULL, E_NOTICE, "Empty string as entry name"); } + if (comment_len > 0xffff) { + php_error_docref(NULL, E_WARNING, "Comment must not exceed 65535 bytes"); + RETURN_FALSE; + } + idx = zip_name_locate(intern, name, 0); if (idx < 0) { RETURN_FALSE; @@ -2045,6 +2059,11 @@ static ZIPARCHIVE_METHOD(setCommentIndex) return; } + if (comment_len > 0xffff) { + php_error_docref(NULL, E_WARNING, "Comment must not exceed 65535 bytes"); + RETURN_FALSE; + } + PHP_ZIP_STAT_INDEX(intern, index, 0, sb); PHP_ZIP_SET_FILE_COMMENT(intern, index, comment, comment_len); } @@ -2249,7 +2268,7 @@ static ZIPARCHIVE_METHOD(getCommentName) size_t name_len; int idx; zend_long flags = 0; - int comment_len = 0; + zip_uint32_t comment_len = 0; const char * comment; char *name; @@ -2269,8 +2288,8 @@ static ZIPARCHIVE_METHOD(getCommentName) RETURN_FALSE; } - comment = zip_get_file_comment(intern, idx, &comment_len, (int)flags); - RETURN_STRINGL((char *)comment, (zend_long)comment_len); + comment = zip_file_get_comment(intern, idx, &comment_len, (zip_flags_t)flags); + RETURN_STRINGL((char *)comment, comment_len); } /* }}} */ @@ -2282,7 +2301,7 @@ static ZIPARCHIVE_METHOD(getCommentIndex) zval *self = ZEND_THIS; zend_long index, flags = 0; const char * comment; - int comment_len = 0; + zip_uint32_t comment_len = 0; struct zip_stat sb; ZIP_FROM_OBJECT(intern, self); @@ -2293,8 +2312,8 @@ static ZIPARCHIVE_METHOD(getCommentIndex) } PHP_ZIP_STAT_INDEX(intern, index, 0, sb); - comment = zip_get_file_comment(intern, index, &comment_len, (int)flags); - RETURN_STRINGL((char *)comment, (zend_long)comment_len); + comment = zip_file_get_comment(intern, index, &comment_len, (zip_flags_t)flags); + RETURN_STRINGL((char *)comment, comment_len); } /* }}} */ @@ -2435,7 +2454,7 @@ static ZIPARCHIVE_METHOD(renameIndex) php_error_docref(NULL, E_NOTICE, "Empty string as new entry name"); RETURN_FALSE; } - if (zip_rename(intern, index, (const char *)new_name) != 0) { + if (zip_file_rename(intern, index, (const char *)new_name, 0) != 0) { RETURN_FALSE; } RETURN_TRUE; @@ -2465,7 +2484,7 @@ static ZIPARCHIVE_METHOD(renameName) PHP_ZIP_STAT_PATH(intern, name, name_len, 0, sb); - if (zip_rename(intern, sb.index, (const char *)new_name)) { + if (zip_file_rename(intern, sb.index, (const char *)new_name, 0)) { RETURN_FALSE; } RETURN_TRUE; @@ -2579,9 +2598,7 @@ static ZIPARCHIVE_METHOD(extractTo) php_stream_statbuf ssb; char *pathto; size_t pathto_len; - int ret, i; - - int nelems; + int ret; if (zend_parse_parameters(ZEND_NUM_ARGS(), "p|z", &pathto, &pathto_len, &zval_files) == FAILURE) { return; @@ -2600,6 +2617,8 @@ static ZIPARCHIVE_METHOD(extractTo) ZIP_FROM_OBJECT(intern, self); if (zval_files && (Z_TYPE_P(zval_files) != IS_NULL)) { + uint32_t nelems, i; + switch (Z_TYPE_P(zval_files)) { case IS_STRING: if (!php_zip_extract_file(intern, pathto, Z_STRVAL_P(zval_files), Z_STRLEN_P(zval_files))) { @@ -2632,7 +2651,7 @@ static ZIPARCHIVE_METHOD(extractTo) } } else { /* Extract all files */ - int filecount = zip_get_num_files(intern); + zip_int64_t i, filecount = zip_get_num_entries(intern, 0); if (filecount == -1) { php_error_docref(NULL, E_WARNING, "Illegal archive"); diff --git a/ext/zip/php_zip.h b/ext/zip/php_zip.h index 0ac9189583..0385c8d3e9 100644 --- a/ext/zip/php_zip.h +++ b/ext/zip/php_zip.h @@ -39,8 +39,8 @@ extern zend_module_entry zip_module_entry; typedef struct _ze_zip_rsrc { struct zip *za; - int index_current; - int num_files; + zip_uint64_t index_current; + zip_int64_t num_files; } zip_rsrc; typedef zip_rsrc * zip_rsrc_ptr; diff --git a/ext/zip/tests/oo_setcomment_error.phpt b/ext/zip/tests/oo_setcomment_error.phpt new file mode 100644 index 0000000000..a8f1e47ab9 --- /dev/null +++ b/ext/zip/tests/oo_setcomment_error.phpt @@ -0,0 +1,43 @@ +--TEST-- +setComment error behavior +--SKIPIF-- + +--FILE-- +open($file, ZIPARCHIVE::CREATE)) { + exit('failed'); +} + +$zip->addFromString('entry1.txt', 'entry #1'); +$zip->addFromString('entry2.txt', 'entry #2'); + +$longComment = str_repeat('a', 0x10000); + +var_dump($zip->setArchiveComment($longComment)); +var_dump($zip->setCommentName('entry1.txt', $longComment)); +var_dump($zip->setCommentIndex(1, $longComment)); + +$zip->close(); +?> +===DONE=== +--EXPECTF-- +Warning: ZipArchive::setArchiveComment(): Comment must not exceed 65535 bytes in %s on line %d +bool(false) + +Warning: ZipArchive::setCommentName(): Comment must not exceed 65535 bytes in %s on line %d +bool(false) + +Warning: ZipArchive::setCommentIndex(): Comment must not exceed 65535 bytes in %s on line %d +bool(false) +===DONE=== +--CLEAN-- + -- 2.40.0