From e3a8b4e1e69f2970e0ad080ad90069588698d2d0 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Fri, 16 Oct 2026 18:55:52 +0000 Subject: [PATCH] mod_cgid: Raise the limit on the number of environment variables passed to a script from 256 to APACHE_ARG_MAX, and bound URI/query/env-value lengths against the configured request limits rather than APR_PATH_MAX. PR 70270. * changes-entries/cgid-env-limit.txt: Add changelog entry. * modules/generators/mod_cgid.c: Replace ENV_COUNT_MAX with ENV_VAL_MAX, a sanity bound on the length of a single environment variable value. (get_req): Take the main server_rec as an argument. Allow up to APACHE_ARG_MAX environment variables; check uri_len and args_len against the configured LimitRequestLine rather than APR_PATH_MAX; check each environment variable value against ENV_VAL_MAX. (send_req): Fail with APR_ENOSPC if more than APACHE_ARG_MAX environment variables are present. (cgid_server): Pass main_server to get_req. * test/modules/generators/htdocs/cgi/: New tests scripts. * test/modules/generators/test_001_cgid.py: New tests. Submitted by: Emmanuel Dreyfus , jorton Co-Authored-By: Claude Fable 5 GitHub: closes #850 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1939165 13f79535-47bb-0310-9956-ffa450edef68 --- changes-entries/cgid-env-limit.txt | 3 ++ modules/generators/mod_cgid.c | 21 ++++++++---- test/modules/generators/htdocs/cgi/arglen.py | 5 +++ .../modules/generators/htdocs/cgi/envcount.py | 5 +++ test/modules/generators/test_001_cgid.py | 32 ++++++++++++++++++- 5 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 changes-entries/cgid-env-limit.txt create mode 100644 test/modules/generators/htdocs/cgi/arglen.py create mode 100644 test/modules/generators/htdocs/cgi/envcount.py diff --git a/changes-entries/cgid-env-limit.txt b/changes-entries/cgid-env-limit.txt new file mode 100644 index 0000000000..47fb96b16d --- /dev/null +++ b/changes-entries/cgid-env-limit.txt @@ -0,0 +1,3 @@ + *) mod_cgid: Increase limits for number and length of + environment variables, URI and arguments. PR 70270. + [Emmanuel Dreyfus , Joe Orton] diff --git a/modules/generators/mod_cgid.c b/modules/generators/mod_cgid.c index 664498339d..c4ea6ff43b 100644 --- a/modules/generators/mod_cgid.c +++ b/modules/generators/mod_cgid.c @@ -192,7 +192,9 @@ typedef struct { } cgid_rlimit_t; #endif -#define ENV_COUNT_MAX (256) +/* Sanity bound on the length of one environment variable value + * received by the daemon. */ +#define ENV_VAL_MAX (128 * 1024) typedef struct { int req_type; /* request type (CGI_REQ, SSI_REQ, etc.) */ @@ -504,7 +506,8 @@ static apr_status_t sock_writev(int fd, int auxfd, request_rec *r, int count, .. return APR_SUCCESS; } -static apr_status_t get_req(int fd, request_rec *r, char **argv0, char ***env, +static apr_status_t get_req(int fd, server_rec *main_server, request_rec *r, + char **argv0, char ***env, int *errfd, cgid_req_t *req) { int i; @@ -532,11 +535,11 @@ static apr_status_t get_req(int fd, request_rec *r, char **argv0, char ***env, } /* Sanity check the structure received. */ - if (req->env_count > ENV_COUNT_MAX + if (req->env_count > APACHE_ARG_MAX || req->filename_len == 0 || req->filename_len > APR_PATH_MAX || req->argv0_len == 0 || req->argv0_len > APR_PATH_MAX - || req->uri_len == 0 || req->uri_len > APR_PATH_MAX - || req->args_len > APR_PATH_MAX) { + || req->uri_len == 0 || req->uri_len > main_server->limit_req_line + || req->args_len > main_server->limit_req_line) { return APR_EINVAL; } @@ -573,7 +576,7 @@ static apr_status_t get_req(int fd, request_rec *r, char **argv0, char ***env, if ((stat = sock_read(fd, &curlen, sizeof(curlen))) != APR_SUCCESS) { return stat; } - if (curlen > APR_PATH_MAX) { + if (curlen > ENV_VAL_MAX) { return APR_EINVAL; } environ[i] = apr_pcalloc(r->pool, curlen + 1); @@ -614,6 +617,9 @@ static apr_status_t send_req(int fd, apr_file_t *errpipe, request_rec *r, for (req.env_count = 0; env[req.env_count]; req.env_count++) { continue; } + if (req.env_count > APACHE_ARG_MAX) { + return APR_ENOSPC; + } req.filename_len = strlen(r->filename); req.argv0_len = strlen(argv0); req.uri_len = strlen(r->uri); @@ -846,7 +852,8 @@ static int cgid_server(void *data) r = apr_pcalloc(ptrans, sizeof(request_rec)); procnew = apr_pcalloc(ptrans, sizeof(*procnew)); r->pool = ptrans; - stat = get_req(sd2, r, &argv0, &env, &errfileno, &cgid_req); + stat = get_req(sd2, main_server, r, + &argv0, &env, &errfileno, &cgid_req); if (stat != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, stat, main_server, APLOGNO(01248) diff --git a/test/modules/generators/htdocs/cgi/arglen.py b/test/modules/generators/htdocs/cgi/arglen.py new file mode 100644 index 0000000000..3d874332a6 --- /dev/null +++ b/test/modules/generators/htdocs/cgi/arglen.py @@ -0,0 +1,5 @@ +#!/usr/bin/env python3 +import os + +print("Content-Type: text/plain\n") +print(len(os.environ.get("QUERY_STRING", ""))) diff --git a/test/modules/generators/htdocs/cgi/envcount.py b/test/modules/generators/htdocs/cgi/envcount.py new file mode 100644 index 0000000000..11e1d2a857 --- /dev/null +++ b/test/modules/generators/htdocs/cgi/envcount.py @@ -0,0 +1,5 @@ +#!/usr/bin/env python3 +import os + +print("Content-Type: text/plain\n") +print(len(os.environ)) diff --git a/test/modules/generators/test_001_cgid.py b/test/modules/generators/test_001_cgid.py index 65bde24c48..aef9eba8c9 100644 --- a/test/modules/generators/test_001_cgid.py +++ b/test/modules/generators/test_001_cgid.py @@ -24,6 +24,7 @@ class TestCgid: conf = HttpdConf(env, extras={ 'base': textwrap.dedent(f""" LogLevel cgid:trace1 + LimitRequestLine 16384 Options +Includes AcceptPathInfo On @@ -43,12 +44,19 @@ class TestCgid: CGIDScriptTimeout 1 - """), + """) + self.setenv_location("/envcount.py/small", 300) + + self.setenv_location("/envcount.py/large", 4200), }) conf.add_vhost_cgi() conf.install() assert env.apache_restart() == 0 + @staticmethod + def setenv_location(path, count): + setenvs = "\n".join(f"SetEnv CGID_TEST_VAR_{i:04} v" + for i in range(count)) + return f'\n{setenvs}\n\n' + def script_log(self, env): return os.path.join(env.server_logs_dir, "cgid_script.log") @@ -133,6 +141,28 @@ class TestCgid: assert r.response["status"] == 200 assert r.response["body"].decode() == "not reading\n" + def test_generators_001_08_env_count_large(self, env): + """More than 256 environment variables reach the script.""" + r = self.get(env, "cgi", "/envcount.py/small") + assert r.response["status"] == 200 + assert int(r.response["body"]) >= 300 + + def test_generators_001_09_env_count_overflow(self, env): + """A request environment larger than APACHE_ARG_MAX is refused.""" + r = self.get(env, "cgi", "/envcount.py/large") + assert r.response["status"] == 503 + # the daemon sees the aborted connection as a read error + env.httpd_error_log.ignore_recent(lognos=["AH10245", "AH01248"]) + + def test_generators_001_15_long_query(self, env): + """A 5000-character request URI, with a query string longer + than PATH_MAX, reaches the script.""" + path = "/arglen.py" + query = "x=" + "y" * (5000 - len(path) - len("?x=")) + r = self.get(env, "cgi", f"{path}?{query}") + assert r.response["status"] == 200 + assert int(r.response["body"]) == len(query) + def test_generators_001_10_ssi_exec_cgi(self, env): """#exec cgi= runs the script with the document's query string.""" r = self.get(env, "cgi", "/ssi/exec_cgi.shtml?hello+world")