Skip to content

Commit ac1a054

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 0548c4c commit ac1a054

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)) {
@@ -536,6 +550,8 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
536550
}
537551
}
538552

553+
php_stream_http_response_header_info_init(&header_info);
554+
539555
if (stream == NULL)
540556
goto out;
541557

@@ -917,8 +933,6 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
917933
}
918934
}
919935

920-
php_stream_http_response_header_info_init(&header_info);
921-
922936
/* read past HTTP headers */
923937
while (!php_stream_eof(stream)) {
924938
size_t http_header_line_length;
@@ -988,12 +1002,12 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
9881002
last_header_line_str, NULL, NULL, response_code, response_header, &header_info);
9891003
}
9901004

991-
if (!reqok || (header_info.location[0] != '\0' && header_info.follow_location)) {
1005+
if (!reqok || (header_info.location != NULL && header_info.follow_location)) {
9921006
if (!header_info.follow_location || (((options & STREAM_ONLY_GET_HEADERS) || ignore_errors) && redirect_max <= 1)) {
9931007
goto out;
9941008
}
9951009

996-
if (header_info.location[0] != '\0')
1010+
if (header_info.location != NULL)
9971011
php_stream_notify_info(context, PHP_STREAM_NOTIFY_REDIRECTED, header_info.location, 0);
9981012

9991013
php_stream_close(stream);
@@ -1004,18 +1018,17 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10041018
header_info.transfer_encoding = NULL;
10051019
}
10061020

1007-
if (header_info.location[0] != '\0') {
1021+
if (header_info.location != NULL) {
10081022

1009-
char new_path[HTTP_HEADER_BLOCK_SIZE];
1010-
char loc_path[HTTP_HEADER_BLOCK_SIZE];
1023+
char *new_path = NULL;
10111024

1012-
*new_path='\0';
10131025
if (strlen(header_info.location) < 8 ||
10141026
(strncasecmp(header_info.location, "http://", sizeof("http://")-1) &&
10151027
strncasecmp(header_info.location, "https://", sizeof("https://")-1) &&
10161028
strncasecmp(header_info.location, "ftp://", sizeof("ftp://")-1) &&
10171029
strncasecmp(header_info.location, "ftps://", sizeof("ftps://")-1)))
10181030
{
1031+
char *loc_path = NULL;
10191032
if (*header_info.location != '/') {
10201033
if (*(header_info.location+1) != '\0' && resource->path) {
10211034
char *s = strrchr(ZSTR_VAL(resource->path), '/');
@@ -1033,31 +1046,35 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10331046
if (resource->path &&
10341047
ZSTR_VAL(resource->path)[0] == '/' &&
10351048
ZSTR_VAL(resource->path)[1] == '\0') {
1036-
snprintf(loc_path, sizeof(loc_path) - 1, "%s%s",
1037-
ZSTR_VAL(resource->path), header_info.location);
1049+
spprintf(&loc_path, 0, "%s%s", ZSTR_VAL(resource->path), header_info.location);
10381050
} else {
1039-
snprintf(loc_path, sizeof(loc_path) - 1, "%s/%s",
1040-
ZSTR_VAL(resource->path), header_info.location);
1051+
spprintf(&loc_path, 0, "%s/%s", ZSTR_VAL(resource->path), header_info.location);
10411052
}
10421053
} else {
1043-
snprintf(loc_path, sizeof(loc_path) - 1, "/%s", header_info.location);
1054+
spprintf(&loc_path, 0, "/%s", header_info.location);
10441055
}
10451056
} else {
1046-
strlcpy(loc_path, header_info.location, sizeof(loc_path));
1057+
loc_path = header_info.location;
1058+
header_info.location = NULL;
10471059
}
10481060
if ((use_ssl && resource->port != 443) || (!use_ssl && resource->port != 80)) {
1049-
snprintf(new_path, sizeof(new_path) - 1, "%s://%s:%d%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), resource->port, loc_path);
1061+
spprintf(&new_path, 0, "%s://%s:%d%s", ZSTR_VAL(resource->scheme),
1062+
ZSTR_VAL(resource->host), resource->port, loc_path);
10501063
} else {
1051-
snprintf(new_path, sizeof(new_path) - 1, "%s://%s%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), loc_path);
1064+
spprintf(&new_path, 0, "%s://%s%s", ZSTR_VAL(resource->scheme),
1065+
ZSTR_VAL(resource->host), loc_path);
10521066
}
1067+
efree(loc_path);
10531068
} else {
1054-
strlcpy(new_path, header_info.location, sizeof(new_path));
1069+
new_path = header_info.location;
1070+
header_info.location = NULL;
10551071
}
10561072

10571073
php_url_free(resource);
10581074
/* check for invalid redirection URLs */
10591075
if ((resource = php_url_parse(new_path)) == NULL) {
10601076
php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path);
1077+
efree(new_path);
10611078
goto out;
10621079
}
10631080

@@ -1069,6 +1086,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10691086
while (s < e) { \
10701087
if (iscntrl(*s)) { \
10711088
php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path); \
1089+
efree(new_path); \
10721090
goto out; \
10731091
} \
10741092
s++; \
@@ -1091,6 +1109,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10911109
stream = php_stream_url_wrap_http_ex(
10921110
wrapper, new_path, mode, options, opened_path, context,
10931111
--redirect_max, new_flags, response_header STREAMS_CC);
1112+
efree(new_path);
10941113
} else {
10951114
php_stream_wrapper_log_error(wrapper, options, "HTTP request failed! %s", tmp_line);
10961115
}
@@ -1103,6 +1122,10 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
11031122
efree(http_header_line);
11041123
}
11051124

1125+
if (header_info.location != NULL) {
1126+
efree(header_info.location);
1127+
}
1128+
11061129
if (resource) {
11071130
php_url_free(resource);
11081131
}

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