Skip to content

Commit adc7e9f

Browse files
bukkaremicollet
bukka
authored and
remicollet
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. (cherry picked from commit ac1a054bb3eb5994a199e8b18cca28cbabf5943e)
1 parent e81d0cd commit adc7e9f

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
@@ -119,17 +120,15 @@ typedef struct _php_stream_http_response_header_info {
119120
size_t file_size;
120121
bool error;
121122
bool follow_location;
122-
char location[HTTP_HEADER_BLOCK_SIZE];
123+
char *location;
124+
size_t location_len;
123125
} php_stream_http_response_header_info;
124126

125127
static void php_stream_http_response_header_info_init(
126128
php_stream_http_response_header_info *header_info)
127129
{
128-
header_info->transfer_encoding = NULL;
129-
header_info->file_size = 0;
130-
header_info->error = false;
130+
memset(header_info, 0, sizeof(php_stream_http_response_header_info));
131131
header_info->follow_location = 1;
132-
header_info->location[0] = '\0';
133132
}
134133

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

555+
php_stream_http_response_header_info_init(&header_info);
556+
541557
if (stream == NULL)
542558
goto out;
543559

@@ -919,8 +935,6 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
919935
}
920936
}
921937

922-
php_stream_http_response_header_info_init(&header_info);
923-
924938
/* read past HTTP headers */
925939
while (!php_stream_eof(stream)) {
926940
size_t http_header_line_length;
@@ -990,12 +1004,12 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
9901004
last_header_line_str, NULL, NULL, response_code, response_header, &header_info);
9911005
}
9921006

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

998-
if (header_info.location[0] != '\0')
1012+
if (header_info.location != NULL)
9991013
php_stream_notify_info(context, PHP_STREAM_NOTIFY_REDIRECTED, header_info.location, 0);
10001014

10011015
php_stream_close(stream);
@@ -1006,18 +1020,17 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10061020
header_info.transfer_encoding = NULL;
10071021
}
10081022

1009-
if (header_info.location[0] != '\0') {
1023+
if (header_info.location != NULL) {
10101024

1011-
char new_path[HTTP_HEADER_BLOCK_SIZE];
1012-
char loc_path[HTTP_HEADER_BLOCK_SIZE];
1025+
char *new_path = NULL;
10131026

1014-
*new_path='\0';
10151027
if (strlen(header_info.location) < 8 ||
10161028
(strncasecmp(header_info.location, "http://", sizeof("http://")-1) &&
10171029
strncasecmp(header_info.location, "https://", sizeof("https://")-1) &&
10181030
strncasecmp(header_info.location, "ftp://", sizeof("ftp://")-1) &&
10191031
strncasecmp(header_info.location, "ftps://", sizeof("ftps://")-1)))
10201032
{
1033+
char *loc_path = NULL;
10211034
if (*header_info.location != '/') {
10221035
if (*(header_info.location+1) != '\0' && resource->path) {
10231036
char *s = strrchr(ZSTR_VAL(resource->path), '/');
@@ -1035,31 +1048,35 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10351048
if (resource->path &&
10361049
ZSTR_VAL(resource->path)[0] == '/' &&
10371050
ZSTR_VAL(resource->path)[1] == '\0') {
1038-
snprintf(loc_path, sizeof(loc_path) - 1, "%s%s",
1039-
ZSTR_VAL(resource->path), header_info.location);
1051+
spprintf(&loc_path, 0, "%s%s", ZSTR_VAL(resource->path), header_info.location);
10401052
} else {
1041-
snprintf(loc_path, sizeof(loc_path) - 1, "%s/%s",
1042-
ZSTR_VAL(resource->path), header_info.location);
1053+
spprintf(&loc_path, 0, "%s/%s", ZSTR_VAL(resource->path), header_info.location);
10431054
}
10441055
} else {
1045-
snprintf(loc_path, sizeof(loc_path) - 1, "/%s", header_info.location);
1056+
spprintf(&loc_path, 0, "/%s", header_info.location);
10461057
}
10471058
} else {
1048-
strlcpy(loc_path, header_info.location, sizeof(loc_path));
1059+
loc_path = header_info.location;
1060+
header_info.location = NULL;
10491061
}
10501062
if ((use_ssl && resource->port != 443) || (!use_ssl && resource->port != 80)) {
1051-
snprintf(new_path, sizeof(new_path) - 1, "%s://%s:%d%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), resource->port, loc_path);
1063+
spprintf(&new_path, 0, "%s://%s:%d%s", ZSTR_VAL(resource->scheme),
1064+
ZSTR_VAL(resource->host), resource->port, loc_path);
10521065
} else {
1053-
snprintf(new_path, sizeof(new_path) - 1, "%s://%s%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), loc_path);
1066+
spprintf(&new_path, 0, "%s://%s%s", ZSTR_VAL(resource->scheme),
1067+
ZSTR_VAL(resource->host), loc_path);
10541068
}
1069+
efree(loc_path);
10551070
} else {
1056-
strlcpy(new_path, header_info.location, sizeof(new_path));
1071+
new_path = header_info.location;
1072+
header_info.location = NULL;
10571073
}
10581074

10591075
php_url_free(resource);
10601076
/* check for invalid redirection URLs */
10611077
if ((resource = php_url_parse(new_path)) == NULL) {
10621078
php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path);
1079+
efree(new_path);
10631080
goto out;
10641081
}
10651082

@@ -1071,6 +1088,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10711088
while (s < e) { \
10721089
if (iscntrl(*s)) { \
10731090
php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path); \
1091+
efree(new_path); \
10741092
goto out; \
10751093
} \
10761094
s++; \
@@ -1086,6 +1104,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10861104
stream = php_stream_url_wrap_http_ex(
10871105
wrapper, new_path, mode, options, opened_path, context,
10881106
--redirect_max, HTTP_WRAPPER_REDIRECTED, response_header STREAMS_CC);
1107+
efree(new_path);
10891108
} else {
10901109
php_stream_wrapper_log_error(wrapper, options, "HTTP request failed! %s", tmp_line);
10911110
}
@@ -1098,6 +1117,10 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper,
10981117
efree(http_header_line);
10991118
}
11001119

1120+
if (header_info.location != NULL) {
1121+
efree(header_info.location);
1122+
}
1123+
11011124
if (resource) {
11021125
php_url_free(resource);
11031126
}

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