Skip to content

Commit ae14a0b

Browse files
bukkabukka
committed
Fix GHSA-52jp-hrpf-2jff: http redirect location truncation
It converts the allocation of location to be on heap instead of stack and errors if the location length is greater than 8086 bytes.
1 parent 455161c commit ae14a0b

File tree

3 files changed

+168
-32
lines changed

3 files changed

+168
-32
lines changed

‎ext/standard/http_fopen_wrapper.c‎

Lines changed: 55 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,16 @@
6767

6868
#include "php_fopen_wrappers.h"
6969

70-
#define HTTP_HEADER_BLOCK_SIZE 1024
71-
#define PHP_URL_REDIRECT_MAX 20
72-
#define HTTP_HEADER_USER_AGENT 1
73-
#define HTTP_HEADER_HOST 2
74-
#define HTTP_HEADER_AUTH 4
75-
#define HTTP_HEADER_FROM 8
76-
#define HTTP_HEADER_CONTENT_LENGTH 16
77-
#define HTTP_HEADER_TYPE 32
78-
#define HTTP_HEADER_CONNECTION 64
70+
#define HTTP_HEADER_BLOCK_SIZE 1024
71+
#define HTTP_HEADER_MAX_LOCATION_SIZE 8182 /* 8192 - 10 (size of "Location: ") */
72+
#define PHP_URL_REDIRECT_MAX 20
73+
#define HTTP_HEADER_USER_AGENT 1
74+
#define HTTP_HEADER_HOST 2
75+
#define HTTP_HEADER_AUTH 4
76+
#define HTTP_HEADER_FROM 8
77+
#define HTTP_HEADER_CONTENT_LENGTH 16
78+
#define HTTP_HEADER_TYPE 32
79+
#define HTTP_HEADER_CONNECTION 64
7980

8081
#define HTTP_WRAPPER_HEADER_INIT 1
8182
#define HTTP_WRAPPER_REDIRECTED 2
@@ -120,17 +121,15 @@ typedef struct _php_stream_http_response_header_info {
120121
size_t file_size;
121122
bool error;
122123
bool follow_location;
123-
char location[HTTP_HEADER_BLOCK_SIZE];
124+
char *location;
125+
size_t location_len;
124126
} php_stream_http_response_header_info;
125127

126128
static void php_stream_http_response_header_info_init(
127129
php_stream_http_response_header_info *header_info)
128130
{
129-
header_info->transfer_encoding = NULL;
130-
header_info->file_size = 0;
131-
header_info->error = false;
131+
memset(header_info, 0, sizeof(php_stream_http_response_header_info));
132132
header_info->follow_location = 1;
133-
header_info->location[0] = '\0';
134133
}
135134

136135
/* Trim white spaces from response header line and update its length */
@@ -256,7 +255,22 @@ static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *w
256255
* RFC 7238 defines 308: http://tools.ietf.org/html/rfc7238 */
257256
header_info->follow_location = 0;
258257
}
259-
strlcpy(header_info->location, last_header_value, sizeof(header_info->location));
258+
size_t last_header_value_len = strlen(last_header_value);
259+
if (last_header_value_len > HTTP_HEADER_MAX_LOCATION_SIZE) {
260+
header_info->error = true;
261+
php_stream_wrapper_log_error(wrapper, options,
262+
"HTTP Location header size is over the limit of %d bytes",
263+
HTTP_HEADER_MAX_LOCATION_SIZE);
264+
zend_string_efree(last_header_line_str);
265+
return NULL;
266+
}
267+
if (header_info->location_len == 0) {
268+
header_info->location = emalloc(last_header_value_len + 1);
269+
} else if (header_info->location_len <= last_header_value_len) {
270+
header_info->location = erealloc(header_info->location, last_header_value_len + 1);
271+
}
272+
header_info->location_len = last_header_value_len;
273+
memcpy(header_info->location, last_header_value, last_header_value_len + 1);
260274
} else if (!strncasecmp(last_header_line, "Content-Type:", sizeof("Content-Type:")-1)) {
261275
php_stream_notify_info(context, PHP_STREAM_NOTIFY_MIME_TYPE_IS, last_header_value, 0);
262276
} else if (!strncasecmp(last_header_line, "Content-Length:", sizeof("Content-Length:")-1)) {
@@ -560,6 +574,8 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
560574
}
561575
}
562576

577+
php_stream_http_response_header_info_init(&header_info);
578+
563579
if (stream == NULL)
564580
goto out;
565581

@@ -941,8 +957,6 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
941957
}
942958
}
943959

944-
php_stream_http_response_header_info_init(&header_info);
945-
946960
/* read past HTTP headers */
947961
while (!php_stream_eof(stream)) {
948962
size_t http_header_line_length;
@@ -1012,12 +1026,12 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10121026
last_header_line_str, NULL, NULL, response_code, response_header, &header_info);
10131027
}
10141028

1015-
if (!reqok || (header_info.location[0] != '\0' && header_info.follow_location)) {
1029+
if (!reqok || (header_info.location != NULL && header_info.follow_location)) {
10161030
if (!header_info.follow_location || (((options & STREAM_ONLY_GET_HEADERS) || ignore_errors) && redirect_max <= 1)) {
10171031
goto out;
10181032
}
10191033

1020-
if (header_info.location[0] != '\0')
1034+
if (header_info.location != NULL)
10211035
php_stream_notify_info(context, PHP_STREAM_NOTIFY_REDIRECTED, header_info.location, 0);
10221036

10231037
php_stream_close(stream);
@@ -1028,18 +1042,17 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10281042
header_info.transfer_encoding = NULL;
10291043
}
10301044

1031-
if (header_info.location[0] != '\0') {
1045+
if (header_info.location != NULL) {
10321046

1033-
char new_path[HTTP_HEADER_BLOCK_SIZE];
1034-
char loc_path[HTTP_HEADER_BLOCK_SIZE];
1047+
char *new_path = NULL;
10351048

1036-
*new_path='\0';
10371049
if (strlen(header_info.location) < 8 ||
10381050
(strncasecmp(header_info.location, "http://", sizeof("http://")-1) &&
10391051
strncasecmp(header_info.location, "https://", sizeof("https://")-1) &&
10401052
strncasecmp(header_info.location, "ftp://", sizeof("ftp://")-1) &&
10411053
strncasecmp(header_info.location, "ftps://", sizeof("ftps://")-1)))
10421054
{
1055+
char *loc_path = NULL;
10431056
if (*header_info.location != '/') {
10441057
if (*(header_info.location+1) != '\0' && resource->path) {
10451058
char *s = strrchr(ZSTR_VAL(resource->path), '/');
@@ -1057,31 +1070,35 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10571070
if (resource->path &&
10581071
ZSTR_VAL(resource->path)[0] == '/' &&
10591072
ZSTR_VAL(resource->path)[1] == '\0') {
1060-
snprintf(loc_path, sizeof(loc_path) - 1, "%s%s",
1061-
ZSTR_VAL(resource->path), header_info.location);
1073+
spprintf(&loc_path, 0, "%s%s", ZSTR_VAL(resource->path), header_info.location);
10621074
} else {
1063-
snprintf(loc_path, sizeof(loc_path) - 1, "%s/%s",
1064-
ZSTR_VAL(resource->path), header_info.location);
1075+
spprintf(&loc_path, 0, "%s/%s", ZSTR_VAL(resource->path), header_info.location);
10651076
}
10661077
} else {
1067-
snprintf(loc_path, sizeof(loc_path) - 1, "/%s", header_info.location);
1078+
spprintf(&loc_path, 0, "/%s", header_info.location);
10681079
}
10691080
} else {
1070-
strlcpy(loc_path, header_info.location, sizeof(loc_path));
1081+
loc_path = header_info.location;
1082+
header_info.location = NULL;
10711083
}
10721084
if ((use_ssl && resource->port != 443) || (!use_ssl && resource->port != 80)) {
1073-
snprintf(new_path, sizeof(new_path) - 1, "%s://%s:%d%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), resource->port, loc_path);
1085+
spprintf(&new_path, 0, "%s://%s:%d%s", ZSTR_VAL(resource->scheme),
1086+
ZSTR_VAL(resource->host), resource->port, loc_path);
10741087
} else {
1075-
snprintf(new_path, sizeof(new_path) - 1, "%s://%s%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), loc_path);
1088+
spprintf(&new_path, 0, "%s://%s%s", ZSTR_VAL(resource->scheme),
1089+
ZSTR_VAL(resource->host), loc_path);
10761090
}
1091+
efree(loc_path);
10771092
} else {
1078-
strlcpy(new_path, header_info.location, sizeof(new_path));
1093+
new_path = header_info.location;
1094+
header_info.location = NULL;
10791095
}
10801096

10811097
php_url_free(resource);
10821098
/* check for invalid redirection URLs */
10831099
if ((resource = php_url_parse(new_path)) == NULL) {
10841100
php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path);
1101+
efree(new_path);
10851102
goto out;
10861103
}
10871104

@@ -1093,6 +1110,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10931110
while (s < e) { \
10941111
if (iscntrl(*s)) { \
10951112
php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path); \
1113+
efree(new_path); \
10961114
goto out; \
10971115
} \
10981116
s++; \
@@ -1115,6 +1133,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
11151133
stream = php_stream_url_wrap_http_ex(
11161134
wrapper, new_path, mode, options, opened_path, context,
11171135
--redirect_max, new_flags, response_header STREAMS_CC);
1136+
efree(new_path);
11181137
} else {
11191138
php_stream_wrapper_log_error(wrapper, options, "HTTP request failed! %s", tmp_line);
11201139
}
@@ -1127,6 +1146,10 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
11271146
efree(http_header_line);
11281147
}
11291148

1149+
if (header_info.location != NULL) {
1150+
efree(header_info.location);
1151+
}
1152+
11301153
if (resource) {
11311154
php_url_free(resource);
11321155
}

‎ext/standard/tests/http/ghsa-52jp-hrpf-2jff-001.phpt‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
--TEST--
2+
GHSA-52jp-hrpf-2jff: HTTP stream wrapper truncate redirect location to 1024 bytes (success)
3+
--FILE--
4+
<?php
5+
$serverCode = <<<'CODE'
6+
$ctxt = stream_context_create([
7+
"socket" => [
8+
"tcp_nodelay" => true
9+
]
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+
$conn = stream_socket_accept($server);
17+
18+
phpt_notify(message:"server-accepted");
19+
20+
$loc = str_repeat("y", 8000);
21+
fwrite($conn, "HTTP/1.0 301 Ok\r\nContent-Type: text/html;\r\nLocation: $loc\r\n\r\nbody\r\n");
22+
CODE;
23+
24+
$clientCode = <<<'CODE'
25+
function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) {
26+
switch($notification_code) {
27+
case STREAM_NOTIFY_MIME_TYPE_IS:
28+
echo "Found the mime-type: ", $message, PHP_EOL;
29+
break;
30+
case STREAM_NOTIFY_REDIRECTED:
31+
echo "Redirected: ";
32+
var_dump($message);
33+
}
34+
}
35+
36+
$ctx = stream_context_create();
37+
stream_context_set_params($ctx, array("notification" => "stream_notification_callback"));
38+
var_dump(trim(file_get_contents("http://{{ ADDR }}", false, $ctx)));
39+
var_dump($http_response_header);
40+
CODE;
41+
42+
include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__);
43+
ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
44+
?>
45+
--EXPECTF--
46+
Found the mime-type: text/html;
47+
Redirected: string(8000) "%s"
48+
49+
Warning: file_get_contents(http://127.0.0.1:%d): Failed to open stream: %s
50+
string(0) ""
51+
array(3) {
52+
[0]=>
53+
string(15) "HTTP/1.0 301 Ok"
54+
[1]=>
55+
string(24) "Content-Type: text/html;"
56+
[2]=>
57+
string(8010) "Location: %s"
58+
}

‎ext/standard/tests/http/ghsa-52jp-hrpf-2jff-002.phpt‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
--TEST--
2+
GHSA-52jp-hrpf-2jff: HTTP stream wrapper truncate redirect location to 1024 bytes (over limit)
3+
--FILE--
4+
<?php
5+
$serverCode = <<<'CODE'
6+
$ctxt = stream_context_create([
7+
"socket" => [
8+
"tcp_nodelay" => true
9+
]
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+
$conn = stream_socket_accept($server);
17+
18+
phpt_notify(message:"server-accepted");
19+
20+
$loc = str_repeat("y", 9000);
21+
fwrite($conn, "HTTP/1.0 301 Ok\r\nContent-Type: text/html;\r\nLocation: $loc\r\n\r\nbody\r\n");
22+
CODE;
23+
24+
$clientCode = <<<'CODE'
25+
function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) {
26+
switch($notification_code) {
27+
case STREAM_NOTIFY_MIME_TYPE_IS:
28+
echo "Found the mime-type: ", $message, PHP_EOL;
29+
break;
30+
case STREAM_NOTIFY_REDIRECTED:
31+
echo "Redirected: ";
32+
var_dump($message);
33+
}
34+
}
35+
36+
$ctx = stream_context_create();
37+
stream_context_set_params($ctx, array("notification" => "stream_notification_callback"));
38+
var_dump(trim(file_get_contents("http://{{ ADDR }}", false, $ctx)));
39+
var_dump($http_response_header);
40+
CODE;
41+
42+
include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__);
43+
ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
44+
?>
45+
--EXPECTF--
46+
Found the mime-type: text/html;
47+
48+
Warning: file_get_contents(http://127.0.0.1:%d): Failed to open stream: HTTP Location header size is over the limit of 8182 bytes in %s
49+
string(0) ""
50+
array(2) {
51+
[0]=>
52+
string(15) "HTTP/1.0 301 Ok"
53+
[1]=>
54+
string(24) "Content-Type: text/html;"
55+
}

0 commit comments

Comments
 (0)