From 59f45d3c16cef070256f05bb8b66193ee072bb65 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Thu, 17 Sep 2026 20:21:29 +0100 Subject: [PATCH 1/2] * test/modules/filters/env.py: Load mod_sed. * test/modules/filters/test_002_sed_ycomp.py: New test suite. Co-Authored-By: Claude Opus 5 (1M context) GitHub: PR #678 --- test/modules/filters/env.py | 4 +- test/modules/filters/test_002_sed_ycomp.py | 60 ++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 test/modules/filters/test_002_sed_ycomp.py diff --git a/test/modules/filters/env.py b/test/modules/filters/env.py index 56b765f2dfc..c78a8fac0b2 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 00000000000..91e818c42f3 --- /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" From 65dea45450084680e350c2a9b5bd858f268ffa52 Mon Sep 17 00:00:00 2001 From: Ankit Prateek Date: Sun, 5 Jul 2026 08:32:25 +0530 Subject: [PATCH 2/2] mod_sed: fix out-of-bounds read in ycomp on mismatched y/// strings ycomp() compiles the sed "y/source/dest/" transliterate command by walking the source and destination strings in lock-step. The first scan loop guards *tsp against '\0'/'\n', but the second loop reads the destination via *tsp++ with no such guard: it stops only when the source pointer reaches the delimiter. When the destination string is shorter than the source, tsp runs past the destination's closing delimiter and the NUL terminator, reading past the end of the LBSIZE+1 line buffer (commands->linebuf) into adjacent memory until it happens to hit a matching byte. The existing size-mismatch check (SEDERR_TSNTSS) already detects this case -- it fires as soon as a destination byte reads back as the delimiter or NUL -- but it only logs and continues, so the over-read proceeds. Make that check abort compilation (return NULL, as every other error path in ycomp does), which both fixes the over-read and rejects the malformed command, matching the behaviour of standard sed ("strings for `y' command are different lengths"). The sed program is supplied via the OutputSed/InputSed configuration directives (ACCESS_CONF), so this is a robustness fix for malformed configuration, not a remotely triggerable issue. (cherry picked from commit f9e81ccc19e9715e5215c626421658fc55cc97a1) --- changes-entries/sed-ycomp-mismatch.txt | 3 +++ modules/filters/sed0.c | 6 ++++++ 2 files changed, 9 insertions(+) create mode 100644 changes-entries/sed-ycomp-mismatch.txt diff --git a/changes-entries/sed-ycomp-mismatch.txt b/changes-entries/sed-ycomp-mismatch.txt new file mode 100644 index 00000000000..66711962232 --- /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 a044f647dba..84ed02d4e94 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) {