From f6878b6ccfe9d7fb2d581d36232452d7de304be1 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 8 Sep 2025 18:48:43 +0200 Subject: [PATCH 1/2] Fix GH-19752: Phar decompression with invalid extension can cause UAF The rename code can error out prior to the reassignment of the filename, which is why the test causes a crash. The rename code can also error out at a later point, which means it will have already assigned the new filename. We detect in which case we are in and act accordingly. Closes GH-19761. --- NEWS | 2 ++ ext/phar/phar_object.c | 6 +++++- ext/phar/tests/gh19752.phpt | 13 +++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 ext/phar/tests/gh19752.phpt diff --git a/NEWS b/NEWS index 2ded7b11adc17..b006f93e0c32c 100644 --- a/NEWS +++ b/NEWS @@ -50,6 +50,8 @@ PHP NEWS . Fix memory leak in phar tar temporary file error handling code. (nielsdos) . Fix metadata leak when phar convert logic fails. (nielsdos) . Fix memory leak on failure in phar_convert_to_other(). (nielsdos) + . Fixed bug GH-19752 (Phar decompression with invalid extension + can cause UAF). (nielsdos) - Standard: . Fixed bug GH-16649 (UAF during array_splice). (alexandre-daubois) diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c index e91ebd0735e83..9f83fa991d4df 100644 --- a/ext/phar/phar_object.c +++ b/ext/phar/phar_object.c @@ -2328,7 +2328,11 @@ static zend_object *phar_convert_to_other(phar_archive_data *source, int convert if (phar->fp) { php_stream_close(phar->fp); } - efree(phar->fname); + if (phar->fname != source->fname) { + /* Depending on when phar_rename_archive() errors, the new filename + * may have already been assigned or it may still be the old one. */ + efree(phar->fname); + } efree(phar); } return NULL; diff --git a/ext/phar/tests/gh19752.phpt b/ext/phar/tests/gh19752.phpt new file mode 100644 index 0000000000000..0b236fff1092c --- /dev/null +++ b/ext/phar/tests/gh19752.phpt @@ -0,0 +1,13 @@ +--TEST-- +GH-19752 (Phar decompression with invalid extension can cause UAF) +--FILE-- +decompress("*"); +} catch (BadMethodCallException $e) { + echo $e->getMessage(), "\n"; +} +?> +--EXPECTF-- +data phar converted from "%sgh19752.1" has invalid extension * From 156c84746778b3bb41d558bfd3b16559148f2358 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Tue, 9 Sep 2025 00:10:39 +0200 Subject: [PATCH 2/2] uri: Fix handling of the `errors == NULL && !silent` for uri_parser_whatwg (#19748) * uri: Fix handling of the `errors == NULL && !silent` for uri_parser_whatwg Previously, when `errors` was `NULL`, the `errors` pointer was used to set the `$errors` property when throwing the exception, leading to a crash. Use a local zval to pass the errors to the Exception and copy it into the `errors` input when it is non-`NULL`. * uri: Only pass the `errors` zval when interested in it in `php_uri_instantiate_uri()` This is no longer necessary since the previous commit and also is a layering violation, since `php_uri_instantiate_uri()` should not care how `parse_uri()` works internally. * uri: Use `ZVAL_EMPTY_ARRAY()` when no parsing errors are available * uri: Avoid redundant refcounting in error handling of uri_parser_whatwg * NEWS --- NEWS | 2 ++ ext/uri/php_uri.c | 2 +- ext/uri/tests/101.phpt | 29 +++++++++++++++++++++++++++ ext/uri/uri_parser_whatwg.c | 39 ++++++++++++++++++++++++------------- 4 files changed, 57 insertions(+), 15 deletions(-) create mode 100644 ext/uri/tests/101.phpt diff --git a/NEWS b/NEWS index 06a6abb4d1c54..46b8c04708090 100644 --- a/NEWS +++ b/NEWS @@ -87,6 +87,8 @@ PHP NEWS (timwolla) . Return null instead of 0 for Uri\Rfc3986\Uri::getPort() when the URI contains an empty port. (timwolla) + . Fixed creation of the InvalidUrlException when not passing an + errors zval to the internal whatwg parser. (timwolla) . Clean up naming of internal API. (timwolla) 28 Aug 2025, PHP 8.5.0beta2 diff --git a/ext/uri/php_uri.c b/ext/uri/php_uri.c index 68204aa532d0b..f9b5dadcc9763 100644 --- a/ext/uri/php_uri.c +++ b/ext/uri/php_uri.c @@ -360,7 +360,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2) PHPAPI void php_uri_instantiate_uri( base_url = internal_base_url->uri; } - void *uri = uri_parser->parse_uri(ZSTR_VAL(uri_str), ZSTR_LEN(uri_str), base_url, should_throw || errors_zv != NULL ? &errors : NULL, !should_throw); + void *uri = uri_parser->parse_uri(ZSTR_VAL(uri_str), ZSTR_LEN(uri_str), base_url, errors_zv != NULL ? &errors : NULL, !should_throw); if (UNEXPECTED(uri == NULL)) { if (should_throw) { zval_ptr_dtor(&errors); diff --git a/ext/uri/tests/101.phpt b/ext/uri/tests/101.phpt new file mode 100644 index 0000000000000..8a44b376a6127 --- /dev/null +++ b/ext/uri/tests/101.phpt @@ -0,0 +1,29 @@ +--TEST-- +Test the Lexbor-based URI parser +--EXTENSIONS-- +uri +zend_test +--FILE-- +getMessage(), PHP_EOL; + var_dump($e->errors); +} + +?> +--EXPECTF-- +The specified URI is malformed (MissingSchemeNonRelativeUrl) +array(1) { + [0]=> + object(Uri\WhatWg\UrlValidationError)#%d (3) { + ["context"]=> + string(11) "invalid uri" + ["type"]=> + enum(Uri\WhatWg\UrlValidationErrorType::MissingSchemeNonRelativeUrl) + ["failure"]=> + bool(true) + } +} diff --git a/ext/uri/uri_parser_whatwg.c b/ext/uri/uri_parser_whatwg.c index 1403fe262b13c..fb5209328bade 100644 --- a/ext/uri/uri_parser_whatwg.c +++ b/ext/uri/uri_parser_whatwg.c @@ -65,19 +65,14 @@ static zend_always_inline void zval_long_or_null_to_lexbor_str(zval *value, lexb */ static const char *fill_errors(zval *errors) { - if (errors == NULL) { + if (lexbor_parser.log == NULL || lexbor_plog_length(lexbor_parser.log) == 0) { + ZVAL_EMPTY_ARRAY(errors); return NULL; } - ZEND_ASSERT(Z_ISUNDEF_P(errors)); - array_init(errors); - - if (lexbor_parser.log == NULL) { - return NULL; - } - const char *result = NULL; + lexbor_plog_entry_t *lxb_error; while ((lxb_error = lexbor_array_obj_pop(&lexbor_parser.log->list)) != NULL) { zval error; @@ -224,7 +219,8 @@ static const char *fill_errors(zval *errors) static void throw_invalid_url_exception_during_write(zval *errors, const char *component) { - const char *reason = fill_errors(errors); + zval err; + const char *reason = fill_errors(&err); zend_object *exception = zend_throw_exception_ex( uri_whatwg_invalid_url_exception_ce, 0, @@ -234,7 +230,13 @@ static void throw_invalid_url_exception_during_write(zval *errors, const char *c reason ? reason : "", reason ? ")" : "" ); - zend_update_property(exception->ce, exception, ZEND_STRL("errors"), errors); + zend_update_property(exception->ce, exception, ZEND_STRL("errors"), &err); + if (errors) { + zval_ptr_dtor(errors); + ZVAL_COPY_VALUE(errors, &err); + } else { + zval_ptr_dtor(&err); + } } static lxb_status_t serialize_to_smart_str_callback(const lxb_char_t *data, size_t length, void *ctx) @@ -561,11 +563,20 @@ lxb_url_t *php_uri_parser_whatwg_parse_ex(const char *uri_str, size_t uri_str_le lxb_url_parser_clean(&lexbor_parser); lxb_url_t *url = lxb_url_parse(&lexbor_parser, lexbor_base_url, (unsigned char *) uri_str, uri_str_len); - const char *reason = fill_errors(errors); - if (url == NULL && !silent) { - zend_object *exception = zend_throw_exception_ex(uri_whatwg_invalid_url_exception_ce, 0, "The specified URI is malformed%s%s%s", reason ? " (" : "", reason ? reason : "", reason ? ")" : ""); - zend_update_property(exception->ce, exception, ZEND_STRL("errors"), errors); + if ((url == NULL && !silent) || errors != NULL) { + zval err; + const char *reason = fill_errors(&err); + if (url == NULL && !silent) { + zend_object *exception = zend_throw_exception_ex(uri_whatwg_invalid_url_exception_ce, 0, "The specified URI is malformed%s%s%s", reason ? " (" : "", reason ? reason : "", reason ? ")" : ""); + zend_update_property(exception->ce, exception, ZEND_STRL("errors"), &err); + } + if (errors != NULL) { + zval_ptr_dtor(errors); + ZVAL_COPY_VALUE(errors, &err); + } else { + zval_ptr_dtor(&err); + } } return url;