Skip to content

Commit 76e5410

Browse files
committed
Fix GH-17976: CRLF injection via from and user_agent in HTTP wrapper
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. Closes GH-17976
1 parent 22aeec7 commit 76e5410

File tree

2 files changed

+72
-19
lines changed

2 files changed

+72
-19
lines changed

ext/standard/http_fopen_wrapper.c

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,16 @@ static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *w
353353
return NULL;
354354
}
355355

356+
static inline void smart_str_append_header_value(smart_str *dest, const char *src, const char *header_name)
357+
{
358+
size_t len = strcspn(src, "\r\n");
359+
smart_str_appendl(dest, src, len);
360+
if (src[len] != '\0') {
361+
php_error_docref(NULL, E_WARNING,
362+
"Header %s value contains newline characters and has been truncated", header_name);
363+
}
364+
}
365+
356366
static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
357367
const char *path, const char *mode, int options, zend_string **opened_path,
358368
php_stream_context *context, int redirect_max, int flags,
@@ -784,7 +794,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
784794
/* if the user has configured who they are, send a From: line */
785795
if (!(have_header & HTTP_HEADER_FROM) && FG(from_address)) {
786796
smart_str_appends(&req_buf, "From: ");
787-
smart_str_appends(&req_buf, FG(from_address));
797+
smart_str_append_header_value(&req_buf, FG(from_address), "From");
788798
smart_str_appends(&req_buf, "\r\n");
789799
}
790800

@@ -818,24 +828,10 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
818828
ua_str = FG(user_agent);
819829
}
820830

821-
if (((have_header & HTTP_HEADER_USER_AGENT) == 0) && ua_str) {
822-
#define _UA_HEADER "User-Agent: %s\r\n"
823-
char *ua;
824-
size_t ua_len;
825-
826-
ua_len = sizeof(_UA_HEADER) + strlen(ua_str);
827-
828-
/* ensure the header is only sent if user_agent is not blank */
829-
if (ua_len > sizeof(_UA_HEADER)) {
830-
ua = emalloc(ua_len + 1);
831-
if ((ua_len = slprintf(ua, ua_len, _UA_HEADER, ua_str)) > 0) {
832-
ua[ua_len] = 0;
833-
smart_str_appendl(&req_buf, ua, ua_len);
834-
} else {
835-
php_error_docref(NULL, E_WARNING, "Cannot construct User-agent header");
836-
}
837-
efree(ua);
838-
}
831+
if (((have_header & HTTP_HEADER_USER_AGENT) == 0) && ua_str && *ua_str) {
832+
smart_str_appends(&req_buf, "User-Agent: ");
833+
smart_str_append_header_value(&req_buf, ua_str, "User-Agent");
834+
smart_str_appends(&req_buf, "\r\n");
839835
}
840836

841837
if (user_headers) {
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
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 < 3; $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+
CODE;
45+
46+
include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__);
47+
ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
48+
?>
49+
--EXPECTF--
50+
Warning: file_get_contents(): Header From value contains newline characters and has been truncated in %s on line %d
51+
OK: from INI
52+
53+
Warning: file_get_contents(): Header User-Agent value contains newline characters and has been truncated in %s on line %d
54+
OK: user_agent INI
55+
56+
Warning: file_get_contents(): Header User-Agent value contains newline characters and has been truncated in %s on line %d
57+
OK: user_agent context

0 commit comments

Comments
 (0)