From 690ff8738d6ee81fe81c26227b2fb60adb7eb167 Mon Sep 17 00:00:00 2001 From: jknapp Date: Wed, 5 Aug 2026 13:05:57 -0700 Subject: [PATCH] fix(cac-lsphp): stop generating php.ini fragments from unquoted interpolation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two generated ini drop-ins interpolated $user/$domain into an unquoted heredoc. Measured against the pre-fix script in a real cac-lsphp:php83 container with domain=$'evil.com\nprecision = 7\n; ': 99-cac-path-parity.ini contained the injected line and lsphp reported `precision => 7 => 7` — an arbitrary ini directive supplied through the domain env var and applied to every request. The `from` value was silently truncated at the newline too, so the site also got a wrong (but "active") mapping. Both values are panel-validated and both already feed `ln -sfn` and the shared-ols vhost config, so this is defense-in-depth rather than a live hole. It is worth closing anyway because the OTHER two hostile inputs the reviewer measured — `$(...)` (ini parse error) and `"` (empty value) — leave the parity extension INERT, which is precisely the silent failure this whole change set exists to eliminate. Two layers, neither of which can fatal a request: - values are emitted double-quoted via printf instead of heredoc interpolation. php.ini double-quoted values may span newlines, so a newline is data, not a new directive. - $user/$SAFE_DOMAIN are checked against [A-Za-z0-9._-]+ first, because quoting does NOT stop php.ini's own ${VAR} interpolation. A rejected value logs a WARNING, writes no mapping at all (not even the degraded auto_prepend fallback, which would not be right for such a site either) and reports `path parity = none (user/domain rejected)` on the startup line. After: same container, same hostile domain — no 99-cac-path-parity.ini is written, `precision => 14` (default), and the warning names the rejected values. Happy path re-verified for domain=site.com and domain=*.site.com: mapping written, `Rewriting => active`, from/to parse back byte-identical. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/entrypoint-lsphp.sh | 50 +++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/scripts/entrypoint-lsphp.sh b/scripts/entrypoint-lsphp.sh index dffd56d..a794714 100644 --- a/scripts/entrypoint-lsphp.sh +++ b/scripts/entrypoint-lsphp.sh @@ -68,6 +68,25 @@ SAFE_DOMAIN="$domain" case "$domain" in \*.*) SAFE_DOMAIN="wildcard.${domain#\*.}" ;; esac + +## Both of these get interpolated into generated php.ini fragments below. They +## are panel-validated and both already feed `ln -sfn` and the shared-ols vhost +## config, so a hostile value is not reachable today — this is the belt to that +## brace. A newline in $domain is an INI-DIRECTIVE INJECTION into the generated +## fragment (measured against the pre-fix script: domain=$'evil.com\nprecision = +## 7\n; ' put that directive in 99-cac-path-parity.ini and lsphp reported +## `precision => 7`); `$(...)` yields an ini parse error and `"` an empty value, +## and BOTH of those leave the +## path-parity extension INERT — the exact silent parity loss this whole change +## exists to eliminate. Quoting the emitted values (done below) neutralises +## newlines and quotes; it does NOT neutralise php.ini's own `${VAR}` +## interpolation, which is why the character class is checked as well. +INI_TOKENS_OK=yes +case "$user" in ''|*[!A-Za-z0-9._-]*) INI_TOKENS_OK=no ;; esac +case "$SAFE_DOMAIN" in ''|*[!A-Za-z0-9._-]*) INI_TOKENS_OK=no ;; esac +if [ "$INI_TOKENS_OK" != yes ]; then + echo "WARNING: entrypoint-lsphp: user/domain contain characters outside [A-Za-z0-9._-] — refusing to write the \$_SERVER path-parity mapping (the extension stays inert; requests are unaffected). user=$(printf '%q' "$user") domain=$(printf '%q' "$domain")" >&2 +fi ## The exact path prefix the shared-ols container serves this site from — the ## string OLS puts in SCRIPT_FILENAME/DOCUMENT_ROOT. Used twice: for the symlink ## that makes it RESOLVE, and for the cac_path_parity mapping that makes it READ @@ -124,11 +143,13 @@ LSPHP_INFO=$("$LSPHP_BIN" -i 2>/dev/null || true) SCAN_DIR=$(printf '%s\n' "$LSPHP_INFO" | awk -F'=> ' '/^Scan this dir/ {print $2; exit}') if [ -n "$SCAN_DIR" ]; then mkdir -p "$SCAN_DIR" - cat > "$SCAN_DIR/99-user-error-log.ini" < "$SCAN_DIR/99-user-error-log.ini" ## ---- $_SERVER path parity with cac-fpm ---- ## Point the cac_path_parity extension at THIS site's mapping. Same two ## values the compatibility symlink above is built from, so the rewrite and @@ -142,12 +163,19 @@ EOF ## normaliser was itself PHP_INI_PERDIR and any site with its own prepend ## silently displaced it, while making OUR prepend win would have disabled ## THEIRS. See ext/cac-path-parity/cac_path_parity.c. - if printf '%s\n' "$LSPHP_INFO" | grep -q '^cac_path_parity support => enabled$'; then - cat > "$SCAN_DIR/99-cac-path-parity.ini" < enabled$'; then + { + echo '; rendered at container start by entrypoint-lsphp.sh' + printf 'cac_path_parity.from = "%s"\n' "$OLS_SITE_PATH" + printf 'cac_path_parity.to = "%s"\n' "/home/$user" + } > "$SCAN_DIR/99-cac-path-parity.ini" ## Drop the pre-extension fallback if an older image left one here — the ## container filesystem survives a "docker restart", so an in-place upgrade ## must not keep a stale auto_prepend pointing at the old normaliser.