From adc7e9f20c9a9aab9cd23ca47ec3fb96287898ae Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Tue, 4 Mar 2025 09:01:34 +0100 Subject: [PATCH 03/11] 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) --- ext/standard/http_fopen_wrapper.c | 87 ++++++++++++------- .../tests/http/ghsa-52jp-hrpf-2jff-001.phpt | 58 +++++++++++++ .../tests/http/ghsa-52jp-hrpf-2jff-002.phpt | 55 ++++++++++++ 3 files changed, 168 insertions(+), 32 deletions(-) create mode 100644 ext/standard/tests/http/ghsa-52jp-hrpf-2jff-001.phpt create mode 100644 ext/standard/tests/http/ghsa-52jp-hrpf-2jff-002.phpt diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index 7ee22b85f88..e9b2486a7c9 100644 --- a/ext/standard/http_fopen_wrapper.c +++ b/ext/standard/http_fopen_wrapper.c @@ -67,15 +67,16 @@ #include "php_fopen_wrappers.h" -#define HTTP_HEADER_BLOCK_SIZE 1024 -#define PHP_URL_REDIRECT_MAX 20 -#define HTTP_HEADER_USER_AGENT 1 -#define HTTP_HEADER_HOST 2 -#define HTTP_HEADER_AUTH 4 -#define HTTP_HEADER_FROM 8 -#define HTTP_HEADER_CONTENT_LENGTH 16 -#define HTTP_HEADER_TYPE 32 -#define HTTP_HEADER_CONNECTION 64 +#define HTTP_HEADER_BLOCK_SIZE 1024 +#define HTTP_HEADER_MAX_LOCATION_SIZE 8182 /* 8192 - 10 (size of "Location: ") */ +#define PHP_URL_REDIRECT_MAX 20 +#define HTTP_HEADER_USER_AGENT 1 +#define HTTP_HEADER_HOST 2 +#define HTTP_HEADER_AUTH 4 +#define HTTP_HEADER_FROM 8 +#define HTTP_HEADER_CONTENT_LENGTH 16 +#define HTTP_HEADER_TYPE 32 +#define HTTP_HEADER_CONNECTION 64 #define HTTP_WRAPPER_HEADER_INIT 1 #define HTTP_WRAPPER_REDIRECTED 2 @@ -119,17 +120,15 @@ typedef struct _php_stream_http_response_header_info { size_t file_size; bool error; bool follow_location; - char location[HTTP_HEADER_BLOCK_SIZE]; + char *location; + size_t location_len; } php_stream_http_response_header_info; static void php_stream_http_response_header_info_init( php_stream_http_response_header_info *header_info) { - header_info->transfer_encoding = NULL; - header_info->file_size = 0; - header_info->error = false; + memset(header_info, 0, sizeof(php_stream_http_response_header_info)); header_info->follow_location = 1; - header_info->location[0] = '\0'; } /* 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 * RFC 7238 defines 308: http://tools.ietf.org/html/rfc7238 */ header_info->follow_location = 0; } - strlcpy(header_info->location, last_header_value, sizeof(header_info->location)); + size_t last_header_value_len = strlen(last_header_value); + if (last_header_value_len > HTTP_HEADER_MAX_LOCATION_SIZE) { + header_info->error = true; + php_stream_wrapper_log_error(wrapper, options, + "HTTP Location header size is over the limit of %d bytes", + HTTP_HEADER_MAX_LOCATION_SIZE); + zend_string_efree(last_header_line_str); + return NULL; + } + if (header_info->location_len == 0) { + header_info->location = emalloc(last_header_value_len + 1); + } else if (header_info->location_len <= last_header_value_len) { + header_info->location = erealloc(header_info->location, last_header_value_len + 1); + } + header_info->location_len = last_header_value_len; + memcpy(header_info->location, last_header_value, last_header_value_len + 1); } else if (!strncasecmp(last_header_line, "Content-Type:", sizeof("Content-Type:")-1)) { php_stream_notify_info(context, PHP_STREAM_NOTIFY_MIME_TYPE_IS, last_header_value, 0); } else if (!strncasecmp(last_header_line, "Content-Length:", sizeof("Content-Length:")-1)) { @@ -538,6 +552,8 @@ finish: } } + php_stream_http_response_header_info_init(&header_info); + if (stream == NULL) goto out; @@ -919,8 +935,6 @@ finish: } } - php_stream_http_response_header_info_init(&header_info); - /* read past HTTP headers */ while (!php_stream_eof(stream)) { size_t http_header_line_length; @@ -990,12 +1004,12 @@ finish: last_header_line_str, NULL, NULL, response_code, response_header, &header_info); } - if (!reqok || (header_info.location[0] != '\0' && header_info.follow_location)) { + if (!reqok || (header_info.location != NULL && header_info.follow_location)) { if (!header_info.follow_location || (((options & STREAM_ONLY_GET_HEADERS) || ignore_errors) && redirect_max <= 1)) { goto out; } - if (header_info.location[0] != '\0') + if (header_info.location != NULL) php_stream_notify_info(context, PHP_STREAM_NOTIFY_REDIRECTED, header_info.location, 0); php_stream_close(stream); @@ -1006,18 +1020,17 @@ finish: header_info.transfer_encoding = NULL; } - if (header_info.location[0] != '\0') { + if (header_info.location != NULL) { - char new_path[HTTP_HEADER_BLOCK_SIZE]; - char loc_path[HTTP_HEADER_BLOCK_SIZE]; + char *new_path = NULL; - *new_path='\0'; if (strlen(header_info.location) < 8 || (strncasecmp(header_info.location, "http://", sizeof("http://")-1) && strncasecmp(header_info.location, "https://", sizeof("https://")-1) && strncasecmp(header_info.location, "ftp://", sizeof("ftp://")-1) && strncasecmp(header_info.location, "ftps://", sizeof("ftps://")-1))) { + char *loc_path = NULL; if (*header_info.location != '/') { if (*(header_info.location+1) != '\0' && resource->path) { char *s = strrchr(ZSTR_VAL(resource->path), '/'); @@ -1035,31 +1048,35 @@ finish: if (resource->path && ZSTR_VAL(resource->path)[0] == '/' && ZSTR_VAL(resource->path)[1] == '\0') { - snprintf(loc_path, sizeof(loc_path) - 1, "%s%s", - ZSTR_VAL(resource->path), header_info.location); + spprintf(&loc_path, 0, "%s%s", ZSTR_VAL(resource->path), header_info.location); } else { - snprintf(loc_path, sizeof(loc_path) - 1, "%s/%s", - ZSTR_VAL(resource->path), header_info.location); + spprintf(&loc_path, 0, "%s/%s", ZSTR_VAL(resource->path), header_info.location); } } else { - snprintf(loc_path, sizeof(loc_path) - 1, "/%s", header_info.location); + spprintf(&loc_path, 0, "/%s", header_info.location); } } else { - strlcpy(loc_path, header_info.location, sizeof(loc_path)); + loc_path = header_info.location; + header_info.location = NULL; } if ((use_ssl && resource->port != 443) || (!use_ssl && resource->port != 80)) { - snprintf(new_path, sizeof(new_path) - 1, "%s://%s:%d%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), resource->port, loc_path); + spprintf(&new_path, 0, "%s://%s:%d%s", ZSTR_VAL(resource->scheme), + ZSTR_VAL(resource->host), resource->port, loc_path); } else { - snprintf(new_path, sizeof(new_path) - 1, "%s://%s%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), loc_path); + spprintf(&new_path, 0, "%s://%s%s", ZSTR_VAL(resource->scheme), + ZSTR_VAL(resource->host), loc_path); } + efree(loc_path); } else { - strlcpy(new_path, header_info.location, sizeof(new_path)); + new_path = header_info.location; + header_info.location = NULL; } php_url_free(resource); /* check for invalid redirection URLs */ if ((resource = php_url_parse(new_path)) == NULL) { php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path); + efree(new_path); goto out; } @@ -1071,6 +1088,7 @@ finish: while (s < e) { \ if (iscntrl(*s)) { \ php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path); \ + efree(new_path); \ goto out; \ } \ s++; \ @@ -1086,6 +1104,7 @@ finish: stream = php_stream_url_wrap_http_ex( wrapper, new_path, mode, options, opened_path, context, --redirect_max, HTTP_WRAPPER_REDIRECTED, response_header STREAMS_CC); + efree(new_path); } else { php_stream_wrapper_log_error(wrapper, options, "HTTP request failed! %s", tmp_line); } @@ -1098,6 +1117,10 @@ out: efree(http_header_line); } + if (header_info.location != NULL) { + efree(header_info.location); + } + if (resource) { php_url_free(resource); } diff --git a/ext/standard/tests/http/ghsa-52jp-hrpf-2jff-001.phpt b/ext/standard/tests/http/ghsa-52jp-hrpf-2jff-001.phpt new file mode 100644 index 00000000000..744cff9cc72 --- /dev/null +++ b/ext/standard/tests/http/ghsa-52jp-hrpf-2jff-001.phpt @@ -0,0 +1,58 @@ +--TEST-- +GHSA-52jp-hrpf-2jff: HTTP stream wrapper truncate redirect location to 1024 bytes (success) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + $loc = str_repeat("y", 8000); + fwrite($conn, "HTTP/1.0 301 Ok\r\nContent-Type: text/html;\r\nLocation: $loc\r\n\r\nbody\r\n"); +CODE; + +$clientCode = <<<'CODE' + function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) { + switch($notification_code) { + case STREAM_NOTIFY_MIME_TYPE_IS: + echo "Found the mime-type: ", $message, PHP_EOL; + break; + case STREAM_NOTIFY_REDIRECTED: + echo "Redirected: "; + var_dump($message); + } + } + + $ctx = stream_context_create(); + stream_context_set_params($ctx, array("notification" => "stream_notification_callback")); + var_dump(trim(file_get_contents("http://{{ ADDR }}", false, $ctx))); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +Found the mime-type: text/html; +Redirected: string(8000) "%s" + +Warning: file_get_contents(http://127.0.0.1:%d): Failed to open stream: %s +string(0) "" +array(3) { + [0]=> + string(15) "HTTP/1.0 301 Ok" + [1]=> + string(24) "Content-Type: text/html;" + [2]=> + string(8010) "Location: %s" +} diff --git a/ext/standard/tests/http/ghsa-52jp-hrpf-2jff-002.phpt b/ext/standard/tests/http/ghsa-52jp-hrpf-2jff-002.phpt new file mode 100644 index 00000000000..bc71fd4e411 --- /dev/null +++ b/ext/standard/tests/http/ghsa-52jp-hrpf-2jff-002.phpt @@ -0,0 +1,55 @@ +--TEST-- +GHSA-52jp-hrpf-2jff: HTTP stream wrapper truncate redirect location to 1024 bytes (over limit) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + $loc = str_repeat("y", 9000); + fwrite($conn, "HTTP/1.0 301 Ok\r\nContent-Type: text/html;\r\nLocation: $loc\r\n\r\nbody\r\n"); +CODE; + +$clientCode = <<<'CODE' + function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) { + switch($notification_code) { + case STREAM_NOTIFY_MIME_TYPE_IS: + echo "Found the mime-type: ", $message, PHP_EOL; + break; + case STREAM_NOTIFY_REDIRECTED: + echo "Redirected: "; + var_dump($message); + } + } + + $ctx = stream_context_create(); + stream_context_set_params($ctx, array("notification" => "stream_notification_callback")); + var_dump(trim(file_get_contents("http://{{ ADDR }}", false, $ctx))); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +Found the mime-type: text/html; + +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 +string(0) "" +array(2) { + [0]=> + string(15) "HTTP/1.0 301 Ok" + [1]=> + string(24) "Content-Type: text/html;" +} -- 2.48.1