Repository navigation
fix: block path traversal through disk and http file proxies - #174
Open
9xj89gzrtw-hue wants to merge 2 commits into
Open
9xj89gzrtw-hue wants to merge 2 commits into
9xj89gzrtw-hue wants to merge 2 commits into
Conversation
normalizePath collapsed only the first repeated slash group (missing /g flag), so paths like a///b///c.txt normalized to a/b///c.txt. The db-client normalizes file paths on write and again on lookup; a non-idempotent normalizer can make those comparisons inconsistent. - replace(/\/\/+/g, "/") now collapses every group - new tests: multi-group collapse, idempotence, windows separators and mixed separators, preprocessing edge cases
…scircuit#173) resolveFileProxy resolved user-controlled segments with join(diskPath, relativePath) without checking for .. segments, so a request like /files/download/local/../secret.txt read files outside the proxy root (CWE-22); the http flavor forwarded the same traversal upstream. - resolveFileProxy rejects any relative path containing a .. segment (400) - resolveDiskProxy additionally require the resolved path to stay inside resolve(diskPath), independent of platform separator handling - unit + route-level regression tests using raw HTTP (clients normalize dot-dot segments)
Author
|
/claim #173 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #173
Vulnerability
resolveFileProxyresolved user-controlled path segments withjoin(diskPath, relativePath)and no../containment check, so a request under a configured disk proxy prefix could read files anywhere the server process can:ctx.db.matchFileProxymatches by plainstartsWith(prefix), andnormalizePathdoes not remove dot segments, so..reachedjoin()intact. The HTTP flavor forwarded the same traversal segments upstream. CWE-22.Fix
resolveFileProxyrejects any relative path containing a..segment with400(covers both disk and http flavors; traversal segments are never forwarded upstream).resolveDiskProxyadds defense in depth: the joined path isresolve()d and required to stay insideresolve(diskPath)(platform-separator independent).Tests — new
tests/utils/resolve-file-proxy-traversal.test.ts+tests/routes/file-proxy-traversal-route.test.ts..attempts -> 400 (unit, disk + http flavors)..guardnode:httpsends the path verbatim — normal HTTP clients collapse..): a secret outside the proxy root never appears in any response; positive control serves normallyFull suite 42/42 pass;
tsc --noEmitclean; biome clean on changed files.