diff --git a/ext/soap/php_http.c b/ext/soap/php_http.c index 1d030caf9d45..92d95874c791 100644 --- a/ext/soap/php_http.c +++ b/ext/soap/php_http.c @@ -626,7 +626,7 @@ int make_http_soap_request( } } else if (FG(user_agent)) { smart_str_append_const(&soap_headers, "User-Agent: "); - smart_str_appends(&soap_headers, FG(user_agent)); + smart_str_appends(&soap_headers, ZSTR_VAL(FG(user_agent))); smart_str_append_const(&soap_headers, "\r\n"); } else { smart_str_append_const(&soap_headers, "User-Agent: PHP-SOAP/"PHP_VERSION"\r\n"); diff --git a/ext/standard/file.c b/ext/standard/file.c index db0fc45385dd..f9474af68735 100644 --- a/ext/standard/file.c +++ b/ext/standard/file.c @@ -147,8 +147,8 @@ static PHP_INI_MH(OnUpdateAutoDetectLineEndings) } PHP_INI_BEGIN() - STD_PHP_INI_ENTRY("user_agent", NULL, PHP_INI_ALL, OnUpdateString, user_agent, php_file_globals, file_globals) - STD_PHP_INI_ENTRY("from", NULL, PHP_INI_ALL, OnUpdateString, from_address, php_file_globals, file_globals) + STD_PHP_INI_ENTRY("user_agent", NULL, PHP_INI_ALL, OnUpdateStr, user_agent, php_file_globals, file_globals) + STD_PHP_INI_ENTRY("from", NULL, PHP_INI_ALL, OnUpdateStr, from_address, php_file_globals, file_globals) STD_PHP_INI_ENTRY("default_socket_timeout", "60", PHP_INI_ALL, OnUpdateLong, default_socket_timeout, php_file_globals, file_globals) STD_PHP_INI_BOOLEAN("auto_detect_line_endings", "0", PHP_INI_ALL, OnUpdateAutoDetectLineEndings, auto_detect_line_endings, php_file_globals, file_globals) PHP_INI_END() diff --git a/ext/standard/file.h b/ext/standard/file.h index ac218169599c..9ba5f5b8b93d 100644 --- a/ext/standard/file.h +++ b/ext/standard/file.h @@ -93,8 +93,8 @@ typedef struct { size_t def_chunk_size; bool auto_detect_line_endings; zend_long default_socket_timeout; - char *user_agent; /* for the http wrapper */ - char *from_address; /* for the ftp and http wrappers */ + zend_string *user_agent; /* for the http wrapper */ + zend_string *from_address; /* for the ftp and http wrappers */ const char *user_stream_current_filename; /* for simple recursion protection */ php_stream_context *default_context; HashTable *stream_wrappers; /* per-request copy of url_stream_wrappers_hash */ diff --git a/ext/standard/ftp_fopen_wrapper.c b/ext/standard/ftp_fopen_wrapper.c index f99ae5e4b4e1..457d1410aabe 100644 --- a/ext/standard/ftp_fopen_wrapper.c +++ b/ext/standard/ftp_fopen_wrapper.c @@ -273,7 +273,7 @@ static php_stream *php_ftp_fopen_connect(php_stream_wrapper *wrapper, const char /* if the user has configured who they are, send that as the password */ if (FG(from_address)) { - php_stream_printf(stream, "PASS %s\r\n", FG(from_address)); + php_stream_printf(stream, "PASS %s\r\n", ZSTR_VAL(FG(from_address))); } else { php_stream_write_string(stream, "PASS anonymous\r\n"); } diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index 9e3db6716048..d0c7faf2aec4 100644 --- a/ext/standard/http_fopen_wrapper.c +++ b/ext/standard/http_fopen_wrapper.c @@ -351,6 +351,24 @@ static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *w return NULL; } +static inline void smart_str_append_header_value(smart_str *dest, const zend_string *value, const char *header_name) +{ + const char *src = ZSTR_VAL(value); + const char *cr = memchr(src, '\r', ZSTR_LEN(value)); + const char *lf = memchr(src, '\n', ZSTR_LEN(value)); + const char *nl = cr; + if (lf != NULL && (nl == NULL || lf < nl)) { + nl = lf; + } + if (nl != NULL) { + smart_str_appendl(dest, src, nl - src); + php_error_docref(NULL, E_WARNING, + "Header %s value contains newline characters and has been truncated", header_name); + } else { + smart_str_append(dest, value); + } +} + static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, const char *path, const char *mode, int options, zend_string **opened_path, php_stream_context *context, int redirect_max, int flags, @@ -361,7 +379,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, int use_ssl; int use_proxy = 0; zend_string *tmp = NULL; - char *ua_str = NULL; + zend_string *ua_str = NULL; zval *ua_zval = NULL, *tmpzval = NULL, ssl_proxy_peer_name; int reqok = 0; char *http_header_line = NULL; @@ -788,7 +806,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, /* if the user has configured who they are, send a From: line */ if (!(have_header & HTTP_HEADER_FROM) && FG(from_address)) { smart_str_appends(&req_buf, "From: "); - smart_str_appends(&req_buf, FG(from_address)); + smart_str_append_header_value(&req_buf, FG(from_address), "From"); smart_str_appends(&req_buf, "\r\n"); } @@ -817,30 +835,15 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, if (context && (ua_zval = php_stream_context_get_option(context, "http", "user_agent")) != NULL && Z_TYPE_P(ua_zval) == IS_STRING) { - ua_str = Z_STRVAL_P(ua_zval); + ua_str = Z_STR_P(ua_zval); } else if (FG(user_agent)) { ua_str = FG(user_agent); } - if (((have_header & HTTP_HEADER_USER_AGENT) == 0) && ua_str) { -#define _UA_HEADER "User-Agent: %s\r\n" - char *ua; - size_t ua_len; - - ua_len = sizeof(_UA_HEADER) + strlen(ua_str); - - /* ensure the header is only sent if user_agent is not blank */ - if (ua_len > sizeof(_UA_HEADER)) { - ua = emalloc(ua_len + 1); - if ((ua_len = slprintf(ua, ua_len, _UA_HEADER, ua_str)) > 0) { - ua[ua_len] = 0; - smart_str_appendl(&req_buf, ua, ua_len); - } else { - php_stream_wrapper_warn_nt(wrapper, context, options, InvalidHeader, - "Cannot construct User-agent header"); - } - efree(ua); - } + if (((have_header & HTTP_HEADER_USER_AGENT) == 0) && ua_str && ZSTR_LEN(ua_str)) { + smart_str_appends(&req_buf, "User-Agent: "); + smart_str_append_header_value(&req_buf, ua_str, "User-Agent"); + smart_str_appends(&req_buf, "\r\n"); } if (user_headers) { diff --git a/ext/standard/tests/http/gh17976.phpt b/ext/standard/tests/http/gh17976.phpt new file mode 100644 index 000000000000..bba9a7a0fcf3 --- /dev/null +++ b/ext/standard/tests/http/gh17976.phpt @@ -0,0 +1,65 @@ +--TEST-- +GH-17976 (CRLF injection via from and user_agent INI settings in HTTP wrapper) +--INI-- +allow_url_fopen=1 +--FILE-- + ["tcp_nodelay" => true] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + for ($i = 0; $i < 4; $i++) { + $conn = stream_socket_accept($server); + $result = fread($conn, 4096); + fwrite($conn, "HTTP/1.0 200 OK\r\nContent-Type: text/plain\r\n\r\n" . base64_encode($result)); + fclose($conn); + } +CODE; + +$clientCode = <<<'CODE' + // Test 1: from INI with CRLF + ini_set("from", "test\r\nInjected-From: evil"); + ini_set("user_agent", "clean_ua"); + $raw = base64_decode(file_get_contents("http://{{ ADDR }}/")); + echo (str_contains($raw, "Injected-From:") ? "FAIL" : "OK") . ": from INI\n"; + + // Test 2: user_agent INI with CRLF + ini_restore("from"); + ini_set("user_agent", "test\r\nInjected-UA: evil"); + $raw = base64_decode(file_get_contents("http://{{ ADDR }}/")); + echo (str_contains($raw, "Injected-UA:") ? "FAIL" : "OK") . ": user_agent INI\n"; + + // Test 3: user_agent context option with CRLF + ini_restore("user_agent"); + $ctx = stream_context_create(["http" => [ + "user_agent" => "test\nInjected-Ctx: evil" + ]]); + $raw = base64_decode(file_get_contents("http://{{ ADDR }}/", false, $ctx)); + echo (str_contains($raw, "Injected-Ctx:") ? "FAIL" : "OK") . ": user_agent context\n"; + + // Test 4: user_agent INI with a NUL byte before the CRLF + ini_set("user_agent", "ua\0valid\r\nInjected-Nul: evil"); + $raw = base64_decode(file_get_contents("http://{{ ADDR }}/")); + echo (str_contains($raw, "Injected-Nul:") ? "FAIL" : "OK") . ": user_agent NUL+CRLF\n"; +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +Warning: file_get_contents(): Header From value contains newline characters and has been truncated in %s on line %d +OK: from INI + +Warning: file_get_contents(): Header User-Agent value contains newline characters and has been truncated in %s on line %d +OK: user_agent INI + +Warning: file_get_contents(): Header User-Agent value contains newline characters and has been truncated in %s on line %d +OK: user_agent context + +Warning: file_get_contents(): Header User-Agent value contains newline characters and has been truncated in %s on line %d +OK: user_agent NUL+CRLF