archive/php-src
1
0
Fork 0
mirror of https://github.com/php/php-src.git synced 2025-08-16 05:58:45 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Fix GHSA-3qgc-jrrr-25jv

Browse source 
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.
This commit is contained in:
Niels Dossche 2024-05-17 21:51:30 +02:00 • committed by Ben Ramsey
parent c8b36406c0
commit 4dd9a36c16
No known key found for this signature in database
GPG key ID: F9C39DC0B9698544
2 changed files with 60 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

23
sapi/cgi/cgi_main.c
View file

@ -1798,8 +1798,13 @@ int main(int argc, char *argv[])
} }
} }
/* Apache CGI will pass the query string to the command line if it doesn't contain a '='.
* This can create an issue where a malicious request can pass command line arguments to
* the executable. Ideally we skip argument parsing when we're in cgi or fastcgi mode,
* but that breaks PHP scripts on Linux with a hashbang: `#!/php-cgi -d option=value`.
* Therefore, this code only prevents passing arguments if the query string starts with a '-'.
* Similarly, scripts spawned in subprocesses on Windows may have the same issue. */
if((query_string = getenv("QUERY_STRING")) != NULL && strchr(query_string, '=') == NULL) { if((query_string = getenv("QUERY_STRING")) != NULL && strchr(query_string, '=') == NULL) {
/* we've got query string that has no = - apache CGI will pass it to command line */
unsigned char *p; unsigned char *p;
decoded_query_string = strdup(query_string); decoded_query_string = strdup(query_string);
php_url_decode(decoded_query_string, strlen(decoded_query_string)); php_url_decode(decoded_query_string, strlen(decoded_query_string));
@ -1809,6 +1814,22 @@ int main(int argc, char *argv[])
if(*p == '-') { if(*p == '-') {
skip_getopt = 1; skip_getopt = 1;
} }
/* On Windows we have to take into account the "best fit" mapping behaviour. */
#ifdef PHP_WIN32
if (*p >= 0x80) {
wchar_t wide_buf[1];
wide_buf[0] = *p;
char char_buf[4];
size_t wide_buf_len = sizeof(wide_buf) / sizeof(wide_buf[0]);
size_t char_buf_len = sizeof(char_buf) / sizeof(char_buf[0]);
if (WideCharToMultiByte(CP_ACP, 0, wide_buf, wide_buf_len, char_buf, char_buf_len, NULL, NULL) == 0
|| char_buf[0] == '-') {
skip_getopt = 1;
}
}
#endif
free(decoded_query_string); free(decoded_query_string);
} }

38
sapi/cgi/tests/ghsa-3qgc-jrrr-25jv.phpt Normal file
View file

@ -0,0 +1,38 @@
--TEST--
GHSA-3qgc-jrrr-25jv
--SKIPIF--
<?php
include 'skipif.inc';
if (PHP_OS_FAMILY !== "Windows") die("skip Only for Windows");
$codepage = trim(shell_exec("powershell Get-ItemPropertyValue HKLM:\\SYSTEM\\CurrentControlSet\\Control\\Nls\\CodePage ACP"));
if ($codepage !== '932' && $codepage !== '936' && $codepage !== '950') die("skip Wrong codepage");
?>
--FILE--
<?php
include 'include.inc';
$filename = __DIR__."/GHSA-3qgc-jrrr-25jv_tmp.php";
$script = '<?php echo "hello "; echo "world"; ?>';
file_put_contents($filename, $script);
$php = get_cgi_path();
reset_env_vars();
putenv("SERVER_NAME=Test");
putenv("SCRIPT_FILENAME=$filename");
putenv("QUERY_STRING=%ads");
putenv("REDIRECT_STATUS=1");
passthru("$php -s");
?>
--CLEAN--
<?php
@unlink(__DIR__."/GHSA-3qgc-jrrr-25jv_tmp.php");
?>
--EXPECTF--
X-Powered-By: PHP/%s
Content-type: %s
hello world