Skip to content

Commit b4dda02

Browse files
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.
1 parent b6130dd commit b4dda02

File tree

2 files changed

+66
-5
lines changed

2 files changed

+66
-5
lines changed

sapi/cli/php_cli_server.c

Lines changed: 23 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 (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,32 @@ 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 (client->request.query_string == NULL) {
1646+
client->request.query_string = pestrndup(at, length, 1);
1647+
client->request.query_string_len = length;
1648+
} else {
1649+
client->request.query_string = perealloc(client->request.query_string, client->request.query_string_len + length + 1, 1);
1650+
memcpy(client->request.query_string + client->request.query_string_len, at, length);
1651+
client->request.query_string_len += length;
1652+
client->request.query_string[client->request.query_string_len] = '\0';
1653+
}
16441654
return 0;
16451655
}
16461656

16471657
static int php_cli_server_client_read_request_on_url(php_http_parser *parser, const char *at, size_t length)
16481658
{
16491659
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;
1660+
if (client->request.request_uri == NULL) {
1661+
client->request.request_method = parser->method;
1662+
client->request.request_uri = pestrndup(at, length, 1);
1663+
client->request.request_uri_len = length;
1664+
} else {
1665+
ZEND_ASSERT(client->request.request_method == parser->method);
1666+
client->request.request_uri = perealloc(client->request.request_uri, client->request.request_uri_len + length + 1, 1);
1667+
memcpy(client->request.request_uri + client->request.request_uri_len, at, length);
1668+
client->request.request_uri_len += length;
1669+
client->request.request_uri[client->request.request_uri_len] = '\0';
1670+
}
16531671
return 0;
16541672
}
16551673

sapi/cli/tests/bug73630.phpt

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
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+
EOF;
13+
14+
include "php_cli_server.inc";
15+
php_cli_server_start($code);
16+
17+
$host = PHP_CLI_SERVER_HOSTNAME;
18+
$fp = php_cli_server_connect();
19+
20+
$path = "/overflow.php?" . str_repeat("x", 16400) . "//example.com";
21+
22+
if (fwrite($fp, <<<HEADER
23+
GET $path HTTP/1.1
24+
Host: {$host}
25+
26+
27+
HEADER
28+
)) {
29+
while (!feof($fp)) {
30+
echo fgets($fp);
31+
}
32+
}
33+
34+
?>
35+
--EXPECTF--
36+
HTTP/1.1 200 OK
37+
Host: %s
38+
Date: %s
39+
Connection: close
40+
X-Powered-By: PHP/%s
41+
Content-type: text/html; charset=UTF-8
42+
43+
int(0)

0 commit comments

Comments
 (0)