Commit 047c4501f28 for php
commit 047c4501f28b85e351c99fa11d31d62065223100
Author: Alejandro Pacheco <alepacheco99@gmail.com>
Date: Thu Oct 15 04:57:11 2026 +0200
Fix in-place URL decoding of interned userinfo in the http and ftp wrappers
The http and ftp stream wrappers decoded the user and password (and,
when checking a redirect, the path) of the parsed URL in place. The
components of a php_uri may be interned strings (the WHATWG parser
returns one-character usernames and passwords as interned single-char
strings, the RFC 3986 parser returns the path "/" as one), so decoding a
redirect target such as http://+@host/ rewrote the shared "+" string for
the whole process.
Decode the components into new strings instead, and build the relative
redirect path without writing into the current path.
Closes GH-24341
diff --git a/NEWS b/NEWS
index 6bcd38f2c56..4ed7877b679 100644
--- a/NEWS
+++ b/NEWS
@@ -52,6 +52,8 @@ PHP NEWS
which could cause readline() to return false prematurely. (Weilin Du)
- Streams:
+ . Fixed in-place URL decoding of interned strings in the HTTP and FTP
+ wrappers. (Alejandro Pacheco)
. Fixed bug GH-22063 (Stream filter chain UAF on self-removal during
callback). (Ilia Alshanetsky)
. Fixed leaks and data loss when user filter leaves buckets on input
diff --git a/ext/standard/ftp_fopen_wrapper.c b/ext/standard/ftp_fopen_wrapper.c
index 72726f5a37d..cacfa9e1013 100644
--- a/ext/standard/ftp_fopen_wrapper.c
+++ b/ext/standard/ftp_fopen_wrapper.c
@@ -133,6 +133,24 @@ static int php_stream_ftp_stream_close(php_stream_wrapper *wrapper, php_stream *
}
/* }}} */
+/* The components of a php_uri may be interned or shared, so they are decoded into a new string. */
+static zend_string *php_ftp_url_decode(const zend_string *component)
+{
+ zend_string *decoded = zend_string_alloc(ZSTR_LEN(component), false);
+ ZSTR_LEN(decoded) = php_raw_url_decode_ex(ZSTR_VAL(decoded), ZSTR_VAL(component), ZSTR_LEN(component));
+ return decoded;
+}
+
+static bool php_ftp_has_cntrl(const zend_string *str)
+{
+ for (size_t i = 0; i < ZSTR_LEN(str); i++) {
+ if (iscntrl((unsigned char) ZSTR_VAL(str)[i])) {
+ return true;
+ }
+ }
+ return false;
+}
+
/* {{{ php_ftp_fopen_connect */
static php_stream *php_ftp_fopen_connect(php_stream_wrapper *wrapper, const char *path, const char *mode, int options,
zend_string **opened_path, php_stream_context *context, php_stream **preuseid,
@@ -243,24 +261,18 @@ static php_stream *php_ftp_fopen_connect(php_stream_wrapper *wrapper, const char
#endif
}
-#define PHP_FTP_CNTRL_CHK(val, val_len, err_msg) { \
- unsigned char *s = (unsigned char *) val, *e = (unsigned char *) s + val_len; \
- while (s < e) { \
- if (iscntrl((unsigned char)*s)) { \
- php_stream_wrapper_log_error(wrapper, options, err_msg, val); \
- goto connect_errexit; \
- } \
- s++; \
- } \
-}
-
/* send the user name */
if (resource->user != NULL) {
- ZSTR_LEN(resource->user) = php_raw_url_decode(ZSTR_VAL(resource->user), ZSTR_LEN(resource->user));
+ zend_string *user = php_ftp_url_decode(resource->user);
- PHP_FTP_CNTRL_CHK(ZSTR_VAL(resource->user), ZSTR_LEN(resource->user), "Invalid login %s")
+ if (php_ftp_has_cntrl(user)) {
+ php_stream_wrapper_log_error(wrapper, options, "Invalid login %s", ZSTR_VAL(user));
+ zend_string_efree(user);
+ goto connect_errexit;
+ }
- php_stream_printf(stream, "USER %s\r\n", ZSTR_VAL(resource->user));
+ php_stream_printf(stream, "USER %s\r\n", ZSTR_VAL(user));
+ zend_string_efree(user);
} else {
php_stream_write_string(stream, "USER anonymous\r\n");
}
@@ -273,11 +285,16 @@ static php_stream *php_ftp_fopen_connect(php_stream_wrapper *wrapper, const char
php_stream_notify_info(context, PHP_STREAM_NOTIFY_AUTH_REQUIRED, tmp_line, 0);
if (resource->password != NULL) {
- ZSTR_LEN(resource->password) = php_raw_url_decode(ZSTR_VAL(resource->password), ZSTR_LEN(resource->password));
+ zend_string *password = php_ftp_url_decode(resource->password);
- PHP_FTP_CNTRL_CHK(ZSTR_VAL(resource->password), ZSTR_LEN(resource->password), "Invalid password %s")
+ if (php_ftp_has_cntrl(password)) {
+ php_stream_wrapper_log_error(wrapper, options, "Invalid password %s", ZSTR_VAL(password));
+ zend_string_efree(password);
+ goto connect_errexit;
+ }
- php_stream_printf(stream, "PASS %s\r\n", ZSTR_VAL(resource->password));
+ php_stream_printf(stream, "PASS %s\r\n", ZSTR_VAL(password));
+ zend_string_efree(password);
} else {
/* if the user has configured who they are,
send that as the password */
diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c
index 302c012bd3e..ac91e4210eb 100644
--- a/ext/standard/http_fopen_wrapper.c
+++ b/ext/standard/http_fopen_wrapper.c
@@ -161,6 +161,32 @@ static bool check_has_header(const char *headers, const char *header) {
return 0;
}
+/* The components of a php_uri may be interned or shared, so they are decoded into a new string. */
+static zend_string *php_stream_http_url_decode(const zend_string *component)
+{
+ zend_string *decoded = zend_string_alloc(ZSTR_LEN(component), false);
+ ZSTR_LEN(decoded) = php_url_decode_ex(ZSTR_VAL(decoded), ZSTR_VAL(component), ZSTR_LEN(component));
+ return decoded;
+}
+
+static bool php_stream_http_url_component_has_cntrl(const zend_string *component)
+{
+ if (!component) {
+ return false;
+ }
+
+ zend_string *decoded = php_stream_http_url_decode(component);
+ bool has_cntrl = false;
+ for (size_t i = 0; i < ZSTR_LEN(decoded); i++) {
+ if (iscntrl((unsigned char) ZSTR_VAL(decoded)[i])) {
+ has_cntrl = true;
+ break;
+ }
+ }
+ zend_string_efree(decoded);
+ return has_cntrl;
+}
+
static zend_result php_stream_handle_proxy_authorization_header(const char *s, smart_str *header)
{
const char *p;
@@ -816,15 +842,16 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
smart_str scratch = {0};
/* decode the strings first */
- ZSTR_LEN(resource->user) = php_url_decode(ZSTR_VAL(resource->user), ZSTR_LEN(resource->user));
-
- smart_str_append(&scratch, resource->user);
+ zend_string *decoded = php_stream_http_url_decode(resource->user);
+ smart_str_append(&scratch, decoded);
+ zend_string_efree(decoded);
smart_str_appendc(&scratch, ':');
/* Note: password is optional! */
if (resource->password) {
- ZSTR_LEN(resource->password) = php_url_decode(ZSTR_VAL(resource->password), ZSTR_LEN(resource->password));
- smart_str_append(&scratch, resource->password);
+ decoded = php_stream_http_url_decode(resource->password);
+ smart_str_append(&scratch, decoded);
+ zend_string_efree(decoded);
}
zend_string *scratch_str = smart_str_extract(&scratch);
@@ -1120,24 +1147,13 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
char *loc_path = NULL;
if (*header_info.location != '/') {
if (header_info.location_len > 1 && resource->path) {
- char *s = strrchr(ZSTR_VAL(resource->path), '/');
- if (!s) {
- s = ZSTR_VAL(resource->path);
- if (!ZSTR_LEN(resource->path)) {
- zend_string_release_ex(resource->path, 0);
- resource->path = ZSTR_INIT_LITERAL("/", 0);
- s = ZSTR_VAL(resource->path);
- } else {
- *s = '/';
- }
- }
- s[1] = '\0';
- if (resource->path &&
- ZSTR_VAL(resource->path)[0] == '/' &&
- ZSTR_VAL(resource->path)[1] == '\0') {
- spprintf(&loc_path, 0, "%s%s", ZSTR_VAL(resource->path), header_info.location);
+ /* directory of the current path, up to and including its last '/' */
+ const char *dir = ZSTR_VAL(resource->path);
+ const char *s = strrchr(dir, '/');
+ if (!s || s == dir) {
+ spprintf(&loc_path, 0, "/%s", header_info.location);
} else {
- spprintf(&loc_path, 0, "%s/%s", ZSTR_VAL(resource->path), header_info.location);
+ spprintf(&loc_path, 0, "%.*s/%s", (int) (s - dir + 1), dir, header_info.location);
}
} else {
spprintf(&loc_path, 0, "/%s", header_info.location);
@@ -1177,26 +1193,15 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
php_uri_struct_free(resource);
resource = new_resource;
-#define CHECK_FOR_CNTRL_CHARS(val) { \
- if (val) { \
- unsigned char *s, *e; \
- ZSTR_LEN(val) = php_url_decode(ZSTR_VAL(val), ZSTR_LEN(val)); \
- s = (unsigned char*)ZSTR_VAL(val); e = s + ZSTR_LEN(val); \
- while (s < e) { \
- if (iscntrl(*s)) { \
- php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path); \
- efree(new_path); \
- goto out; \
- } \
- s++; \
- } \
- } \
-}
/* check for control characters in login, password & path */
if (strncasecmp(new_path, "http://", sizeof("http://") - 1) || strncasecmp(new_path, "https://", sizeof("https://") - 1)) {
- CHECK_FOR_CNTRL_CHARS(resource->user);
- CHECK_FOR_CNTRL_CHARS(resource->password);
- CHECK_FOR_CNTRL_CHARS(resource->path);
+ if (php_stream_http_url_component_has_cntrl(resource->user)
+ || php_stream_http_url_component_has_cntrl(resource->password)
+ || php_stream_http_url_component_has_cntrl(resource->path)) {
+ php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path);
+ efree(new_path);
+ goto out;
+ }
}
int new_flags = HTTP_WRAPPER_REDIRECTED | (flags & HTTP_WRAPPER_STRIP_AUTH);
if (!same_origin) {
diff --git a/ext/standard/tests/http/http_redirect_userinfo_whatwg_decode.phpt b/ext/standard/tests/http/http_redirect_userinfo_whatwg_decode.phpt
new file mode 100644
index 00000000000..7bb11b97a19
--- /dev/null
+++ b/ext/standard/tests/http/http_redirect_userinfo_whatwg_decode.phpt
@@ -0,0 +1,49 @@
+--TEST--
+Redirect userinfo is decoded into a new string with the WHATWG URL parser
+--FILE--
+<?php
+$serverCode = <<<'CODE'
+$server = stream_socket_server("tcp://127.0.0.1:0", $errno, $errstr);
+phpt_notify_server_start($server);
+$addr = stream_socket_get_name($server, false);
+
+/* a one-character user, then a one-character password */
+$locations = ["http://+@$addr/b", "http://a:+@$addr/d"];
+
+for ($n = 0; $n < 4; $n++) {
+ $conn = stream_socket_accept($server, 10);
+ if (!$conn) {
+ break;
+ }
+ $auth = '';
+ fgets($conn);
+ while (($line = trim(fgets($conn))) !== '') {
+ if (stripos($line, 'Authorization:') === 0) {
+ $auth = $line;
+ }
+ }
+ if ($n % 2 == 0) {
+ fwrite($conn, "HTTP/1.1 302 Found\r\nLocation: {$locations[$n / 2]}\r\nContent-Length: 0\r\n\r\n");
+ } else {
+ fwrite($conn, "HTTP/1.1 200 OK\r\nContent-Length: " . strlen($auth) . "\r\n\r\n$auth");
+ }
+ fclose($conn);
+}
+CODE;
+
+$clientCode = <<<'CODE'
+$ctx = stream_context_create(['http' => ['follow_location' => 1, 'uri_parser_class' => \Uri\WhatWg\Url::class]]);
+echo file_get_contents("http://{{ ADDR }}/a", false, $ctx), "\n";
+var_dump(bin2hex(chr(43)));
+echo file_get_contents("http://{{ ADDR }}/c", false, $ctx), "\n";
+var_dump(bin2hex(chr(43)));
+CODE;
+
+include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__);
+ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
+?>
+--EXPECT--
+Authorization: Basic IDo=
+string(2) "2b"
+Authorization: Basic YTog
+string(2) "2b"