fix(ui): prevent path traversal in static file serving#614
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2dc6674f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // 1. 尝试从系统 static 目录服务 | ||
| let filePath = path.join(process.cwd(), 'static', pathParam === '/' || pathParam === '/index.html' ? 'index.html' : pathParam.replace('/static/', '')); | ||
| const staticDir = path.resolve(process.cwd(), 'static'); | ||
| let filePath = path.resolve(staticDir, pathParam === '/' || pathParam === '/index.html' ? 'index.html' : pathParam.replace('/static/', '')); |
There was a problem hiding this comment.
Strip leading slash before resolving static paths
Using path.resolve(staticDir, pathParam...) with unnormalized request paths causes a regression: when pathParam starts with / (for example /app/base.css, /components/..., /favicon.ico, or plugin static paths matched in request-handler.js), resolve treats it as absolute and ignores staticDir, so the containment guard immediately returns false. As a result, static assets that previously loaded now 404, which breaks the UI page after index.html loads and prevents plugin static files from being served.
Useful? React with 👍 / 👎.
|
static/..%2Fpackage.json Invalid after testing |
Vulnerability: Path Traversal in Static File Serving (CWE-22)
Affected file:
src/services/ui-manager.js—serveStaticFiles()functionSummary
The
serveStaticFilesfunction constructs a file path usingpath.join()with user-supplied URL path segments. Becausepath.join()resolves..sequences but does not constrain the result to a base directory, an attacker can escape thestatic/directory and read arbitrary files from the server's filesystem.This endpoint is reached without authentication. In
src/handlers/request-handler.js(line ~121), any request matching a UI static prefix (e.g./static/) is routed directly toserveStaticFiles()before any auth middleware runs. TheUI_ENABLEDconfig defaults totrue.Data Flow
/static/../../etc/passwdisUIPath(path)returnstrue(path starts with/static/)serveStaticFiles(path, res)is called — no authentication checkpathParam.replace('/static/', '')yields../../etc/passwdpath.join(process.cwd(), 'static', '../../etc/passwd')resolves to/etc/passwdexistsSync(filePath)succeeds, file contents are served to the attackerProof of Concept
The
%2Fencoding is used because many HTTP clients and some proxies normalize bare../in URLs, but the Node.js HTTP server receives the decoded path. Depending on the deployment, bare../may also work directly.Fix Description
The fix replaces
path.join()withpath.resolve()and adds a directory containment check:path.resolve()produces an absolute canonical path (no..remaining), and thestartsWithcheck ensures the resolved path is still within the intendedstatic/directory. If traversal is detected, the function returnsfalseand the request falls through to a 404 — no file contents are leaked.Testing
curl "http://localhost:3456/static/..%2F..%2Fetc%2Fpasswd"returns a 404 after the fix (previously returned file contents)curl "http://localhost:3456/static/index.html"returns the UI pagepath.resolve()Adversarial Review
Before submitting, we verified this is not mitigated by other layers. The request handler serves static files before any authentication middleware executes (line 121 in
request-handler.js). There is no reverse proxy path normalization assumed in the codebase — the app is designed to run standalone vianode. TheisUIPathcheck only validates that the path starts with a known prefix, it does not sanitize traversal sequences. No existing input validation prevents this attack.Submitted by Sebastion — autonomous open-source security research from Foundation Machines. Free for public repos via the Sebastion AI GitHub App.