mirror of
https://github.com/tdurieux/anonymous_github.git
synced 2026-09-02 17:10:48 +02:00
Security hardening + gist UI fixes (#731)
* security: harden against XSS, ReDoS, path traversal, and injection Defensive fixes across the server, storage, and viewer: - XSS (CWE-79): sanitise rendered notebooks with DOMPurify, escape file names interpolated into AngularJS expressions (escapeNgString), set Mermaid securityLevel to 'strict', and stop urlRel2abs from returning javascript:/vbscript:/data:text/html URLs. - Path traversal / zip-slip (CWE-22/23/24): validate URL-derived path components before they reach the storage layer (file/webview routes + StorageBase.assertSafePath) and sanitise zip entry names on extract for both the filesystem and S3 backends. - ReDoS (CWE-1333): escape anonymization terms with catastrophic backtracking shapes to literals instead of compiling them as regexes. - Secret hardening (CWE-798): require SESSION_SECRET / OAuth creds / DB password in production, random dev SESSION_SECRET fallback. - Rate-limit spoofing (CWE-290): derive request.ip via trust-proxy hop count instead of the client-settable cf-connecting-ip header. - NoSQL injection (CWE-943): allow only plain field paths as admin sort keys. - Reject malformed streamer requests missing required string fields. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ui): make gists reachable/visible and clarify the ZIP button - Gist & PR routes now accept a trailing slash (/gist/:id/:path*?), so the dashboard links (which end in "/") resolve to the gist/PR page instead of falling through to the 404 route (#725). - Gist viewer picks the default tab after content loads, defaulting to "files" when files exist; previously the ng-init ran before the async load and a files-only gist rendered blank under the hidden comments tab. - Explorer toolbar: relabel ZIP to "Full repo ZIP" with a tooltip, and add tooltips to Raw/Download clarifying they apply to the current file (#721). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: report SAML-enforced orgs clearly instead of "token expired" When a repo's organization enforces SAML SSO, GitHub returns a 403 whose message differs from the OAuth-App-restriction case. That 403 fell through to the generic handler and surfaced as "token_expired", pushing users to re-login when the real fix is authorizing their token for the org. Detect the "SAML enforcement" message and raise a dedicated, actionable error instead (#379, #550). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * security: catch nested quantified groups in ReDoS guard and backslash path traversal - hasCatastrophicBacktracking now scans across nested parens ([\s\S]*?) so shapes like ((a+))+ are detected; comment reframed as a heuristic backstop rather than a proof. - file route path-traversal check now rejects backslash separators and a leading backslash, covering Windows-style "..\" payloads (CWE-22/25). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(dev): track dev-proxy script, ignore .DS_Store and .claude/ scripts/dev-proxy.js is referenced by the "dev:ui" npm script but was never committed, breaking the command on a fresh clone. Add it and ignore local-only macOS/Claude Code files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
bdfcc56d81
commit
e4ffd74068
+9
-3
@@ -109,6 +109,10 @@ export default async function start() {
|
||||
})
|
||||
);
|
||||
app.set("etag", "strong");
|
||||
// Trust exactly TRUST_PROXY proxy hops so Express derives request.ip from
|
||||
// the right X-Forwarded-For entry. This is what makes request.ip
|
||||
// trustworthy for rate limiting instead of a client-spoofable header.
|
||||
app.set("trust proxy", config.TRUST_PROXY);
|
||||
|
||||
// handle session and connection
|
||||
app.use(initSession());
|
||||
@@ -134,9 +138,11 @@ export default async function start() {
|
||||
request: express.Request,
|
||||
_response: express.Response
|
||||
): string {
|
||||
if (request.headers["cf-connecting-ip"]) {
|
||||
return request.headers["cf-connecting-ip"] as string;
|
||||
}
|
||||
// Use request.ip, which Express resolves from X-Forwarded-For honouring
|
||||
// the configured "trust proxy" hop count. Do NOT key off the
|
||||
// cf-connecting-ip header directly: when the server isn't actually behind
|
||||
// Cloudflare a client can set that header to an arbitrary value per
|
||||
// request and trivially bypass the rate limiter (CWE-290).
|
||||
if (!request.ip && request.socket.remoteAddress) {
|
||||
logger.warn("request.ip is missing");
|
||||
return request.socket.remoteAddress;
|
||||
|
||||
@@ -105,9 +105,18 @@ function escapeRegex(s: string): string {
|
||||
return s.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, "\\$&");
|
||||
}
|
||||
|
||||
// Only plain field paths may be used as a Mongo sort key. Rejecting keys that
|
||||
// start with "$" or contain anything other than [A-Za-z0-9_.] prevents an
|
||||
// admin-supplied req.query.sort from injecting operator-prefixed keys
|
||||
// (e.g. "$where") into the query object (CWE-943).
|
||||
function isSafeSortField(field: unknown): field is string {
|
||||
return typeof field === "string" && /^[A-Za-z_][A-Za-z0-9_.]*$/.test(field);
|
||||
}
|
||||
|
||||
function parseSort(req: express.Request, fallbackField = "_id"): Record<string, 1 | -1> {
|
||||
const direction = req.query.direction === "asc" ? 1 : -1;
|
||||
const field = (req.query.sort as string) || fallbackField;
|
||||
const requested = req.query.sort;
|
||||
const field = isSafeSortField(requested) ? requested : fallbackField;
|
||||
return { [field]: direction };
|
||||
}
|
||||
|
||||
|
||||
@@ -50,6 +50,23 @@ router.get(
|
||||
res
|
||||
);
|
||||
}
|
||||
// Reject path traversal before the path reaches the storage layer. The
|
||||
// storage backends also validate, but failing fast here keeps a crafted
|
||||
// "../" URL from being treated as a real lookup (CWE-22/25).
|
||||
if (
|
||||
anonymizedPath
|
||||
.split(/[\\/]/)
|
||||
.some((segment) => segment === "..") ||
|
||||
/^[\\/]/.test(anonymizedPath)
|
||||
) {
|
||||
return handleError(
|
||||
new AnonymousError("invalid_path", {
|
||||
httpStatus: 400,
|
||||
object: anonymizedPath,
|
||||
}),
|
||||
res
|
||||
);
|
||||
}
|
||||
|
||||
const repo = await getRepo(req, res, {
|
||||
nocheck: false,
|
||||
|
||||
@@ -85,6 +85,15 @@ async function webView(req: express.Request, res: express.Response) {
|
||||
const filePath = req.path.substring(
|
||||
indexRepoId + req.params.repoId.length + 1
|
||||
);
|
||||
// Reject traversal in the URL-derived segment before joining it onto the
|
||||
// page-source root. Stripping a single leading "/" or "." is not enough
|
||||
// to stop "../../" sequences from climbing out of the repo (CWE-22).
|
||||
if (filePath.split(/[\\/]/).some((segment) => segment === "..")) {
|
||||
throw new AnonymousError("invalid_path", {
|
||||
httpStatus: 400,
|
||||
object: filePath,
|
||||
});
|
||||
}
|
||||
let requestPath = path.join(wRoot, filePath);
|
||||
if (requestPath.at(0) == "/" || requestPath.at(0) == ".") {
|
||||
requestPath = requestPath.substring(1);
|
||||
|
||||
Reference in New Issue
Block a user