Skip to content

Commit b6d1d39

Browse files
ndosscheericmann
authored andcommitted
The original code is error-prone due to the "best fit mapping" that happens with the argument parsing but not with the query string. When we get a non-ASCII character, try to remap it and see if it becomes a hyphen. An alternative approach is to create a custom main `wmain` receiving wide-character variations that does the ANSI transformation with the best-fit mapping, but that's more error-prone and could cause unexpected breakage. Another alternative was just don't doing this check altogether and always check for `cgi || fastcgi` instead, but that breaks real-world use-cases.
1 parent e6a82ad commit b6d1d39

File tree

2 files changed

+60
-1
lines changed

2 files changed

+60
-1
lines changed

sapi/cgi/cgi_main.c

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1796,8 +1796,13 @@ int main(int argc, char *argv[])
17961796
}
17971797
}
17981798

1799+
/* Apache CGI will pass the query string to the command line if it doesn't contain a '='.
1800+
* This can create an issue where a malicious request can pass command line arguments to
1801+
* the executable. Ideally we skip argument parsing when we're in cgi or fastcgi mode,
1802+
* but that breaks PHP scripts on Linux with a hashbang: `#!/php-cgi -d option=value`.
1803+
* Therefore, this code only prevents passing arguments if the query string starts with a '-'.
1804+
* Similarly, scripts spawned in subprocesses on Windows may have the same issue. */
17991805
if((query_string = getenv("QUERY_STRING")) != NULL && strchr(query_string, '=') == NULL) {
1800-
/* we've got query string that has no = - apache CGI will pass it to command line */
18011806
unsigned char *p;
18021807
decoded_query_string = strdup(query_string);
18031808
php_url_decode(decoded_query_string, strlen(decoded_query_string));
@@ -1807,6 +1812,22 @@ int main(int argc, char *argv[])
18071812
if(*p == '-') {
18081813
skip_getopt = 1;
18091814
}
1815+
1816+
/* On Windows we have to take into account the "best fit" mapping behaviour. */
1817+
#ifdef PHP_WIN32
1818+
if (*p >= 0x80) {
1819+
wchar_t wide_buf[1];
1820+
wide_buf[0] = *p;
1821+
char char_buf[4];
1822+
size_t wide_buf_len = sizeof(wide_buf) / sizeof(wide_buf[0]);
1823+
size_t char_buf_len = sizeof(char_buf) / sizeof(char_buf[0]);
1824+
if (WideCharToMultiByte(CP_ACP, 0, wide_buf, wide_buf_len, char_buf, char_buf_len, NULL, NULL) == 0
1825+
|| char_buf[0] == '-') {
1826+
skip_getopt = 1;
1827+
}
1828+
}
1829+
#endif
1830+
18101831
free(decoded_query_string);
18111832
}
18121833

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
GHSA-3qgc-jrrr-25jv
3+
--SKIPIF--
4+
<?php
5+
include 'skipif.inc';
6+
if (PHP_OS_FAMILY !== "Windows") die("skip Only for Windows");
7+
8+
$codepage = trim(shell_exec("powershell Get-ItemPropertyValue HKLM:\\SYSTEM\\CurrentControlSet\\Control\\Nls\\CodePage ACP"));
9+
if ($codepage !== '932' && $codepage !== '936' && $codepage !== '950') die("skip Wrong codepage");
10+
?>
11+
--FILE--
12+
<?php
13+
include 'include.inc';
14+
15+
$filename = __DIR__."/GHSA-3qgc-jrrr-25jv_tmp.php";
16+
$script = '<?php echo "hello "; echo "world"; ?>';
17+
file_put_contents($filename, $script);
18+
19+
$php = get_cgi_path();
20+
reset_env_vars();
21+
22+
putenv("SERVER_NAME=Test");
23+
putenv("SCRIPT_FILENAME=$filename");
24+
putenv("QUERY_STRING=%ads");
25+
putenv("REDIRECT_STATUS=1");
26+
27+
passthru("$php -s");
28+
29+
?>
30+
--CLEAN--
31+
<?php
32+
@unlink(__DIR__."/GHSA-3qgc-jrrr-25jv_tmp.php");
33+
?>
34+
--EXPECTF--
35+
X-Powered-By: PHP/%s
36+
Content-type: %s
37+
38+
hello world

0 commit comments

Comments
 (0)