Commit Graph
7 Commits
Author SHA1 Message Date
pamnarioandClaude Opus 5.5 4ec5a5c39a Merge commit from fork
* fastcgi: derive SCRIPT_FILENAME from the normalized request path

Path matchers evaluate a normalized request path — MatchPath applies
normalizeWindowsPath and then CleanPath before comparing — but the FastCGI
transport split r.URL.Path verbatim. Caddy does not rewrite r.URL.Path itself,
so the two disagreed whenever the path contained dot segments after the split
token: /admin.php/../../other.php was matched as /other.php while
SCRIPT_FILENAME resolved to <root>/admin.php, and on Windows the same held for
/admin.php\..\other.php. A route gated by a path matcher could therefore execute
a script that matcher never evaluated.

php_fastcgi generates a rewrite route that normally normalizes the path first,
but that route can fail to match, so the transport establishes the invariant
itself rather than inheriting it. The reason it can fail to match is addressed
in the following commit; this change does not depend on it.

caddyhttp.NormalizedPath is added so a consumer acting on a path a matcher has
already judged derives it exactly as the matcher did, rather than reassembling
the two steps and risking drift.

Scope — two divergences of the same class are NOT addressed here:

  * SCRIPT_NAME remains a prefix of the request path, so a matcher naming a
    script exactly (path /admin.php) is still reachable via PATH_INFO at
    /admin.php/anything. Matchers protecting a script need a trailing wildcard
    regardless; that is inherent to CGI, not to this bug.
  * On Windows, NTFS 8.3 short names are not rejected here. fileserver's
    ServeHTTP rejects them (and ':' for ADS) precisely because matchers cannot
    see through them, so GET /ADMIN~1.PHP is judged as /admin~1.php by a
    path matcher while NTFS resolves SCRIPT_FILENAME to admin.php. Closing that
    means sharing fileserver's hasWindowsShortName across packages, which is a
    larger change than this fix; it is reported alongside this one and a patch
    is available on request.

Side effects: PATH_INFO and DOCUMENT_URI are now derived from the normalized
path, so repeated slashes are merged and "." segments removed (/index.php//a//b
yields PATH_INFO=/a/b rather than //a//b), and on Windows per-component trailing
dots and spaces are folded. An application routing on a raw PATH_INFO will see
this. An empty URL.Path stays empty rather than becoming ".". REQUEST_URI is
unchanged and still carries the original target.

Tests: a buildEnv regression over literal and percent-encoded dot segments,
covering that legitimate PATH_INFO is preserved and an empty path stays empty; a
Windows-only counterpart whose cases each fail without the Windows
normalization; and an end-to-end test of a basic_auth-protected php_fastcgi
route against a FastCGI responder reporting the SCRIPT_FILENAME it received. The
end-to-end test uses `index off` so no rewrite route exists, ensuring it
exercises the transport rather than the rewrite.

* fileserver: make first_exist_fallback fall back when the file is absent

The try_policy first_exist_fallback is documented to match unconditionally,
falling back to the last try_files entry when no earlier candidate exists. It
did not: candidates are built with fs.Glob, which returns only paths that exist,
so an absent final entry left the list empty and the fallback branch — nested
inside the loop over that list — was unreachable.

php_fastcgi hard-codes this policy so its generated rewrite route always runs;
when the fallback failed, that rewrite was silently skipped.

Keep the pattern itself as a candidate when nothing globbed. This also aligns
Linux with Windows, where globResults is populated without an existence check
and the fallback already fired.

Behaviour change worth calling out: on a php_fastcgi site whose root has no
index.php (or no file matching the configured index), the rewrite now always
runs, so a request for a path that exists neither on the filesystem nor as a PHP
script is proxied to the backend as <root>/index.php instead of falling through
to file_server and returning 404. That is what the documented fallback promises,
but for a site of standalone .php scripts plus static assets it is a visible
change: unmatched requests now reach PHP-FPM.

The other try policies still test existence explicitly, so a candidate that does
not exist is skipped — but note this applies to ANY pattern that expands to
nothing, not only a malformed glob: a file whose literal name contains glob
metacharacters can now match under first_exist, largest_size and smallest_size
where previously globbing found it only via expansion.

* fileserver: synthesize the literal candidate only for the fallback entry

Address review: the previous commit appended the pattern itself as a
candidate inside makeCandidates whenever globbing found nothing. Since
makeCandidates is shared by every try_policy, first_exist, largest_size,
smallest_size and most_recently_modified could then stat the literal
pattern, so a file whose name contains glob metacharacters (for example
"lit[1].txt") matched where it did not before.

Restore makeCandidates to its original behaviour and build the literal
candidate only for the final entry of first_exist_fallback, the one
entry that is documented to match unconditionally. Placeholders in that
entry are expanded without glob escaping, since nothing is globbed, so
a request for /a[b].txt falls back to /a[b].txt rather than /a\[b].txt.
The path is still cleaned and joined with SanitizedPathJoin, so it
cannot escape the root.

This supersedes the last paragraph of the previous commit message: the
other policies now behave exactly as they did before that commit. The
php_fastcgi behaviour change described there still applies.

Add a regression test that creates a file literally named "lit[1].txt"
and checks that only the fallback entry of first_exist_fallback matches
it by its literal name, including the split_path remainder.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fastcgi: add license headers to new test files

Address review: phpfastcgi_test.go and fastcgi_windows_test.go were
added without the Apache license header used across the codebase. On
the Windows test, the header goes above the build constraint, matching
replacer_fuzz.go.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* fastcgi: avoid hardcoded Unix paths in dot-segment tests

Address review: the new tests compared SCRIPT_FILENAME against strings
with forward slashes ("/srv/other.php", a "/admin.php" suffix), but the
transport makes the root absolute and joins it with filepath.Join, so
on Windows the value is C:\srv\other.php and the assertions would fail.
Use a temporary directory as the root and build the expected paths with
filepath instead.

Also lower the bcrypt cost of the integration test's basic_auth hash to
the minimum. At cost 14 the authenticated request exceeded the 5s test
client timeout under -race.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-10-13 10:13:03 -06:00
bzyy1024 45ba3278b5 fastcgi: fix HTTPoxy vulnerability (#7934)
* Implement HTTPoxy mitigation in FastCGI
Added HTTPoxy mitigation to prevent trusting client-supplied Proxy header for HTTP_PROXY environment variable.

* Implement test for HTTPoxy vulnerability protection
Add test to ensure HTTPoxy vulnerability is mitigated by dropping client-supplied Proxy headers.

* gofmt: format code
* revert unrelated gofmt change to replacer_test.go
2026-08-19 13:07:24 +10:00
Kévin Dunglas 947087cadd Merge commit from fork
Caddy's underscore header filter (GHSA-f59h-q822-g45g) only checked
for `_`. PHP folds `.` to `_` when registering $_SERVER keys the same
way CGI/FastCGI folds `-` to `_`, so a dotted alias (e.g. Remote.User)
survived the filter and collided with the legitimate hyphenated
header once it reached a PHP/FastCGI backend, bypassing forward_auth
copy_headers the same way the underscore alias did.

Extends the filter to drop `.` symmetrically, adds an
`expected_dot_headers` allowlist mirroring `expected_underscore_headers`,
and handles header names containing both separators (only an exact
allowlist entry is honored there, since a prefix glob's free-form
suffix can't be vetted for an embedded second separator).

Root cause identified by @iliaal in the FrankenPHP advisory
GHSA-49wc-4hcv-v58q.
2026-08-11 08:00:15 -06:00
Renich Bon Ciric 546752e214 feat(fastcgi): populate SERVER_ADDR by default (#7912)
* feat(fastcgi): populate SERVER_ADDR by default

Populate the SERVER_ADDR FastCGI environment variable using the local socket address of the incoming connection from the request context. This improves out-of-the-box compatibility for PHP applications that rely on $_SERVER['SERVER_ADDR'] (such as legacy frameworks and custom telemetry packages) when migrating from Nginx or Apache.

Co-developed-by: Gemini AI <renich+gemini@woralelandia.com>
Signed-off-by: Rénich Bon Ćirić <renich@woralelandia.com>

* fix(fastcgi): validate SERVER_ADDR IP format and add test coverage

Validate that SERVER_ADDR is only populated when the local address parses as a valid IP address, preventing Unix socket paths from being assigned if SplitHostPort fails. Add test cases covering IPv4, IPv6, missing context, Unix sockets, and explicit overrides.

Co-developed-by: Gemini AI <renich+gemini@woralelandia.com>
Signed-off-by: Rénich Bon Ćirić <renich@woralelandia.com>
2026-08-10 15:24:13 +10:00
Kévin Dunglas 3eb8e48ff0 Merge commit from fork
* feat: drop headers with underscore in their names

* feat: Caddyfile binding and tests for underscore-in-header drop

Add the `allow_underscore_in_headers` global server option, refine the
doc comment, and cover the filter end-to-end: server-level unit tests
(drop, opt-out, debug log, RFC-7230 space rejection), a fastcgi unit
test for the trimmed header name replacer, and forward_auth integration
tests for both the default-drop and opt-out paths.

* remove allow_underscore_in_headers option for now
2026-05-29 11:37:17 -06:00
Kévin Dunglas fb324331f4 Merge commit from fork
Both fallbacks in splitPos relied on golang.org/x/text/search with
search.IgnoreCase, which performs Unicode equivalence matching far beyond
ASCII case folding. Combined with the validated-ASCII guarantee on every
SplitPath entry, that fallback turned non-PHP filenames into PHP scripts:

- when the inner loop hit a non-ASCII byte and the IndexString fallback
  returned -1, the loop broke without resetting match=false, so a stale
  match=true caused a non-existent .php to be reported (PoC:
  "/name.<U+00A1>.txt").
- search.IgnoreCase folded fullwidth, mathematical and circled letters
  onto ASCII, so "/shell.<math sans-serif php>",
  "/shell.<fullwidth p>hp", "/shell.<circled php>" were all detected as
  ".php" files.

Replace the fallback with strict byte-level ASCII case-insensitive
matching: any byte >= utf8.RuneSelf in the path can never be part of a
match, since SplitPath entries are validated ASCII-only and lower-cased
in Provision(). This keeps the hot path branch-light and removes the
x/text/search dependency from the main module.

Reported against FrankenPHP as GHSA-3g8v-8r37-cgjm and
GHSA-v4h7-cj44-8fc8. The vulnerable function in this module was adapted
from the same FrankenPHP code.
2026-05-07 13:59:42 -06:00
Kévin Dunglas 7c28c0c07a Merge commit from fork
* fix: FastCGI split SCRIPT_NAME/PATH_INFO confusion

* fix comment
2026-02-10 11:52:36 -07:00