serve_error_page skips the path containment guard #65
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
From the v2.0.0 security audit, found while attacking the guard added in
911633a.serve_from_dirnow resolves throughcontained_path, which rebuilds the path fromComponent::Normalparts only.serve_error_pagedoes not — it joins itsfileargument onto the site directory directly, so anerror_pagesvalue of../../etc/passwdescapes.Not client-controlled:
filecomes from the operator's ownerror_pagesmap, and the directory is their own config too. So this is not a vulnerability — an operator who can seterror_pagescan already pointdiranywhere. Filing it because it is now the one static-serving path that does not go through the guard, which is exactly the kind of asymmetry that becomes a vulnerability when someone later makes error pages configurable over the API.Fix: route it through
contained_pathas well. Cheap, and it makes "every filesystem read from a request is contained" true without exception.Also recorded from the same attack pass
The guard itself held against:
/../secret,/a/../../secret,/....//....//etc(....is aNormalcomponent),/..%2f..%2fetc(no percent-decoding anywhere on the path, so it becomes a literal filename), backslash/..\\..\\etc(0x5C is admitted by the URI parser but is aNormalbyte on Linux), overlong and high-byte UTF-8 (reachcomponents()but compare unequal to.., and Linux compares raw bytes), NUL (rejected by the URI parser), leading.components, and theindex.htmlappend.Component::Prefixis unreachable on Unix. Symlink-following remains the one real reach and is deliberate — Kyoshi serves a Nix store path through one — so containment is on the requested path, not the resolved target. Worth a line in the release notes: a site directory containing an attacker-writable symlink is a full escape.Fixed in
fee27e1.serve_error_pagenow resolves throughcontained_path, so "every filesystem read from a request is contained" holds without exception.One deviation from a straight reroute: an empty
filereturnsNonerather than inheritingcontained_path's empty-path default. That default isindex.html, which is right for a request path and wrong here —error_pages = { 404 = "" }would have served the app's landing page dressed as the error body, which is the same "a file from an unrelated source dressed as the app" failure the error-page design already guards against. A test pins it.The escape test uses your
site_with_a_secret_above_ithelper, which asserts the target is readable before asserting the refusal — without that it would pass unfixed on ENOENT, which is how two tests fooled me earlier this week.Release-notes line added for the symlink reach, under Known limits: containment is on the requested path, not the resolved target, so a site directory containing an attacker-writable symlink is a full escape. It names the forms you tested that held, so the next person knows the guard was attacked rather than assumed, and says plainly not to put a site directory anywhere untrusted content can create links.
376 tests pass, clippy
--all-targets -D warningsclean, fmt clean.