From 9761157a6b611204361a664a9263ec639e764e59 Mon Sep 17 00:00:00 2001 From: jknapp Date: Wed, 5 Aug 2026 13:06:34 -0700 Subject: [PATCH] harden(cac-path-parity): make degenerate mappings inert instead of subtly wrong MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three loose ends from the review, none reachable from entrypoint-lsphp.sh today. The rewrite semantics and the prefix-boundary logic are untouched; both new guards only NARROW the set of configurations that do anything, and neither adds an error path — fail-open is unchanged. - to="/" produced "//public_html": cacpp_trim() keeps a lone separator, and the tail already starts with one. Collapse the prefix when there is a tail, keep it when there is not (value == from exactly, where "/" is correct). A doubled leading slash is not the same string as the cac-fpm value, which is the entire point of the extension. - a non-absolute `from`/`to` was accepted and applied. Both are now required to start with '/', otherwise RINIT returns exactly as it does for an absent mapping: inert, no diagnostic, request proceeds. - a well-formed but WRONG mapping stays undetectable, and now the FAILURE MODES block says so explicitly rather than leaving it as an unlisted gap, along with why that is acceptable (the entrypoint derives from/to from the same two variables it builds the compatibility symlink from, so a wrong mapping means the symlink is wrong too and the site is already broken more loudly) and where the only runtime signal is (`lsphp -i`). Two tests added, both non-vacuous — 007 rewrites without the absolute-path guard, 008 returns "//public_html" without the collapse. 8/8 pass on PHP 8.1/8.3/8.5, and the FPM harness still reports 9/9 against the changed .so. Co-Authored-By: Claude Opus 5 (1M context) --- ext/cac-path-parity/cac_path_parity.c | 45 ++++++++++++++++--- .../007-inert-when-mapping-relative.phpt | 20 +++++++++ .../tests/008-to-root-no-double-slash.phpt | 20 +++++++++ 3 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 ext/cac-path-parity/tests/007-inert-when-mapping-relative.phpt create mode 100644 ext/cac-path-parity/tests/008-to-root-no-double-slash.phpt diff --git a/ext/cac-path-parity/cac_path_parity.c b/ext/cac-path-parity/cac_path_parity.c index b59da77..00aea48 100644 --- a/ext/cac-path-parity/cac_path_parity.c +++ b/ext/cac-path-parity/cac_path_parity.c @@ -68,6 +68,10 @@ * request proceed. Nothing here can warn, throw, or 500 a customer site: * - mapping unset/empty (any tier that is not shared-ols) -> RINIT returns * immediately, extension is inert. + * - either side of the mapping not an ABSOLUTE path -> inert. Nothing + * the entrypoint writes is anything else, and a relative prefix cannot + * usefully match a SAPI-supplied path, so a malformed mapping is treated + * exactly like an absent one. * - $_SERVER absent or not an array -> return. * - key absent from $_SERVER -> skip that key. * - key present but not a string -> skip that key. @@ -76,6 +80,14 @@ * There is no error path, no userland-visible diagnostic, and no dependency on * the filesystem being readable. * + * The one thing this CANNOT detect is a well-formed but WRONG mapping: it will + * confidently rewrite to a wrong path and say nothing. That is accepted by + * construction rather than overlooked — entrypoint-lsphp.sh derives from/to from + * the same two variables it builds the compatibility symlink from, so a wrong + * mapping means the symlink is wrong too and the site is already broken in a far + * louder way. The only runtime signal is `lsphp -i`, which prints + * "Rewriting => active" alongside the live from/to values. + * * SCOPE / KNOWN LIMITS * -------------------- * Only $_SERVER is rewritten. LSAPI also answers getenv('DOCUMENT_ROOT') from @@ -180,12 +192,25 @@ static void cacpp_rewrite_key(zval *server, const char *key, size_t key_len, return; } - size_t tail_len = len - from_len; - zend_string *out = zend_string_alloc(to_len + tail_len, 0); + size_t tail_len = len - from_len; - memcpy(ZSTR_VAL(out), to, to_len); - memcpy(ZSTR_VAL(out) + to_len, s + from_len, tail_len); - ZSTR_VAL(out)[to_len + tail_len] = '\0'; + /* + * to="/" is the one absolute prefix that survives cacpp_trim() as a bare + * separator, and the tail always starts with one — splicing both would give + * "//public_html". Drop it when there IS a tail; keep it when there is not + * (value == from exactly, where "/" is the correct answer). Unreachable from + * the entrypoint, which always writes to=/home/. + */ + size_t eff_to_len = to_len; + if (tail_len > 0 && eff_to_len == 1 && to[0] == '/') { + eff_to_len = 0; + } + + zend_string *out = zend_string_alloc(eff_to_len + tail_len, 0); + + memcpy(ZSTR_VAL(out), to, eff_to_len); + memcpy(ZSTR_VAL(out) + eff_to_len, s + from_len, tail_len); + ZSTR_VAL(out)[eff_to_len + tail_len] = '\0'; zval nv; ZVAL_STR(&nv, out); @@ -214,6 +239,16 @@ PHP_RINIT_FUNCTION(cac_path_parity) return SUCCESS; } + /* + * Both sides must be ABSOLUTE. The entrypoint only ever writes absolute + * paths; a relative prefix would be a typo or a mangled ini, and matching it + * against a SAPI-supplied path could only ever produce nonsense. Treat it + * like an absent mapping — inert, no diagnostic, request proceeds. + */ + if (*from != '/' || *to != '/') { + return SUCCESS; + } + /* * With auto_globals_jit=On (the default) $_SERVER is not built yet at * RINIT — php_hash_environment() only MARKED it for lazy creation. Reading diff --git a/ext/cac-path-parity/tests/007-inert-when-mapping-relative.phpt b/ext/cac-path-parity/tests/007-inert-when-mapping-relative.phpt new file mode 100644 index 0000000..6fcefed --- /dev/null +++ b/ext/cac-path-parity/tests/007-inert-when-mapping-relative.phpt @@ -0,0 +1,20 @@ +--TEST-- +cac_path_parity: a non-absolute mapping is inert, not applied +--EXTENSIONS-- +cac_path_parity +--INI-- +cac_path_parity.from=mnt/users/bob/site.com +cac_path_parity.to=/home/bob +variables_order=EGPCS +--ENV-- +CONTEXT_DOCUMENT_ROOT=mnt/users/bob/site.com/public_html +--FILE-- + +--EXPECT-- +string(34) "mnt/users/bob/site.com/public_html" diff --git a/ext/cac-path-parity/tests/008-to-root-no-double-slash.phpt b/ext/cac-path-parity/tests/008-to-root-no-double-slash.phpt new file mode 100644 index 0000000..8f8adb0 --- /dev/null +++ b/ext/cac-path-parity/tests/008-to-root-no-double-slash.phpt @@ -0,0 +1,20 @@ +--TEST-- +cac_path_parity: to=/ does not produce a doubled separator +--EXTENSIONS-- +cac_path_parity +--INI-- +cac_path_parity.from=/mnt/users/bob/site.com +cac_path_parity.to=/ +variables_order=EGPCS +--ENV-- +CONTEXT_DOCUMENT_ROOT=/mnt/users/bob/site.com/public_html +--FILE-- +). Before the eff_to_len collapse this returned +// "//public_html". A path with a doubled leading slash is not the same string as +// the cac-fpm value, which is the entire point of this extension. +var_dump($_SERVER['CONTEXT_DOCUMENT_ROOT']); +?> +--EXPECT-- +string(12) "/public_html"