Skip to content

Commit 4a0c42f

Browse files
bukkaadoy
bukka
authored and
adoy
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 45687b5 commit 4a0c42f

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)) {
@@ -547,6 +561,8 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
547561
}
548562
}
549563

564+
php_stream_http_response_header_info_init(&header_info);
565+
550566
if (stream == NULL)
551567
goto out;
552568

@@ -928,8 +944,6 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
928944
}
929945
}
930946

931-
php_stream_http_response_header_info_init(&header_info);
932-
933947
/* read past HTTP headers */
934948
while (!php_stream_eof(stream)) {
935949
size_t http_header_line_length;
@@ -999,12 +1013,12 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
9991013
last_header_line_str, NULL, NULL, response_code, response_header, &header_info);
10001014
}
10011015

1002-
if (!reqok || (header_info.location[0] != '\0' && header_info.follow_location)) {
1016+
if (!reqok || (header_info.location != NULL && header_info.follow_location)) {
10031017
if (!header_info.follow_location || (((options & STREAM_ONLY_GET_HEADERS) || ignore_errors) && redirect_max <= 1)) {
10041018
goto out;
10051019
}
10061020

1007-
if (header_info.location[0] != '\0')
1021+
if (header_info.location != NULL)
10081022
php_stream_notify_info(context, PHP_STREAM_NOTIFY_REDIRECTED, header_info.location, 0);
10091023

10101024
php_stream_close(stream);
@@ -1015,18 +1029,17 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10151029
header_info.transfer_encoding = NULL;
10161030
}
10171031

1018-
if (header_info.location[0] != '\0') {
1032+
if (header_info.location != NULL) {
10191033

1020-
char new_path[HTTP_HEADER_BLOCK_SIZE];
1021-
char loc_path[HTTP_HEADER_BLOCK_SIZE];
1034+
char *new_path = NULL;
10221035

1023-
*new_path='\0';
10241036
if (strlen(header_info.location) < 8 ||
10251037
(strncasecmp(header_info.location, "http://", sizeof("http://")-1) &&
10261038
strncasecmp(header_info.location, "https://", sizeof("https://")-1) &&
10271039
strncasecmp(header_info.location, "ftp://", sizeof("ftp://")-1) &&
10281040
strncasecmp(header_info.location, "ftps://", sizeof("ftps://")-1)))
10291041
{
1042+
char *loc_path = NULL;
10301043
if (*header_info.location != '/') {
10311044
if (*(header_info.location+1) != '\0' && resource->path) {
10321045
char *s = strrchr(ZSTR_VAL(resource->path), '/');
@@ -1044,31 +1057,35 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10441057
if (resource->path &&
10451058
ZSTR_VAL(resource->path)[0] == '/' &&
10461059
ZSTR_VAL(resource->path)[1] == '\0') {
1047-
snprintf(loc_path, sizeof(loc_path) - 1, "%s%s",
1048-
ZSTR_VAL(resource->path), header_info.location);
1060+
spprintf(&loc_path, 0, "%s%s", ZSTR_VAL(resource->path), header_info.location);
10491061
} else {
1050-
snprintf(loc_path, sizeof(loc_path) - 1, "%s/%s",
1051-
ZSTR_VAL(resource->path), header_info.location);
1062+
spprintf(&loc_path, 0, "%s/%s", ZSTR_VAL(resource->path), header_info.location);
10521063
}
10531064
} else {
1054-
snprintf(loc_path, sizeof(loc_path) - 1, "/%s", header_info.location);
1065+
spprintf(&loc_path, 0, "/%s", header_info.location);
10551066
}
10561067
} else {
1057-
strlcpy(loc_path, header_info.location, sizeof(loc_path));
1068+
loc_path = header_info.location;
1069+
header_info.location = NULL;
10581070
}
10591071
if ((use_ssl && resource->port != 443) || (!use_ssl && resource->port != 80)) {
1060-
snprintf(new_path, sizeof(new_path) - 1, "%s://%s:%d%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), resource->port, loc_path);
1072+
spprintf(&new_path, 0, "%s://%s:%d%s", ZSTR_VAL(resource->scheme),
1073+
ZSTR_VAL(resource->host), resource->port, loc_path);
10611074
} else {
1062-
snprintf(new_path, sizeof(new_path) - 1, "%s://%s%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), loc_path);
1075+
spprintf(&new_path, 0, "%s://%s%s", ZSTR_VAL(resource->scheme),
1076+
ZSTR_VAL(resource->host), loc_path);
10631077
}
1078+
efree(loc_path);
10641079
} else {
1065-
strlcpy(new_path, header_info.location, sizeof(new_path));
1080+
new_path = header_info.location;
1081+
header_info.location = NULL;
10661082
}
10671083

10681084
php_url_free(resource);
10691085
/* check for invalid redirection URLs */
10701086
if ((resource = php_url_parse(new_path)) == NULL) {
10711087
php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path);
1088+
efree(new_path);
10721089
goto out;
10731090
}
10741091

@@ -1080,6 +1097,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10801097
while (s < e) { \
10811098
if (iscntrl(*s)) { \
10821099
php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path); \
1100+
efree(new_path); \
10831101
goto out; \
10841102
} \
10851103
s++; \
@@ -1102,6 +1120,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
11021120
stream = php_stream_url_wrap_http_ex(
11031121
wrapper, new_path, mode, options, opened_path, context,
11041122
--redirect_max, new_flags, response_header STREAMS_CC);
1123+
efree(new_path);
11051124
} else {
11061125
php_stream_wrapper_log_error(wrapper, options, "HTTP request failed! %s", tmp_line);
11071126
}
@@ -1114,6 +1133,10 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
11141133
efree(http_header_line);
11151134
}
11161135

1136+
if (header_info.location != NULL) {
1137+
efree(header_info.location);
1138+
}
1139+
11171140
if (resource) {
11181141
php_url_free(resource);
11191142
}

‎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)