From 9f95e17cc0a9a79da82157e34e3effe1bc395037 Mon Sep 17 00:00:00 2001 From: Jan Ehrhardt Date: Wed, 5 Jun 2024 20:44:46 +0200 Subject: [PATCH 1/8] Fix GHSA-3qgc-jrrr-25jv --- sapi/cgi/cgi_main.c | 23 ++++++++++++++- sapi/cgi/tests/ghsa-3qgc-jrrr-25jv.phpt | 38 +++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 sapi/cgi/tests/ghsa-3qgc-jrrr-25jv.phpt diff --git a/sapi/cgi/cgi_main.c b/sapi/cgi/cgi_main.c index 0d52941c5a1..0d3b54ed8b8 100644 --- a/sapi/cgi/cgi_main.c +++ b/sapi/cgi/cgi_main.c @@ -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) { - /* we've got query string that has no = - apache CGI will pass it to command line */ unsigned char *p; decoded_query_string = strdup(query_string); php_url_decode(decoded_query_string, strlen(decoded_query_string)); @@ -1809,6 +1814,22 @@ int main(int argc, char *argv[]) if(*p == '-') { 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); } diff --git a/sapi/cgi/tests/ghsa-3qgc-jrrr-25jv.phpt b/sapi/cgi/tests/ghsa-3qgc-jrrr-25jv.phpt new file mode 100644 index 00000000000..fd2fcdfbf89 --- /dev/null +++ b/sapi/cgi/tests/ghsa-3qgc-jrrr-25jv.phpt @@ -0,0 +1,38 @@ +--TEST-- +GHSA-3qgc-jrrr-25jv +--SKIPIF-- + +--FILE-- +'; +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-- + +--EXPECTF-- +X-Powered-By: PHP/%s +Content-type: %s + +hello world -- 2.46.1 From dc40d2d7960dd35f0178ff52c1f8590b7b1a08b2 Mon Sep 17 00:00:00 2001 From: Jan Ehrhardt Date: Sun, 9 Jun 2024 20:10:36 +0200 Subject: [PATCH 2/8] NEWS: Add backports from 8.1.29 --- NEWS | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/NEWS b/NEWS index 7a9b6bdae18..79133f558af 100644 --- a/NEWS +++ b/NEWS @@ -3,10 +3,18 @@ PHP NEWS Backported from 8.1.29 +- CGI: + . Fixed bug GHSA-3qgc-jrrr-25jv (Bypass of CVE-2012-1823, Argument Injection + in PHP-CGI). (CVE-2024-4577) (nielsdos) + - Filter: . Fixed bug GHSA-w8qr-v226-r27w (Filter bypass in filter_var FILTER_VALIDATE_URL). (CVE-2024-5458) (nielsdos) +- Standard: + . Fixed bug GHSA-9fcc-425m-g385 (Bypass of CVE-2024-1874). + (CVE-2024-5585) (nielsdos) + Backported from 8.1.28 - Standard: -- 2.46.1 From 2d2552e092b6ff32cd823692d512f126ee629842 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 14 Jun 2024 19:49:22 +0200 Subject: [PATCH 4/8] Fix GHSA-p99j-rfp4-xqvq It's no use trying to work around whatever the operating system and Apache do because we'll be fighting that until eternity. Change the skip_getopt condition such that when we're running in CGI or FastCGI mode we always skip the argument parsing. This is a BC break, but this seems to be the only way to get rid of this class of issues. (cherry picked from commit abcfd980bfa03298792fd3aba051c78d52f10642) --- sapi/cgi/cgi_main.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/sapi/cgi/cgi_main.c b/sapi/cgi/cgi_main.c index 0d3b54ed8b8..6e148874e4f 100644 --- a/sapi/cgi/cgi_main.c +++ b/sapi/cgi/cgi_main.c @@ -1748,7 +1748,6 @@ int main(int argc, char *argv[]) int status = 0; #endif char *query_string; - char *decoded_query_string; int skip_getopt = 0; #if defined(SIGPIPE) && defined(SIG_IGN) @@ -1803,10 +1802,15 @@ int main(int argc, char *argv[]) * 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. */ + * Similarly, scripts spawned in subprocesses on Windows may have the same issue. + * However, Windows has lots of conversion rules and command line parsing rules that + * are too difficult and dangerous to reliably emulate. */ if((query_string = getenv("QUERY_STRING")) != NULL && strchr(query_string, '=') == NULL) { +#ifdef PHP_WIN32 + skip_getopt = cgi || fastcgi; +#else unsigned char *p; - decoded_query_string = strdup(query_string); + char *decoded_query_string = strdup(query_string); php_url_decode(decoded_query_string, strlen(decoded_query_string)); for (p = (unsigned char *)decoded_query_string; *p && *p <= ' '; p++) { /* skip all leading spaces */ @@ -1815,22 +1819,8 @@ int main(int argc, char *argv[]) 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); +#endif } while (!skip_getopt && (c = php_getopt(argc, argv, OPTIONS, &php_optarg, &php_optind, 0, 2)) != -1) { -- 2.46.1