Skip to content

Commit 5c163ca

Browse files
iliaaliliaal
authored
Fix GH-17976: CRLF injection via from and user_agent in HTTP wrapper (#21658)
The from and user_agent INI settings and the user_agent stream context option were written into HTTP request headers without stripping CR/LF characters, allowing header injection. Truncate at the first \r or \n and emit E_WARNING. The from and user_agent globals are stored as zend_string so the check scans the full length and a NUL byte no longer pre-truncates the value before the CR/LF scan. Fixes GH-17976
1 parent 65c9b6d commit 5c163ca

6 files changed

Lines changed: 95 additions & 28 deletions

File tree

‎ext/soap/php_http.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -626,7 +626,7 @@ int make_http_soap_request(
626626
}
627627
} else if (FG(user_agent)) {
628628
smart_str_append_const(&soap_headers, "User-Agent: ");
629-
smart_str_appends(&soap_headers, FG(user_agent));
629+
smart_str_append(&soap_headers, FG(user_agent));
630630
smart_str_append_const(&soap_headers, "\r\n");
631631
} else {
632632
smart_str_append_const(&soap_headers, "User-Agent: PHP-SOAP/"PHP_VERSION"\r\n");

‎ext/standard/file.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ static PHP_INI_MH(OnUpdateAutoDetectLineEndings)
147147
}
148148

149149
PHP_INI_BEGIN()
150-
STD_PHP_INI_ENTRY("user_agent", NULL, PHP_INI_ALL, OnUpdateString, user_agent, php_file_globals, file_globals)
151-
STD_PHP_INI_ENTRY("from", NULL, PHP_INI_ALL, OnUpdateString, from_address, php_file_globals, file_globals)
150+
STD_PHP_INI_ENTRY("user_agent", NULL, PHP_INI_ALL, OnUpdateStr, user_agent, php_file_globals, file_globals)
151+
STD_PHP_INI_ENTRY("from", NULL, PHP_INI_ALL, OnUpdateStr, from_address, php_file_globals, file_globals)
152152
STD_PHP_INI_ENTRY("default_socket_timeout", "60", PHP_INI_ALL, OnUpdateLong, default_socket_timeout, php_file_globals, file_globals)
153153
STD_PHP_INI_BOOLEAN("auto_detect_line_endings", "0", PHP_INI_ALL, OnUpdateAutoDetectLineEndings, auto_detect_line_endings, php_file_globals, file_globals)
154154
PHP_INI_END()

‎ext/standard/file.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ typedef struct {
9393
size_t def_chunk_size;
9494
bool auto_detect_line_endings;
9595
zend_long default_socket_timeout;
96-
char *user_agent; /* for the http wrapper */
97-
char *from_address; /* for the ftp and http wrappers */
96+
zend_string *user_agent; /* for the http wrapper */
97+
zend_string *from_address; /* for the ftp and http wrappers */
9898
const char *user_stream_current_filename; /* for simple recursion protection */
9999
php_stream_context *default_context;
100100
HashTable *stream_wrappers; /* per-request copy of url_stream_wrappers_hash */

‎ext/standard/ftp_fopen_wrapper.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ static php_stream *php_ftp_fopen_connect(php_stream_wrapper *wrapper, const char
273273
/* if the user has configured who they are,
274274
send that as the password */
275275
if (FG(from_address)) {
276-
php_stream_printf(stream, "PASS %s\r\n", FG(from_address));
276+
php_stream_printf(stream, "PASS %s\r\n", ZSTR_VAL(FG(from_address)));
277277
} else {
278278
php_stream_write_string(stream, "PASS anonymous\r\n");
279279
}

‎ext/standard/http_fopen_wrapper.c‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,23 @@ static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *w
351351
return NULL;
352352
}
353353

354+
static inline void smart_str_append_header_value(smart_str *dest, const zend_string *value, const char *header_name)
355+
{
356+
const char *src = ZSTR_VAL(value);
357+
size_t len = ZSTR_LEN(value);
358+
size_t i = 0;
359+
while (i < len && src[i] != '\r' && src[i] != '\n') {
360+
i++;
361+
}
362+
if (i < len) {
363+
smart_str_appendl(dest, src, i);
364+
php_error_docref(NULL, E_WARNING,
365+
"Header %s value contains newline characters and has been truncated", header_name);
366+
} else {
367+
smart_str_append(dest, value);
368+
}
369+
}
370+
354371
static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
355372
const char *path, const char *mode, int options, zend_string **opened_path,
356373
php_stream_context *context, int redirect_max, int flags,
@@ -361,7 +378,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
361378
int use_ssl;
362379
int use_proxy = 0;
363380
zend_string *tmp = NULL;
364-
char *ua_str = NULL;
381+
zend_string *ua_str = NULL;
365382
zval *ua_zval = NULL, *tmpzval = NULL, ssl_proxy_peer_name;
366383
int reqok = 0;
367384
char *http_header_line = NULL;
@@ -788,7 +805,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
788805
/* if the user has configured who they are, send a From: line */
789806
if (!(have_header & HTTP_HEADER_FROM) && FG(from_address)) {
790807
smart_str_appends(&req_buf, "From: ");
791-
smart_str_appends(&req_buf, FG(from_address));
808+
smart_str_append_header_value(&req_buf, FG(from_address), "From");
792809
smart_str_appends(&req_buf, "\r\n");
793810
}
794811

@@ -817,30 +834,15 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
817834
if (context &&
818835
(ua_zval = php_stream_context_get_option(context, "http", "user_agent")) != NULL &&
819836
Z_TYPE_P(ua_zval) == IS_STRING) {
820-
ua_str = Z_STRVAL_P(ua_zval);
837+
ua_str = Z_STR_P(ua_zval);
821838
} else if (FG(user_agent)) {
822839
ua_str = FG(user_agent);
823840
}
824841

825-
if (((have_header & HTTP_HEADER_USER_AGENT) == 0) && ua_str) {
826-
#define _UA_HEADER "User-Agent: %s\r\n"
827-
char *ua;
828-
size_t ua_len;
829-
830-
ua_len = sizeof(_UA_HEADER) + strlen(ua_str);
831-
832-
/* ensure the header is only sent if user_agent is not blank */
833-
if (ua_len > sizeof(_UA_HEADER)) {
834-
ua = emalloc(ua_len + 1);
835-
if ((ua_len = slprintf(ua, ua_len, _UA_HEADER, ua_str)) > 0) {
836-
ua[ua_len] = 0;
837-
smart_str_appendl(&req_buf, ua, ua_len);
838-
} else {
839-
php_stream_wrapper_warn_nt(wrapper, context, options, InvalidHeader,
840-
"Cannot construct User-agent header");
841-
}
842-
efree(ua);
843-
}
842+
if (((have_header & HTTP_HEADER_USER_AGENT) == 0) && ua_str && ZSTR_LEN(ua_str)) {
843+
smart_str_appends(&req_buf, "User-Agent: ");
844+
smart_str_append_header_value(&req_buf, ua_str, "User-Agent");
845+
smart_str_appends(&req_buf, "\r\n");
844846
}
845847

846848
if (user_headers) {

‎ext/standard/tests/http/gh17976.phpt‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
--TEST--
2+
GH-17976 (CRLF injection via from and user_agent INI settings in HTTP wrapper)
3+
--INI--
4+
allow_url_fopen=1
5+
--FILE--
6+
<?php
7+
$serverCode = <<<'CODE'
8+
$ctxt = stream_context_create([
9+
"socket" => ["tcp_nodelay" => true]
10+
]);
11+
12+
$server = stream_socket_server(
13+
"tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt);
14+
phpt_notify_server_start($server);
15+
16+
for ($i = 0; $i < 4; $i++) {
17+
$conn = stream_socket_accept($server);
18+
$result = fread($conn, 4096);
19+
fwrite($conn, "HTTP/1.0 200 OK\r\nContent-Type: text/plain\r\n\r\n" . base64_encode($result));
20+
fclose($conn);
21+
}
22+
CODE;
23+
24+
$clientCode = <<<'CODE'
25+
// Test 1: from INI with CRLF
26+
ini_set("from", "test\r\nInjected-From: evil");
27+
ini_set("user_agent", "clean_ua");
28+
$raw = base64_decode(file_get_contents("http://{{ ADDR }}/"));
29+
echo (str_contains($raw, "Injected-From:") ? "FAIL" : "OK") . ": from INI\n";
30+
31+
// Test 2: user_agent INI with CRLF
32+
ini_restore("from");
33+
ini_set("user_agent", "test\r\nInjected-UA: evil");
34+
$raw = base64_decode(file_get_contents("http://{{ ADDR }}/"));
35+
echo (str_contains($raw, "Injected-UA:") ? "FAIL" : "OK") . ": user_agent INI\n";
36+
37+
// Test 3: user_agent context option with CRLF
38+
ini_restore("user_agent");
39+
$ctx = stream_context_create(["http" => [
40+
"user_agent" => "test\nInjected-Ctx: evil"
41+
]]);
42+
$raw = base64_decode(file_get_contents("http://{{ ADDR }}/", false, $ctx));
43+
echo (str_contains($raw, "Injected-Ctx:") ? "FAIL" : "OK") . ": user_agent context\n";
44+
45+
// Test 4: user_agent INI with a NUL byte before the CRLF
46+
ini_set("user_agent", "ua\0valid\r\nInjected-Nul: evil");
47+
$raw = base64_decode(file_get_contents("http://{{ ADDR }}/"));
48+
echo (str_contains($raw, "Injected-Nul:") ? "FAIL" : "OK") . ": user_agent NUL+CRLF\n";
49+
CODE;
50+
51+
include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__);
52+
ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
53+
?>
54+
--EXPECTF--
55+
Warning: file_get_contents(): Header From value contains newline characters and has been truncated in %s on line %d
56+
OK: from INI
57+
58+
Warning: file_get_contents(): Header User-Agent value contains newline characters and has been truncated in %s on line %d
59+
OK: user_agent INI
60+
61+
Warning: file_get_contents(): Header User-Agent value contains newline characters and has been truncated in %s on line %d
62+
OK: user_agent context
63+
64+
Warning: file_get_contents(): Header User-Agent value contains newline characters and has been truncated in %s on line %d
65+
OK: user_agent NUL+CRLF

0 commit comments

Comments
 (0)