Skip to content

Commit 91a07e7

Browse files
ramseyramsey
committed
Merge branch 'PHP-8.4'
2 parents c34b37f + d854a54 commit 91a07e7

File tree

7 files changed

+227
-31
lines changed

7 files changed

+227
-31
lines changed

‎main/rfc1867.c‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -742,6 +742,13 @@ SAPI_API SAPI_POST_HANDLER_FUNC(rfc1867_post_handler)
742742
boundary_len = boundary_end-boundary;
743743
}
744744

745+
/* Boundaries larger than FILLUNIT-strlen("\r\n--") characters lead to
746+
* erroneous parsing */
747+
if (boundary_len > FILLUNIT-strlen("\r\n--")) {
748+
sapi_module.sapi_error(E_WARNING, "Boundary too large in multipart/form-data POST data");
749+
return;
750+
}
751+
745752
/* Initialize the buffer */
746753
mbuff = multipart_buffer_new(boundary, boundary_len);
747754

‎sapi/cgi/cgi_main.c‎

Lines changed: 19 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1749,7 +1749,6 @@ int main(int argc, char *argv[])
17491749
int status = 0;
17501750
#endif
17511751
char *query_string;
1752-
char *decoded_query_string;
17531752
int skip_getopt = 0;
17541753

17551754
#if defined(SIGPIPE) && defined(SIG_IGN)
@@ -1804,10 +1803,15 @@ int main(int argc, char *argv[])
18041803
* the executable. Ideally we skip argument parsing when we're in cgi or fastcgi mode,
18051804
* but that breaks PHP scripts on Linux with a hashbang: `#!/php-cgi -d option=value`.
18061805
* Therefore, this code only prevents passing arguments if the query string starts with a '-'.
1807-
* Similarly, scripts spawned in subprocesses on Windows may have the same issue. */
1806+
* Similarly, scripts spawned in subprocesses on Windows may have the same issue.
1807+
* However, Windows has lots of conversion rules and command line parsing rules that
1808+
* are too difficult and dangerous to reliably emulate. */
18081809
if((query_string = getenv("QUERY_STRING")) != NULL && strchr(query_string, '=') == NULL) {
1810+
#ifdef PHP_WIN32
1811+
skip_getopt = cgi || fastcgi;
1812+
#else
18091813
unsigned char *p;
1810-
decoded_query_string = strdup(query_string);
1814+
char *decoded_query_string = strdup(query_string);
18111815
php_url_decode(decoded_query_string, strlen(decoded_query_string));
18121816
for (p = (unsigned char *)decoded_query_string; *p && *p <= ' '; p++) {
18131817
/* skip all leading spaces */
@@ -1816,22 +1820,8 @@ int main(int argc, char *argv[])
18161820
skip_getopt = 1;
18171821
}
18181822

1819-
/* On Windows we have to take into account the "best fit" mapping behaviour. */
1820-
#ifdef PHP_WIN32
1821-
if (*p >= 0x80) {
1822-
wchar_t wide_buf[1];
1823-
wide_buf[0] = *p;
1824-
char char_buf[4];
1825-
size_t wide_buf_len = sizeof(wide_buf) / sizeof(wide_buf[0]);
1826-
size_t char_buf_len = sizeof(char_buf) / sizeof(char_buf[0]);
1827-
if (WideCharToMultiByte(CP_ACP, 0, wide_buf, wide_buf_len, char_buf, char_buf_len, NULL, NULL) == 0
1828-
|| char_buf[0] == '-') {
1829-
skip_getopt = 1;
1830-
}
1831-
}
1832-
#endif
1833-
18341823
free(decoded_query_string);
1824+
#endif
18351825
}
18361826

18371827
php_ini_builder_init(&ini_builder);
@@ -1898,18 +1888,17 @@ int main(int argc, char *argv[])
18981888

18991889
/* check force_cgi after startup, so we have proper output */
19001890
if (cgi && CGIG(force_redirect)) {
1901-
/* Apache will generate REDIRECT_STATUS,
1902-
* Netscape and redirect.so will generate HTTP_REDIRECT_STATUS.
1903-
* redirect.so and installation instructions available from
1904-
* http://www.koehntopp.de/php.
1905-
* -- [email protected]
1906-
*/
1907-
if (!getenv("REDIRECT_STATUS") &&
1908-
!getenv ("HTTP_REDIRECT_STATUS") &&
1909-
/* this is to allow a different env var to be configured
1910-
* in case some server does something different than above */
1911-
(!CGIG(redirect_status_env) || !getenv(CGIG(redirect_status_env)))
1912-
) {
1891+
/* This is to allow a different environment variable to be configured
1892+
* in case the we cannot auto-detect which environment variable to use.
1893+
* Checking this first to allow user overrides in case the environment
1894+
* variable can be set by an untrusted party. */
1895+
const char *redirect_status_env = CGIG(redirect_status_env);
1896+
if (!redirect_status_env) {
1897+
/* Apache will generate REDIRECT_STATUS. */
1898+
redirect_status_env = "REDIRECT_STATUS";
1899+
}
1900+
1901+
if (!getenv(redirect_status_env)) {
19131902
zend_try {
19141903
SG(sapi_headers).http_response_code = 400;
19151904
PUTS("<b>Security Alert!</b> The PHP CGI cannot be accessed directly.\n\n\

‎sapi/fpm/fpm/fpm_stdio.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ static void fpm_stdio_child_said(struct fpm_event_s *ev, short which, void *arg)
228228
if ((sizeof(FPM_STDIO_CMD_FLUSH) - cmd_pos) <= in_buf &&
229229
!memcmp(buf, &FPM_STDIO_CMD_FLUSH[cmd_pos], sizeof(FPM_STDIO_CMD_FLUSH) - cmd_pos)) {
230230
zlog_stream_finish(log_stream);
231-
start = cmd_pos;
231+
start = sizeof(FPM_STDIO_CMD_FLUSH) - cmd_pos;
232232
} else {
233233
zlog_stream_str(log_stream, &FPM_STDIO_CMD_FLUSH[0], cmd_pos);
234234
}

‎sapi/fpm/tests/log-bwp-msg-flush-split-sep-pos-end.phpt‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
FPM: Buffered worker output plain log with msg with flush split position towards separator end
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$cfg = <<<EOT
11+
[global]
12+
error_log = {{FILE:LOG}}
13+
[unconfined]
14+
listen = {{ADDR}}
15+
pm = dynamic
16+
pm.max_children = 5
17+
pm.start_servers = 1
18+
pm.min_spare_servers = 1
19+
pm.max_spare_servers = 3
20+
catch_workers_output = yes
21+
decorate_workers_output = no
22+
EOT;
23+
24+
$code = <<<EOT
25+
<?php
26+
file_put_contents('php://stderr', str_repeat('a', 1013) . "Quarkslab\0fscf\0Quarkslab");
27+
EOT;
28+
29+
$tester = new FPM\Tester($cfg, $code);
30+
$tester->start();
31+
$tester->expectLogStartNotices();
32+
$tester->request()->expectEmptyBody();
33+
$tester->expectLogLine(str_repeat('a', 1013) . "Quarkslab", decorated: false);
34+
$tester->expectLogLine("Quarkslab", decorated: false);
35+
$tester->terminate();
36+
$tester->expectLogTerminatingNotices();
37+
$tester->close();
38+
39+
?>
40+
Done
41+
--EXPECT--
42+
Done
43+
--CLEAN--
44+
<?php
45+
require_once "tester.inc";
46+
FPM\Tester::clean();
47+
?>

‎sapi/fpm/tests/log-bwp-msg-flush-split-sep-pos-start.phpt‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
FPM: Buffered worker output plain log with msg with flush split position towards separator start
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$cfg = <<<EOT
11+
[global]
12+
error_log = {{FILE:LOG}}
13+
[unconfined]
14+
listen = {{ADDR}}
15+
pm = dynamic
16+
pm.max_children = 5
17+
pm.start_servers = 1
18+
pm.min_spare_servers = 1
19+
pm.max_spare_servers = 3
20+
catch_workers_output = yes
21+
decorate_workers_output = no
22+
EOT;
23+
24+
$code = <<<EOT
25+
<?php
26+
file_put_contents('php://stderr', str_repeat('a', 1009) . "Quarkslab\0fscf\0Quarkslab");
27+
EOT;
28+
29+
$tester = new FPM\Tester($cfg, $code);
30+
$tester->start();
31+
$tester->expectLogStartNotices();
32+
$tester->request()->expectEmptyBody();
33+
$tester->expectLogLine(str_repeat('a', 1009) . "Quarkslab", decorated: false);
34+
$tester->expectLogLine("Quarkslab", decorated: false);
35+
$tester->terminate();
36+
$tester->expectLogTerminatingNotices();
37+
$tester->close();
38+
39+
?>
40+
Done
41+
--EXPECT--
42+
Done
43+
--CLEAN--
44+
<?php
45+
require_once "tester.inc";
46+
FPM\Tester::clean();
47+
?>

‎tests/basic/GHSA-9pqp-7h25-4f32.inc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<?php
2+
print "Hello world\n";
3+
var_dump($_POST);

‎tests/basic/GHSA-9pqp-7h25-4f32.phpt‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
--TEST--
2+
GHSA-9pqp-7h25-4f32
3+
--SKIPIF--
4+
<?php
5+
if (!getenv('TEST_PHP_CGI_EXECUTABLE')) {
6+
die("skip php-cgi not available");
7+
}
8+
if (substr(PHP_OS, 0, 3) == 'WIN') {
9+
die("skip not for Windows in CI - probably resource issue");
10+
}
11+
?>
12+
--FILE--
13+
<?php
14+
15+
const FILLUNIT = 5 * 1024;
16+
17+
function test($boundaryLen) {
18+
printf("Boundary len: %d\n", $boundaryLen);
19+
20+
$cmd = [
21+
getenv('TEST_PHP_CGI_EXECUTABLE'),
22+
'-C',
23+
'-n',
24+
__DIR__ . '/GHSA-9pqp-7h25-4f32.inc',
25+
];
26+
27+
$boundary = str_repeat('A', $boundaryLen);
28+
$body = ""
29+
. "--$boundary\r\n"
30+
. "Content-Disposition: form-data; name=\"koko\"\r\n"
31+
. "\r\n"
32+
. "BBB\r\n--" . substr($boundary, 0, -1) . "CCC\r\n"
33+
. "--$boundary--\r\n"
34+
;
35+
36+
$env = array_merge($_ENV, [
37+
'REDIRECT_STATUS' => '1',
38+
'CONTENT_TYPE' => "multipart/form-data; boundary=$boundary",
39+
'CONTENT_LENGTH' => strlen($body),
40+
'REQUEST_METHOD' => 'POST',
41+
'SCRIPT_FILENAME' => __DIR__ . '/GHSA-9pqp-7h25-4f32.inc',
42+
]);
43+
44+
$spec = [
45+
0 => ['pipe', 'r'],
46+
1 => STDOUT,
47+
2 => STDOUT,
48+
];
49+
50+
$pipes = [];
51+
52+
print "Starting...\n";
53+
54+
$handle = proc_open($cmd, $spec, $pipes, getcwd(), $env);
55+
56+
fwrite($pipes[0], $body);
57+
58+
$status = proc_close($handle);
59+
60+
print "\n";
61+
}
62+
63+
for ($offset = -1; $offset <= 1; $offset++) {
64+
test(FILLUNIT - strlen("\r\n--") + $offset);
65+
}
66+
67+
?>
68+
--EXPECTF--
69+
Boundary len: 5115
70+
Starting...
71+
X-Powered-By: %s
72+
Content-type: text/html; charset=UTF-8
73+
74+
Hello world
75+
array(1) {
76+
["koko"]=>
77+
string(5124) "BBB
78+
--AAA%sCCC"
79+
}
80+
81+
Boundary len: 5116
82+
Starting...
83+
X-Powered-By: %s
84+
Content-type: text/html; charset=UTF-8
85+
86+
Hello world
87+
array(1) {
88+
["koko"]=>
89+
string(5125) "BBB
90+
--AAA%sCCC"
91+
}
92+
93+
Boundary len: 5117
94+
Starting...
95+
X-Powered-By: %s
96+
Content-type: text/html; charset=UTF-8
97+
98+
<br />
99+
<b>Warning</b>: Boundary too large in multipart/form-data POST data in <b>Unknown</b> on line <b>0</b><br />
100+
Hello world
101+
array(0) {
102+
}
103+

0 commit comments

Comments
 (0)