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 <manu netbsd.org>, jorton
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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

@@ -0,0 +1,3 @@
+  *) mod_cgid: Increase limits for number and length of 
+     environment variables, URI and arguments.  PR 70270.
+     [Emmanuel Dreyfus <manu netbsd.org>, Joe Orton]

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)

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", "")))

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))

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
             <Directory "{env.server_docs_dir}/cgi/ssi">
                 Options +Includes
                 AcceptPathInfo On
@@ -43,12 +44,19 @@ class TestCgid:
             <LocationMatch "^/sleep">
                 CGIDScriptTimeout 1
             </LocationMatch>
-            """),
+            """) + 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'<Location "{path}">\n{setenvs}\n</Location>\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")