Skip to content

Commit d7db570

Browse files
cmb69cmb69
committed
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.
1 parent 98a21d1 commit d7db570

File tree

4 files changed

+97
-5
lines changed

4 files changed

+97
-5
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ PHP NEWS
99
(krakjoe)
1010
. Fixed bug #80728 (PHP built-in web server resets timeout when it can kill
1111
the process). (Calvin Buckley)
12+
. Fixed bug #73630 (Built-in Weberver - overwrite $_SERVER['request_uri']).
13+
(cmb)
1214

1315
- Intl:
1416
. Fixed bug #72809 (Locale::lookup() wrong result with canonicalize option).

‎sapi/cli/php_cli_server.c

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1629,6 +1629,9 @@ static int php_cli_server_client_read_request_on_path(php_http_parser *parser, c
16291629
{
16301630
char *vpath;
16311631
size_t vpath_len;
1632+
if (UNEXPECTED(client->request.vpath != NULL)) {
1633+
return 1;
1634+
}
16321635
normalize_vpath(&vpath, &vpath_len, at, length, 1);
16331636
client->request.vpath = vpath;
16341637
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
16391642
static int php_cli_server_client_read_request_on_query_string(php_http_parser *parser, const char *at, size_t length)
16401643
{
16411644
php_cli_server_client *client = parser->data;
1642-
client->request.query_string = pestrndup(at, length, 1);
1643-
client->request.query_string_len = length;
1645+
if (EXPECTED(client->request.query_string == NULL)) {
1646+
client->request.query_string = pestrndup(at, length, 1);
1647+
client->request.query_string_len = length;
1648+
} else {
1649+
ZEND_ASSERT(length <= PHP_HTTP_MAX_HEADER_SIZE && PHP_HTTP_MAX_HEADER_SIZE - length >= client->request.query_string_len);
1650+
client->request.query_string = perealloc(client->request.query_string, client->request.query_string_len + length + 1, 1);
1651+
memcpy(client->request.query_string + client->request.query_string_len, at, length);
1652+
client->request.query_string_len += length;
1653+
client->request.query_string[client->request.query_string_len] = '\0';
1654+
}
16441655
return 0;
16451656
}
16461657

16471658
static int php_cli_server_client_read_request_on_url(php_http_parser *parser, const char *at, size_t length)
16481659
{
16491660
php_cli_server_client *client = parser->data;
1650-
client->request.request_method = parser->method;
1651-
client->request.request_uri = pestrndup(at, length, 1);
1652-
client->request.request_uri_len = length;
1661+
if (EXPECTED(client->request.request_uri == NULL)) {
1662+
client->request.request_method = parser->method;
1663+
client->request.request_uri = pestrndup(at, length, 1);
1664+
client->request.request_uri_len = length;
1665+
} else {
1666+
ZEND_ASSERT(client->request.request_method == parser->method);
1667+
ZEND_ASSERT(length <= PHP_HTTP_MAX_HEADER_SIZE && PHP_HTTP_MAX_HEADER_SIZE - length >= client->request.query_string_len);
1668+
client->request.request_uri = perealloc(client->request.request_uri, client->request.request_uri_len + length + 1, 1);
1669+
memcpy(client->request.request_uri + client->request.request_uri_len, at, length);
1670+
client->request.request_uri_len += length;
1671+
client->request.request_uri[client->request.request_uri_len] = '\0';
1672+
}
16531673
return 0;
16541674
}
16551675

‎sapi/cli/tests/bug73630.phpt

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
--TEST--
2+
Bug #73630 (Built-in Weberver - overwrite $_SERVER['request_uri'])
3+
--SKIPIF--
4+
<?php
5+
include "skipif.inc";
6+
?>
7+
--FILE--
8+
<?php
9+
10+
$code = <<<'EOF'
11+
var_dump(strncmp($_SERVER['REQUEST_URI'], "/overflow.php", strlen("/overflow.php")));
12+
var_dump(strlen($_SERVER['QUERY_STRING']));
13+
EOF;
14+
15+
include "php_cli_server.inc";
16+
php_cli_server_start($code);
17+
18+
$host = PHP_CLI_SERVER_HOSTNAME;
19+
$fp = php_cli_server_connect();
20+
21+
$path = "/overflow.php?" . str_repeat("x", 16400) . "//example.com";
22+
23+
if (fwrite($fp, <<<HEADER
24+
GET $path HTTP/1.1
25+
Host: {$host}
26+
27+
28+
HEADER
29+
)) {
30+
while (!feof($fp)) {
31+
echo fgets($fp);
32+
}
33+
}
34+
35+
?>
36+
--EXPECTF--
37+
HTTP/1.1 200 OK
38+
Host: %s
39+
Date: %s
40+
Connection: close
41+
X-Powered-By: PHP/%s
42+
Content-type: text/html; charset=UTF-8
43+
44+
int(0)
45+
int(16413)

‎sapi/cli/tests/bug73630a.phpt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
Bug #73630 (Built-in Weberver - overwrite $_SERVER['request_uri'])
3+
--DESCRIPTION--
4+
Check that too long paths result in invalid request
5+
--SKIPIF--
6+
<?php
7+
include "skipif.inc";
8+
?>
9+
--FILE--
10+
<?php
11+
$code = <<<'EOF'
12+
echo "won't happen\n";
13+
EOF;
14+
15+
include "php_cli_server.inc";
16+
php_cli_server_start($code);
17+
18+
$host = PHP_CLI_SERVER_HOSTNAME;
19+
$fp = php_cli_server_connect();
20+
$path = "/" . str_repeat("x", 16400) . "//example.com";
21+
var_dump(file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS . "$path"));
22+
?>
23+
--EXPECTF--
24+
Warning: file_get_contents(http://%s//example.com): failed to open stream: HTTP request failed! in %s on line %d
25+
bool(false)

0 commit comments

Comments
 (0)