From fb718aa6f2117933566bb7bb2f70b2b0d9a9c08f Mon Sep 17 00:00:00 2001
From: Jan Ehrhardt <github@ehrhardt.nl>
Date: Wed, 5 Jun 2024 20:24:52 +0200
Subject: [PATCH 01/11] 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 a36f426d266..8d1342727dc 100644
--- a/sapi/cgi/cgi_main.c
+++ b/sapi/cgi/cgi_main.c
@@ -1827,8 +1827,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));
@@ -1838,6 +1843,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--
+<?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
--
2.46.1
From a634d3f5169c884715d9e26ac213ecf2a25c3666 Mon Sep 17 00:00:00 2001
From: Jan Ehrhardt <github@ehrhardt.nl>
Date: Sun, 9 Jun 2024 20:09:02 +0200
Subject: [PATCH 03/11] NEWS: Add backports from 8.1.29
---
NEWS | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/NEWS b/NEWS
index 34ad33cf5c4..a96518695fb 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 1158d06f0b20532ab7309cb20f0be843f9662e3c 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 05/11] 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)
(cherry picked from commit 2d2552e092b6ff32cd823692d512f126ee629842)
---
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 8d1342727dc..a2761aafd7b 100644
--- a/sapi/cgi/cgi_main.c
+++ b/sapi/cgi/cgi_main.c
@@ -1777,7 +1777,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)
@@ -1832,10 +1831,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 */
@@ -1844,22 +1848,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