From d7db5701a30f0e678f379a05360f8c91f89868ac Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 30 Jun 2021 13:46:43 +0200 Subject: [PATCH] Fix #73630: Built-in Weberver - overwrite $_SERVER['request_uri'] The built-in Webserver's `on_path`, `on_query_string` and `on_url` callbacks may be called multiple times from the parser; we must not simply replace the old values, but need to concatenate the new values instead. This appears to be tricky for `on_path` due to the path normalization, so we fail if the function is called again. The built-in Webserver logs errors during request parsing to stderr, but this is ignored by the php_cli_server framework, and apparently the Webserver does not send a resonse at all in such cases (instead of an 4xx). Thus we can only check that a request with an overly long path fails. Closes GH-7207. --- NEWS | 2 ++ sapi/cli/php_cli_server.c | 30 +++++++++++++++++++---- sapi/cli/tests/bug73630.phpt | 45 +++++++++++++++++++++++++++++++++++ sapi/cli/tests/bug73630a.phpt | 25 +++++++++++++++++++ 4 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 sapi/cli/tests/bug73630.phpt create mode 100644 sapi/cli/tests/bug73630a.phpt diff --git a/NEWS b/NEWS index 09a14fa6cec..22248a11ad8 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,8 @@ PHP NEWS (krakjoe) . Fixed bug #80728 (PHP built-in web server resets timeout when it can kill the process). (Calvin Buckley) + . Fixed bug #73630 (Built-in Weberver - overwrite $_SERVER['request_uri']). + (cmb) - Intl: . Fixed bug #72809 (Locale::lookup() wrong result with canonicalize option). diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c index befa4c65f5d..c3097861e3f 100644 --- a/sapi/cli/php_cli_server.c +++ b/sapi/cli/php_cli_server.c @@ -1629,6 +1629,9 @@ static int php_cli_server_client_read_request_on_path(php_http_parser *parser, c { char *vpath; size_t vpath_len; + if (UNEXPECTED(client->request.vpath != NULL)) { + return 1; + } normalize_vpath(&vpath, &vpath_len, at, length, 1); client->request.vpath = vpath; client->request.vpath_len = vpath_len; @@ -1639,17 +1642,34 @@ static int php_cli_server_client_read_request_on_path(php_http_parser *parser, c static int php_cli_server_client_read_request_on_query_string(php_http_parser *parser, const char *at, size_t length) { php_cli_server_client *client = parser->data; - client->request.query_string = pestrndup(at, length, 1); - client->request.query_string_len = length; + if (EXPECTED(client->request.query_string == NULL)) { + client->request.query_string = pestrndup(at, length, 1); + client->request.query_string_len = length; + } else { + ZEND_ASSERT(length <= PHP_HTTP_MAX_HEADER_SIZE && PHP_HTTP_MAX_HEADER_SIZE - length >= client->request.query_string_len); + client->request.query_string = perealloc(client->request.query_string, client->request.query_string_len + length + 1, 1); + memcpy(client->request.query_string + client->request.query_string_len, at, length); + client->request.query_string_len += length; + client->request.query_string[client->request.query_string_len] = '\0'; + } return 0; } static int php_cli_server_client_read_request_on_url(php_http_parser *parser, const char *at, size_t length) { php_cli_server_client *client = parser->data; - client->request.request_method = parser->method; - client->request.request_uri = pestrndup(at, length, 1); - client->request.request_uri_len = length; + if (EXPECTED(client->request.request_uri == NULL)) { + client->request.request_method = parser->method; + client->request.request_uri = pestrndup(at, length, 1); + client->request.request_uri_len = length; + } else { + ZEND_ASSERT(client->request.request_method == parser->method); + ZEND_ASSERT(length <= PHP_HTTP_MAX_HEADER_SIZE && PHP_HTTP_MAX_HEADER_SIZE - length >= client->request.query_string_len); + client->request.request_uri = perealloc(client->request.request_uri, client->request.request_uri_len + length + 1, 1); + memcpy(client->request.request_uri + client->request.request_uri_len, at, length); + client->request.request_uri_len += length; + client->request.request_uri[client->request.request_uri_len] = '\0'; + } return 0; } diff --git a/sapi/cli/tests/bug73630.phpt b/sapi/cli/tests/bug73630.phpt new file mode 100644 index 00000000000..431b323feb2 --- /dev/null +++ b/sapi/cli/tests/bug73630.phpt @@ -0,0 +1,45 @@ +--TEST-- +Bug #73630 (Built-in Weberver - overwrite $_SERVER['request_uri']) +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +HTTP/1.1 200 OK +Host: %s +Date: %s +Connection: close +X-Powered-By: PHP/%s +Content-type: text/html; charset=UTF-8 + +int(0) +int(16413) diff --git a/sapi/cli/tests/bug73630a.phpt b/sapi/cli/tests/bug73630a.phpt new file mode 100644 index 00000000000..2ad9829510f --- /dev/null +++ b/sapi/cli/tests/bug73630a.phpt @@ -0,0 +1,25 @@ +--TEST-- +Bug #73630 (Built-in Weberver - overwrite $_SERVER['request_uri']) +--DESCRIPTION-- +Check that too long paths result in invalid request +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: file_get_contents(http://%s//example.com): failed to open stream: HTTP request failed! in %s on line %d +bool(false)