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