From 7b93fd7027accbd3d2bfe9d482b5fd31ab4397db Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Thu, 17 Sep 2026 21:17:38 +0000 Subject: [PATCH] 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 * 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) 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 | 3 ++ modules/filters/sed0.c | 6 +++ test/modules/filters/env.py | 4 +- test/modules/filters/test_002_sed_ycomp.py | 60 ++++++++++++++++++++++ 4 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 changes-entries/sed-ycomp-mismatch.txt create mode 100644 test/modules/filters/test_002_sed_ycomp.py diff --git a/changes-entries/sed-ycomp-mismatch.txt b/changes-entries/sed-ycomp-mismatch.txt new file mode 100644 index 0000000000..6671196223 --- /dev/null +++ b/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 ] diff --git a/modules/filters/sed0.c b/modules/filters/sed0.c index a044f647db..84ed02d4e9 100644 --- a/modules/filters/sed0.c +++ b/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) { diff --git a/test/modules/filters/env.py b/test/modules/filters/env.py index 56b765f2df..c78a8fac0b 100644 --- a/test/modules/filters/env.py +++ b/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)) diff --git a/test/modules/filters/test_002_sed_ycomp.py b/test/modules/filters/test_002_sed_ycomp.py new file mode 100644 index 0000000000..91e818c42f --- /dev/null +++ b/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""" + + AddOutputFilterByType SED text/html + OutputSed "{expr}" + + """, + }) + 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"