diff --git a/NEWS b/NEWS index 2bbe43032ece..363c52506403 100644 --- a/NEWS +++ b/NEWS @@ -164,6 +164,7 @@ PHP NEWS ignored. (ndossche) . Fixed a bypass of the magic ".phar" directory protection in Phar::addEmptyDir() for paths starting with "/.phar". (Weilin Du) + . Fixed an integer underflow when parsing ZIP extra fields. (Weilin Du) . Phar::addEmptyDir() now allows non-magic directory names that merely share the ".phar" prefix. (Weilin Du) . Support overridden methods in SplFileInfo for getMTime() and getPathname() @@ -254,6 +255,8 @@ PHP NEWS (Weilin Du) . getenv() and putenv() now raises a ValueError when the first argument contains null bytes. (Weilin Du) + . dl() now raises a ValueError when the $extension_filename argument + contains null bytes. (Weilin Du) . parse_str() now raises a ValueError when the $string argument contains null bytes. (Weilin Du) . proc_open() now raises a ValueError when the $cwd argument contains diff --git a/UPGRADING b/UPGRADING index bdadc6efbefc..a9b1f34eae5a 100644 --- a/UPGRADING +++ b/UPGRADING @@ -151,6 +151,8 @@ PHP 8.6 UPGRADE NOTES argument value is passed. . getenv() and putenv() now raises a ValueError when the first argument contains null bytes. + . dl() now raises a ValueError when the $extension_filename argument + contains null bytes. . parse_str() now raises a ValueError when the $string argument contains null bytes. . linkinfo() now raises a ValueError when the $path argument is empty. @@ -233,6 +235,10 @@ PHP 8.6 UPGRADE NOTES tcp_keepintvl and tcp_keepcnt that allow setting socket keepalive options. . Allowed casting casting filtered streams as file descriptor for select. + . Added the "write_seek_mode stream" filter parameter for the bz2, iconv, + zlib, and string stream filters. This parameter must be set via an + associative array where the key is "write_seek_mode stream" and the + value is one of the following strings "preserve", "reset", or "strict". - URI: . Added Uri\Rfc3986\Uri:getUriType() and Uri\WhatWg\Url:isSpecialScheme(). diff --git a/ext/pdo_odbc/odbc_stmt.c b/ext/pdo_odbc/odbc_stmt.c index dfc42147c30f..2fe47cef6e68 100644 --- a/ext/pdo_odbc/odbc_stmt.c +++ b/ext/pdo_odbc/odbc_stmt.c @@ -702,8 +702,9 @@ static int odbc_stmt_get_col(pdo_stmt_t *stmt, int colno, zval *result, enum pdo } ssize_t to_fetch_byte = to_fetch_len + 1; char *buf2 = emalloc(to_fetch_byte); - zend_string *str = zend_string_init(C->data, to_fetch_byte, 0); - size_t used = to_fetch_len; + ssize_t seed_len = to_fetch_len > (LONG_COLUMN_BUFFER_SIZE - 1) ? (LONG_COLUMN_BUFFER_SIZE - 1) : to_fetch_len; + zend_string *str = zend_string_init(C->data, seed_len + 1, 0); + size_t used = seed_len; do { C->fetched_len = 0; diff --git a/ext/pdo_odbc/tests/gh22349.phpt b/ext/pdo_odbc/tests/gh22349.phpt new file mode 100644 index 000000000000..58219dea0ac6 --- /dev/null +++ b/ext/pdo_odbc/tests/gh22349.phpt @@ -0,0 +1,45 @@ +--TEST-- +GH-22349 (Heap over-read fetching a long column past the internal buffer) +--EXTENSIONS-- +pdo_odbc +--SKIPIF-- + +--FILE-- +setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT); + +$db->exec('DROP TABLE test_gh22349'); +if (false === $db->exec('CREATE TABLE test_gh22349 (data text)') + && false === $db->exec('CREATE TABLE test_gh22349 (data CLOB)') + && false === $db->exec('CREATE TABLE test_gh22349 (data longtext)')) { + die("BORK: no large text column type available here: " . implode(", ", $db->errorInfo()) . "\n"); +} + +$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + +// The driver fetches a long column into an internal buffer of roughly one +// memory page and reassembles the remainder. Exercise values that span and +// exceed that buffer so the seeded length must match the bytes present. +foreach ([4096, 8192, 65536] as $len) { + $db->exec('DELETE FROM test_gh22349'); + $text = str_repeat('A', $len); + $db->exec("INSERT INTO test_gh22349 VALUES ('$text')"); + $got = $db->query('SELECT data FROM test_gh22349')->fetchColumn(); + printf("%d: %s\n", $len, ($got === $text) ? 'ok' : ('MISMATCH len=' . strlen($got))); +} +?> +--CLEAN-- +exec('DROP TABLE test_gh22349'); +?> +--EXPECT-- +4096: ok +8192: ok +65536: ok 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 7811865e3e69..5aa3f552b2ab 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -39,19 +39,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 zend_result phar_zip_process_extra(php_stream *fp, phar_entry_info *entry, uint16_t len) /* {{{ */ +static zend_result 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. @@ -60,7 +71,6 @@ static zend_result phar_zip_process_extra(php_stream *fp, phar_entry_info *entry * 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) { @@ -71,12 +81,11 @@ static zend_result phar_zip_process_extra(php_stream *fp, phar_entry_info *entry 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 */ @@ -84,23 +93,36 @@ static zend_result phar_zip_process_extra(php_stream *fp, phar_entry_info *entry 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; @@ -111,7 +133,7 @@ static zend_result phar_zip_process_extra(php_stream *fp, phar_entry_info *entry entry->flags |= PHAR_GET_16(h.unix3.perms) & PHAR_ENT_PERM_MASK; } - } while (len); + } return SUCCESS; } diff --git a/ext/standard/dl.c b/ext/standard/dl.c index a6d0ced6fa86..ca8ba57a16e9 100644 --- a/ext/standard/dl.c +++ b/ext/standard/dl.c @@ -43,7 +43,7 @@ PHPAPI PHP_FUNCTION(dl) size_t filename_len; ZEND_PARSE_PARAMETERS_START(1, 1) - Z_PARAM_STRING(filename, filename_len) + Z_PARAM_PATH(filename, filename_len) ZEND_PARSE_PARAMETERS_END(); if (!PG(enable_dl)) { diff --git a/ext/standard/proc_open.c b/ext/standard/proc_open.c index 111111406799..bd7a2e58854f 100644 --- a/ext/standard/proc_open.c +++ b/ext/standard/proc_open.c @@ -1377,7 +1377,6 @@ PHP_FUNCTION(proc_open) if (newprocok == FALSE) { DWORD dw = GetLastError(); - close_all_descriptors(descriptors, ndesc); char *msg = php_win32_error_to_msg(dw); php_error_docref(NULL, E_WARNING, "CreateProcess failed: %s", msg); php_win32_error_msg_free(msg); @@ -1394,7 +1393,6 @@ PHP_FUNCTION(proc_open) if (close_parentends_of_pipes(&factions, descriptors, ndesc) == FAILURE) { posix_spawn_file_actions_destroy(&factions); - close_all_descriptors(descriptors, ndesc); goto exit_fail; } @@ -1414,7 +1412,6 @@ PHP_FUNCTION(proc_open) } posix_spawn_file_actions_destroy(&factions); if (r != 0) { - close_all_descriptors(descriptors, ndesc); php_error_docref(NULL, E_WARNING, "posix_spawn() failed: %s", strerror(r)); goto exit_fail; } @@ -1456,7 +1453,6 @@ PHP_FUNCTION(proc_open) _exit(127); } else if (child < 0) { /* Failed to fork() */ - close_all_descriptors(descriptors, ndesc); php_error_docref(NULL, E_WARNING, "Fork failed: %s", strerror(errno)); goto exit_fail; } @@ -1546,6 +1542,9 @@ PHP_FUNCTION(proc_open) } else { exit_fail: _php_free_envp(env); + if (descriptors) { + close_all_descriptors(descriptors, ndesc); + } RETVAL_FALSE; } diff --git a/ext/standard/tests/general_functions/dl_null_bytes.phpt b/ext/standard/tests/general_functions/dl_null_bytes.phpt new file mode 100644 index 000000000000..7f251393ba3b --- /dev/null +++ b/ext/standard/tests/general_functions/dl_null_bytes.phpt @@ -0,0 +1,14 @@ +--TEST-- +dl() rejects null bytes in extension filename +--FILE-- +getMessage(), "\n"; +} + +?> +--EXPECT-- +dl(): Argument #1 ($extension_filename) must not contain any null bytes diff --git a/ext/standard/tests/general_functions/proc_open_fd_leak_on_setup_failure.phpt b/ext/standard/tests/general_functions/proc_open_fd_leak_on_setup_failure.phpt new file mode 100644 index 000000000000..e072f75a82d2 --- /dev/null +++ b/ext/standard/tests/general_functions/proc_open_fd_leak_on_setup_failure.phpt @@ -0,0 +1,20 @@ +--TEST-- +proc_open() does not leak file descriptors when descriptor setup fails mid-spec +--SKIPIF-- + +--FILE-- + ["pipe", "r"], 1 => ["bogus_type"]], $pipes); +} +$after = count(scandir("/proc/self/fd")); +var_dump($after <= $before + 2); +?> +--EXPECT-- +bool(true) diff --git a/main/streams/filter.c b/main/streams/filter.c index 35dbef455a30..e53c4fa14ba0 100644 --- a/main/streams/filter.c +++ b/main/streams/filter.c @@ -277,7 +277,7 @@ PHPAPI php_stream_filter *_php_stream_filter_alloc(const php_stream_filter_ops * } PHPAPI zend_result php_stream_filter_parse_write_seek_mode( - zval *filterparams, + const zval *filterparams, php_stream_filter_seekable_t *write_seekable) { *write_seekable = PSFS_SEEKABLE_ALWAYS; @@ -285,18 +285,17 @@ PHPAPI zend_result php_stream_filter_parse_write_seek_mode( if (filterparams == NULL) { return SUCCESS; } - if (Z_TYPE_P(filterparams) != IS_ARRAY && Z_TYPE_P(filterparams) != IS_OBJECT) { + if (Z_TYPE_P(filterparams) != IS_ARRAY) { return SUCCESS; } - zval *tmp = zend_hash_str_find_ind(HASH_OF(filterparams), - "write_seek_mode", sizeof("write_seek_mode") - 1); - if (tmp == NULL) { + const zval *write_seek_mode = zend_hash_str_find(Z_ARR_P(filterparams), ZEND_STRL("write_seek_mode")); + if (write_seek_mode == NULL) { return SUCCESS; } zend_string *tmp_str; - zend_string *str = zval_get_tmp_string(tmp, &tmp_str); + const zend_string *str = zval_get_tmp_string(write_seek_mode, &tmp_str); zend_result result = SUCCESS; if (zend_string_equals_literal(str, "preserve")) { diff --git a/main/streams/php_stream_filter_api.h b/main/streams/php_stream_filter_api.h index 111127a8ad1a..20df33897799 100644 --- a/main/streams/php_stream_filter_api.h +++ b/main/streams/php_stream_filter_api.h @@ -144,7 +144,7 @@ PHPAPI void php_stream_filter_free(php_stream_filter *filter); PHPAPI php_stream_filter *_php_stream_filter_alloc(const php_stream_filter_ops *fops, void *abstract, bool persistent, php_stream_filter_seekable_t read_seekable, php_stream_filter_seekable_t write_seekable STREAMS_DC); -PHPAPI zend_result php_stream_filter_parse_write_seek_mode(zval *filterparams, +PHPAPI zend_result php_stream_filter_parse_write_seek_mode(const zval *filterparams, php_stream_filter_seekable_t *write_seekable); PHPAPI int php_stream_filter_get_chain_type(php_stream *stream, php_stream_filter *filter);