Skip to content

Commit 2488e73

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 61bb8ef commit 2488e73

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
@@ -148,17 +149,15 @@ typedef struct _php_stream_http_response_header_info {
148149
size_t file_size;
149150
bool error;
150151
bool follow_location;
151-
char location[HTTP_HEADER_BLOCK_SIZE];
152+
char *location;
153+
size_t location_len;
152154
} php_stream_http_response_header_info;
153155

154156
static void php_stream_http_response_header_info_init(
155157
php_stream_http_response_header_info *header_info)
156158
{
157-
header_info->transfer_encoding = NULL;
158-
header_info->file_size = 0;
159-
header_info->error = false;
159+
memset(header_info, 0, sizeof(php_stream_http_response_header_info));
160160
header_info->follow_location = 1;
161-
header_info->location[0] = '\0';
162161
}
163162

164163
/* Trim white spaces from response header line and update its length */
@@ -284,7 +283,22 @@ static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *w
284283
* RFC 7238 defines 308: http://tools.ietf.org/html/rfc7238 */
285284
header_info->follow_location = 0;
286285
}
287-
strlcpy(header_info->location, last_header_value, sizeof(header_info->location));
286+
size_t last_header_value_len = strlen(last_header_value);
287+
if (last_header_value_len > HTTP_HEADER_MAX_LOCATION_SIZE) {
288+
header_info->error = true;
289+
php_stream_wrapper_log_error(wrapper, options,
290+
"HTTP Location header size is over the limit of %d bytes",
291+
HTTP_HEADER_MAX_LOCATION_SIZE);
292+
zend_string_efree(last_header_line_str);
293+
return NULL;
294+
}
295+
if (header_info->location_len == 0) {
296+
header_info->location = emalloc(last_header_value_len + 1);
297+
} else if (header_info->location_len <= last_header_value_len) {
298+
header_info->location = erealloc(header_info->location, last_header_value_len + 1);
299+
}
300+
header_info->location_len = last_header_value_len;
301+
memcpy(header_info->location, last_header_value, last_header_value_len + 1);
288302
} else if (!strncasecmp(last_header_line, "Content-Type:", sizeof("Content-Type:")-1)) {
289303
php_stream_notify_info(context, PHP_STREAM_NOTIFY_MIME_TYPE_IS, last_header_value, 0);
290304
} else if (!strncasecmp(last_header_line, "Content-Length:", sizeof("Content-Length:")-1)) {
@@ -554,6 +568,8 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
554568
}
555569
}
556570

571+
php_stream_http_response_header_info_init(&header_info);
572+
557573
if (stream == NULL)
558574
goto out;
559575

@@ -935,8 +951,6 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
935951
}
936952
}
937953

938-
php_stream_http_response_header_info_init(&header_info);
939-
940954
/* read past HTTP headers */
941955
while (!php_stream_eof(stream)) {
942956
size_t http_header_line_length;
@@ -1006,12 +1020,12 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10061020
last_header_line_str, NULL, NULL, response_code, response_header, &header_info);
10071021
}
10081022

1009-
if (!reqok || (header_info.location[0] != '\0' && header_info.follow_location)) {
1023+
if (!reqok || (header_info.location != NULL && header_info.follow_location)) {
10101024
if (!header_info.follow_location || (((options & STREAM_ONLY_GET_HEADERS) || ignore_errors) && redirect_max <= 1)) {
10111025
goto out;
10121026
}
10131027

1014-
if (header_info.location[0] != '\0')
1028+
if (header_info.location != NULL)
10151029
php_stream_notify_info(context, PHP_STREAM_NOTIFY_REDIRECTED, header_info.location, 0);
10161030

10171031
php_stream_close(stream);
@@ -1022,18 +1036,17 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10221036
header_info.transfer_encoding = NULL;
10231037
}
10241038

1025-
if (header_info.location[0] != '\0') {
1039+
if (header_info.location != NULL) {
10261040

1027-
char new_path[HTTP_HEADER_BLOCK_SIZE];
1028-
char loc_path[HTTP_HEADER_BLOCK_SIZE];
1041+
char *new_path = NULL;
10291042

1030-
*new_path='\0';
10311043
if (strlen(header_info.location) < 8 ||
10321044
(strncasecmp(header_info.location, "http://", sizeof("http://")-1) &&
10331045
strncasecmp(header_info.location, "https://", sizeof("https://")-1) &&
10341046
strncasecmp(header_info.location, "ftp://", sizeof("ftp://")-1) &&
10351047
strncasecmp(header_info.location, "ftps://", sizeof("ftps://")-1)))
10361048
{
1049+
char *loc_path = NULL;
10371050
if (*header_info.location != '/') {
10381051
if (*(header_info.location+1) != '\0' && resource->path) {
10391052
char *s = strrchr(ZSTR_VAL(resource->path), '/');
@@ -1051,31 +1064,35 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10511064
if (resource->path &&
10521065
ZSTR_VAL(resource->path)[0] == '/' &&
10531066
ZSTR_VAL(resource->path)[1] == '\0') {
1054-
snprintf(loc_path, sizeof(loc_path) - 1, "%s%s",
1055-
ZSTR_VAL(resource->path), header_info.location);
1067+
spprintf(&loc_path, 0, "%s%s", ZSTR_VAL(resource->path), header_info.location);
10561068
} else {
1057-
snprintf(loc_path, sizeof(loc_path) - 1, "%s/%s",
1058-
ZSTR_VAL(resource->path), header_info.location);
1069+
spprintf(&loc_path, 0, "%s/%s", ZSTR_VAL(resource->path), header_info.location);
10591070
}
10601071
} else {
1061-
snprintf(loc_path, sizeof(loc_path) - 1, "/%s", header_info.location);
1072+
spprintf(&loc_path, 0, "/%s", header_info.location);
10621073
}
10631074
} else {
1064-
strlcpy(loc_path, header_info.location, sizeof(loc_path));
1075+
loc_path = header_info.location;
1076+
header_info.location = NULL;
10651077
}
10661078
if ((use_ssl && resource->port != 443) || (!use_ssl && resource->port != 80)) {
1067-
snprintf(new_path, sizeof(new_path) - 1, "%s://%s:%d%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), resource->port, loc_path);
1079+
spprintf(&new_path, 0, "%s://%s:%d%s", ZSTR_VAL(resource->scheme),
1080+
ZSTR_VAL(resource->host), resource->port, loc_path);
10681081
} else {
1069-
snprintf(new_path, sizeof(new_path) - 1, "%s://%s%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), loc_path);
1082+
spprintf(&new_path, 0, "%s://%s%s", ZSTR_VAL(resource->scheme),
1083+
ZSTR_VAL(resource->host), loc_path);
10701084
}
1085+
efree(loc_path);
10711086
} else {
1072-
strlcpy(new_path, header_info.location, sizeof(new_path));
1087+
new_path = header_info.location;
1088+
header_info.location = NULL;
10731089
}
10741090

10751091
php_url_free(resource);
10761092
/* check for invalid redirection URLs */
10771093
if ((resource = php_url_parse(new_path)) == NULL) {
10781094
php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path);
1095+
efree(new_path);
10791096
goto out;
10801097
}
10811098

@@ -1087,6 +1104,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10871104
while (s < e) { \
10881105
if (iscntrl(*s)) { \
10891106
php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path); \
1107+
efree(new_path); \
10901108
goto out; \
10911109
} \
10921110
s++; \
@@ -1109,6 +1127,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
11091127
stream = php_stream_url_wrap_http_ex(
11101128
wrapper, new_path, mode, options, opened_path, context,
11111129
--redirect_max, new_flags, response_header STREAMS_CC);
1130+
efree(new_path);
11121131
} else {
11131132
php_stream_wrapper_log_error(wrapper, options, "HTTP request failed! %s", tmp_line);
11141133
}
@@ -1121,6 +1140,10 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
11211140
efree(http_header_line);
11221141
}
11231142

1143+
if (header_info.location != NULL) {
1144+
efree(header_info.location);
1145+
}
1146+
11241147
if (resource) {
11251148
php_url_free(resource);
11261149
}

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