mod_sed: fix out-of-bounds read when a y/// command has mismatched string lengths

* modules/filters/sed0.c (ycomp): Stop parsing after reporting
  SEDERR_TSNTSS when the destination string is shorter than the
  source string.  Previously the loop kept advancing tsp past the
  closing delimiter and past the NUL terminator of the command
  line, reading beyond the end of linebuf and eventually reporting
  a bogus "ending delimiter missing" error.

Submitted by: Ankit Prateek <ankit offbyquant.com>

* test/modules/filters/test_002_sed_ycomp.py: New test.  Check that
  y/// commands with unequal string lengths are rejected with the
  size-mismatch error rather than the delimiter error, in both
  directions, and that a well-formed y/// still compiles and the
  server starts.  Skipped on Windows, where "httpd -k start" is
  service control and fails before the configuration is read.

* test/modules/filters/env.py (FiltersTestSetup.__init__): Load
  mod_sed.
  (FiltersTestEnv.__init__): Add sed to the logged modules.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GitHub: closes #766, closes #678


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1938337 13f79535-47bb-0310-9956-ffa450edef68

changes-entries/sed-ycomp-mismatch.txt

@@ -0,0 +1,3 @@
+  *) mod_sed: Reject a y/// command whose two strings differ in length,
+     reporting the size mismatch rather than a missing delimiter.
+     [Ankit Prateek <ankit offbyquant.com>]

modules/filters/sed0.c

@@ -960,7 +960,13 @@ static char *ycomp(sed_commands_t *commands, char *expbuf)
             tsp++;
         }
         if(ep[cint] == commands->sseof || ep[cint] == '\0') {
+            /* Destination string is shorter than the source string: the byte
+             * just read is the closing delimiter or the NUL terminator, not a
+             * real replacement character.  Report the size mismatch and stop
+             * now -- continuing would walk tsp past the end of the line buffer
+             * (out-of-bounds read of linebuf). */
             command_errf(commands, SEDERR_TSNTSS, commands->linebuf);
+            return NULL;
         }
     }
     if(*tsp != commands->sseof) {

test/modules/filters/env.py

@@ -12,14 +12,14 @@ class FiltersTestSetup(HttpdTestSetup):
     def __init__(self, env: 'HttpdTestEnv'):
         super().__init__(env=env)
         self.add_source_dir(os.path.dirname(inspect.getfile(FiltersTestSetup)))
-        self.add_modules(["substitute"])
+        self.add_modules(["substitute", "sed"])
 
 
 class FiltersTestEnv(HttpdTestEnv):
 
     def __init__(self, pytestconfig=None):
         super().__init__(pytestconfig=pytestconfig)
-        self.add_httpd_log_modules(["substitute", "core"])
+        self.add_httpd_log_modules(["substitute", "sed", "core"])
 
     def setup_httpd(self, setup: HttpdTestSetup = None):
         super().setup_httpd(setup=FiltersTestSetup(env=self))

test/modules/filters/test_002_sed_ycomp.py

@@ -0,0 +1,60 @@
+import sys
+
+import pytest
+
+from pyhttpd.conf import HttpdConf
+
+# ycomp() reports "transform strings not the same size" when the two halves of
+# a y/// command differ in length, but used to carry on afterwards: the loop
+# kept advancing tsp, past the closing delimiter and past the NUL terminating
+# the config line, reading whatever followed until it happened to stop.  The
+# error eventually reported was therefore the wrong one, "ending delimiter
+# missing", raised once the walk fell off the end of the string.
+TOO_SHORT = "transform strings not the same size"
+FELL_OFF_THE_END = "ending delimiter missing"
+
+
+# On Windows the "apachectl" pyhttpd drives is httpd.exe itself, where
+# "-k start" means "start the installed service" and fails with AH00436
+# before the configuration is ever read, so there is no compile error to
+# assert on.
+@pytest.mark.skipif(sys.platform == "win32",
+                    reason="httpd -k start is service control on Windows")
+class TestSedYComp:
+
+    def install(self, env, expr):
+        conf = HttpdConf(env, extras={
+            f"test1.{env.http_tld}": f"""
+            <Location "/">
+                AddOutputFilterByType SED text/html
+                OutputSed "{expr}"
+            </Location>
+            """,
+        })
+        conf.add_vhost_test1()
+        conf.install()
+        return env._run_apachectl("start")
+
+    # Source longer than destination: rejected, naming the real problem.
+    @pytest.mark.parametrize("expr", ["y/abc/de/", "y/abcdef/de/",
+                                      "y/abcdefgh/x/"])
+    def test_filters_002_01(self, env, expr):
+        r = self.install(env, expr)
+        assert r.exit_code != 0, f"httpd started with bad expression {expr}"
+        assert TOO_SHORT in r.stderr, \
+            f"{expr}: expected '{TOO_SHORT}', got: {r.stderr.strip()}"
+        assert FELL_OFF_THE_END not in r.stderr, \
+            f"{expr}: parser walked off the end: {r.stderr.strip()}"
+
+    # Destination longer than source is caught by the trailing check and has
+    # always reported the right error; keep it that way.
+    def test_filters_002_02(self, env):
+        r = self.install(env, "y/ab/abc/")
+        assert r.exit_code != 0
+        assert TOO_SHORT in r.stderr, r.stderr
+
+    # A well-formed y/// still compiles and the server starts.
+    def test_filters_002_03(self, env):
+        r = self.install(env, "y/abc/xyz/")
+        assert r.exit_code == 0, f"valid expression rejected: {r.stderr}"
+        assert env.is_live(), "httpd did not come up"