From affd824a2c137189c5a92a1ab2e47cb282e0f323 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 9 Feb 2021 11:39:05 +0100 Subject: [PATCH] Fix #78719: http wrapper silently ignores long Location headers Currently, the HTTP wrapper reads and processes header lines of at most 1024 bytes (including the trailing line break). This appears to be overly restrictive, particularly with regard to Location headers, which may contain long URIs. There is no standard regarding the maximum length of an URI, but assuming about 2000 bytes appears to be prudent. We therefore increase the size of the buffer to 2048. That still does not properly cater to header lines which are even longer. According to RFC 7230, section 3.2.5, it is okay to discard long header lines, only if that does not change the "message framing or response semantics". So, e.g. an overlong Location header must not be discarded if `follow_location` is set (the default), and probably should not even if `follow_location` is not set. We therefore let the attempt to open the stream fail in this case. --- ext/standard/http_fopen_wrapper.c | 12 ++++++++-- ext/standard/tests/http/bug78719.phpt | 33 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 ext/standard/tests/http/bug78719.phpt diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index bf0363fd3cca4..d6d98fee76276 100644 --- a/ext/standard/http_fopen_wrapper.c +++ b/ext/standard/http_fopen_wrapper.c @@ -69,7 +69,7 @@ #include "php_fopen_wrappers.h" -#define HTTP_HEADER_BLOCK_SIZE 1024 +#define HTTP_HEADER_BLOCK_SIZE 2024 #define PHP_URL_REDIRECT_MAX 20 #define HTTP_HEADER_USER_AGENT 1 #define HTTP_HEADER_HOST 2 @@ -741,7 +741,15 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, char *e = http_header_line + http_header_line_length - 1; char *http_header_value; if (*e != '\n') { - do { /* partial header */ + /* fail on overlong Location header */ + if (!strncasecmp(http_header_line, "Location:", sizeof("Location:")-1)) { + php_stream_close(stream); + stream = NULL; + php_stream_wrapper_log_error(wrapper, options, "Location header too long"); + goto out; + } + /* silently discard other overlong headers */ + do { if (php_stream_get_line(stream, http_header_line, HTTP_HEADER_BLOCK_SIZE, &http_header_line_length) == NULL) { php_stream_wrapper_log_error(wrapper, options, "Failed to read HTTP headers"); goto out; diff --git a/ext/standard/tests/http/bug78719.phpt b/ext/standard/tests/http/bug78719.phpt new file mode 100644 index 0000000000000..e5e3362cb2ac0 --- /dev/null +++ b/ext/standard/tests/http/bug78719.phpt @@ -0,0 +1,33 @@ +--TEST-- +Bug #78719 (http wrapper silently ignores long Location headers) +--SKIPIF-- + +--INI-- +allow_url_fopen=1 +--FILE-- + ['follow_location' => 0]]); + $f = file_get_contents('http://127.0.0.1:12342/', false, $context); + var_dump($f); +} +test(); +test(); + +http_server_kill($pid); +?> +--EXPECTF-- +string(4) "Body" + +Warning: file_get_contents(http://127.0.0.1:12342/): failed to open stream: Location header too long in %s on line %d +bool(false)