Security: add SSRF, path traversal, shell injection, and rate limiting protections
- SSRF protection in media fetch: block private IPs, localhost, link-local addresses using ipaddr.js; manually follow redirects to validate each hop - Path traversal protection in web media: allowlist ~/.clawdbot and tmpdir, follow symlinks to prevent escape attacks - Shell injection fix in CLI credentials: use spawnSync with argument arrays instead of execSync with string interpolation - Rate limiting for gateway auth: 5 failures = 1 minute lockout, prevents brute-force attacks - Tailscale auth failure tracking: record failures for whois mismatches - Logger permissions: set 0o700 on log directory, warn if chmod fails Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
jspisak
parent
dce7925e2a
commit
062760fc28
@ -183,6 +183,7 @@
|
||||
"file-type": "^21.3.0",
|
||||
"grammy": "^1.39.3",
|
||||
"hono": "4.11.4",
|
||||
"ipaddr.js": "^2.2.0",
|
||||
"jiti": "^2.6.1",
|
||||
"json5": "^2.2.3",
|
||||
"jszip": "^3.10.1",
|
||||
|
||||
9
pnpm-lock.yaml
generated
9
pnpm-lock.yaml
generated
@ -112,6 +112,9 @@ importers:
|
||||
hono:
|
||||
specifier: 4.11.4
|
||||
version: 4.11.4
|
||||
ipaddr.js:
|
||||
specifier: ^2.2.0
|
||||
version: 2.3.0
|
||||
jiti:
|
||||
specifier: ^2.6.1
|
||||
version: 2.6.1
|
||||
@ -3863,6 +3866,10 @@ packages:
|
||||
resolution: {integrity: sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g==}
|
||||
engines: {node: '>= 0.10'}
|
||||
|
||||
ipaddr.js@2.3.0:
|
||||
resolution: {integrity: sha512-Zv/pA+ciVFbCSBBjGfaKUya/CcGmUHzTydLMaTwrUUEM2DIEO3iZvueGxmacvmN50fGpGVKeTXpb2LcYQxeVdg==}
|
||||
engines: {node: '>= 10'}
|
||||
|
||||
ipull@3.9.3:
|
||||
resolution: {integrity: sha512-ZMkxaopfwKHwmEuGDYx7giNBdLxbHbRCWcQVA1D2eqE4crUguupfxej6s7UqbidYEwT69dkyumYkY8DPHIxF9g==}
|
||||
engines: {node: '>=18.0.0'}
|
||||
@ -9936,6 +9943,8 @@ snapshots:
|
||||
|
||||
ipaddr.js@1.9.1: {}
|
||||
|
||||
ipaddr.js@2.3.0: {}
|
||||
|
||||
ipull@3.9.3:
|
||||
dependencies:
|
||||
'@tinyhttp/content-disposition': 2.2.2
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
import { execSync } from "node:child_process";
|
||||
import { execSync, spawnSync } from "node:child_process";
|
||||
import { createHash } from "node:crypto";
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
@ -78,6 +78,37 @@ type ClaudeCliWriteOptions = ClaudeCliFileOptions & {
|
||||
|
||||
type ExecSyncFn = typeof execSync;
|
||||
|
||||
// Secure keychain operations using spawnSync to prevent shell injection
|
||||
function secureKeychainFind(service: string, account?: string): string | null {
|
||||
const args = ["find-generic-password", "-s", service];
|
||||
if (account) {
|
||||
args.push("-a", account);
|
||||
}
|
||||
args.push("-w");
|
||||
const result = spawnSync("security", args, {
|
||||
encoding: "utf8",
|
||||
timeout: 5000,
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
});
|
||||
if (result.status !== 0 || result.error) {
|
||||
return null;
|
||||
}
|
||||
return result.stdout?.trim() ?? null;
|
||||
}
|
||||
|
||||
function secureKeychainWrite(service: string, account: string, value: string): boolean {
|
||||
const result = spawnSync(
|
||||
"security",
|
||||
["add-generic-password", "-U", "-s", service, "-a", account, "-w", value],
|
||||
{
|
||||
encoding: "utf8",
|
||||
timeout: 5000,
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
},
|
||||
);
|
||||
return result.status === 0 && !result.error;
|
||||
}
|
||||
|
||||
function resolveClaudeCliCredentialsPath(homeDir?: string) {
|
||||
const baseDir = homeDir ?? resolveUserPath("~");
|
||||
return path.join(baseDir, CLAUDE_CLI_CREDENTIALS_RELATIVE_PATH);
|
||||
@ -113,20 +144,22 @@ function readCodexKeychainCredentials(options?: {
|
||||
}): CodexCliCredential | null {
|
||||
const platform = options?.platform ?? process.platform;
|
||||
if (platform !== "darwin") return null;
|
||||
const execSyncImpl = options?.execSync ?? execSync;
|
||||
// Note: execSync option preserved for test mocking but secure helper used by default
|
||||
const _execSyncImpl = options?.execSync;
|
||||
|
||||
const codexHome = resolveCodexHomePath();
|
||||
const account = computeCodexKeychainAccount(codexHome);
|
||||
|
||||
try {
|
||||
const secret = execSyncImpl(
|
||||
`security find-generic-password -s "Codex Auth" -a "${account}" -w`,
|
||||
{
|
||||
encoding: "utf8",
|
||||
timeout: 5000,
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
},
|
||||
).trim();
|
||||
// Use secure helper to prevent shell injection
|
||||
const secret = _execSyncImpl
|
||||
? _execSyncImpl(`security find-generic-password -s "Codex Auth" -a "${account}" -w`, {
|
||||
encoding: "utf8",
|
||||
timeout: 5000,
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
}).trim()
|
||||
: secureKeychainFind("Codex Auth", account);
|
||||
if (!secret) return null;
|
||||
|
||||
const parsed = JSON.parse(secret) as Record<string, unknown>;
|
||||
const tokens = parsed.tokens as Record<string, unknown> | undefined;
|
||||
@ -186,16 +219,19 @@ function readQwenCliCredentials(options?: { homeDir?: string }): QwenCliCredenti
|
||||
};
|
||||
}
|
||||
|
||||
function readClaudeCliKeychainCredentials(
|
||||
execSyncImpl: ExecSyncFn = execSync,
|
||||
): ClaudeCliCredential | null {
|
||||
function readClaudeCliKeychainCredentials(execSyncImpl?: ExecSyncFn): ClaudeCliCredential | null {
|
||||
try {
|
||||
const result = execSyncImpl(
|
||||
`security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w`,
|
||||
{ encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
|
||||
);
|
||||
// Use secure helper to prevent shell injection (unless test mock provided)
|
||||
const result = execSyncImpl
|
||||
? execSyncImpl(`security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w`, {
|
||||
encoding: "utf8",
|
||||
timeout: 5000,
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
})
|
||||
: secureKeychainFind(CLAUDE_CLI_KEYCHAIN_SERVICE);
|
||||
if (!result) return null;
|
||||
|
||||
const data = JSON.parse(result.trim());
|
||||
const data = JSON.parse(typeof result === "string" ? result.trim() : result);
|
||||
const claudeOauth = data?.claudeAiOauth;
|
||||
if (!claudeOauth || typeof claudeOauth !== "object") return null;
|
||||
|
||||
@ -311,14 +347,20 @@ export function writeClaudeCliKeychainCredentials(
|
||||
newCredentials: OAuthCredentials,
|
||||
options?: { execSync?: ExecSyncFn },
|
||||
): boolean {
|
||||
const execSyncImpl = options?.execSync ?? execSync;
|
||||
const execSyncImpl = options?.execSync;
|
||||
try {
|
||||
const existingResult = execSyncImpl(
|
||||
`security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w 2>/dev/null`,
|
||||
{ encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
|
||||
);
|
||||
// Use secure helper to prevent shell injection (unless test mock provided)
|
||||
const existingResult = execSyncImpl
|
||||
? execSyncImpl(
|
||||
`security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w 2>/dev/null`,
|
||||
{ encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
|
||||
)
|
||||
: secureKeychainFind(CLAUDE_CLI_KEYCHAIN_SERVICE);
|
||||
if (!existingResult) return false;
|
||||
|
||||
const existingData = JSON.parse(existingResult.trim());
|
||||
const existingData = JSON.parse(
|
||||
typeof existingResult === "string" ? existingResult.trim() : existingResult,
|
||||
);
|
||||
const existingOauth = existingData?.claudeAiOauth;
|
||||
if (!existingOauth || typeof existingOauth !== "object") {
|
||||
return false;
|
||||
@ -333,10 +375,23 @@ export function writeClaudeCliKeychainCredentials(
|
||||
|
||||
const newValue = JSON.stringify(existingData);
|
||||
|
||||
execSyncImpl(
|
||||
`security add-generic-password -U -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -a "${CLAUDE_CLI_KEYCHAIN_ACCOUNT}" -w '${newValue.replace(/'/g, "'\"'\"'")}'`,
|
||||
{ encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
|
||||
);
|
||||
// Use secure helper for write (unless test mock provided)
|
||||
if (execSyncImpl) {
|
||||
execSyncImpl(
|
||||
`security add-generic-password -U -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -a "${CLAUDE_CLI_KEYCHAIN_ACCOUNT}" -w '${newValue.replace(/'/g, "'\"'\"'")}'`,
|
||||
{ encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] },
|
||||
);
|
||||
} else {
|
||||
const writeOk = secureKeychainWrite(
|
||||
CLAUDE_CLI_KEYCHAIN_SERVICE,
|
||||
CLAUDE_CLI_KEYCHAIN_ACCOUNT,
|
||||
newValue,
|
||||
);
|
||||
if (!writeOk) {
|
||||
log.warn("failed to write credentials to claude cli keychain via secure helper");
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
log.info("wrote refreshed credentials to claude cli keychain", {
|
||||
expires: new Date(newCredentials.expires).toISOString(),
|
||||
|
||||
@ -1,6 +1,11 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { describe, expect, it, beforeEach } from "vitest";
|
||||
|
||||
import { authorizeGatewayConnect } from "./auth.js";
|
||||
import {
|
||||
authorizeGatewayConnect,
|
||||
checkRateLimit,
|
||||
recordAuthFailure,
|
||||
resetRateLimiter,
|
||||
} from "./auth.js";
|
||||
|
||||
describe("gateway auth", () => {
|
||||
it("does not throw when req is missing socket", async () => {
|
||||
@ -100,3 +105,85 @@ describe("gateway auth", () => {
|
||||
expect(res.user).toBe("peter");
|
||||
});
|
||||
});
|
||||
|
||||
describe("rate limiting", () => {
|
||||
const mockReq = (ip: string) => ({ socket: { remoteAddress: ip } }) as never;
|
||||
|
||||
beforeEach(() => {
|
||||
resetRateLimiter();
|
||||
});
|
||||
|
||||
it("allows requests initially", () => {
|
||||
const result = checkRateLimit(mockReq("192.0.2.1"));
|
||||
expect(result.allowed).toBe(true);
|
||||
expect(result.retryAfterMs).toBeUndefined();
|
||||
});
|
||||
|
||||
it("allows requests after fewer than 5 failures", () => {
|
||||
const req = mockReq("192.0.2.2");
|
||||
for (let i = 0; i < 4; i++) {
|
||||
recordAuthFailure(req);
|
||||
}
|
||||
const result = checkRateLimit(req);
|
||||
expect(result.allowed).toBe(true);
|
||||
});
|
||||
|
||||
it("blocks requests after 5 failures", () => {
|
||||
const req = mockReq("192.0.2.3");
|
||||
for (let i = 0; i < 5; i++) {
|
||||
recordAuthFailure(req);
|
||||
}
|
||||
const result = checkRateLimit(req);
|
||||
expect(result.allowed).toBe(false);
|
||||
expect(result.retryAfterMs).toBeGreaterThan(0);
|
||||
expect(result.retryAfterMs).toBeLessThanOrEqual(60000);
|
||||
});
|
||||
|
||||
it("tracks different IPs independently", () => {
|
||||
const req1 = mockReq("192.0.2.10");
|
||||
const req2 = mockReq("192.0.2.20");
|
||||
|
||||
// Exhaust rate limit for req1
|
||||
for (let i = 0; i < 5; i++) {
|
||||
recordAuthFailure(req1);
|
||||
}
|
||||
|
||||
// req1 should be blocked
|
||||
expect(checkRateLimit(req1).allowed).toBe(false);
|
||||
|
||||
// req2 should still be allowed
|
||||
expect(checkRateLimit(req2).allowed).toBe(true);
|
||||
});
|
||||
|
||||
it("uses 'unknown' key when socket is missing", () => {
|
||||
const reqWithoutSocket = {} as never;
|
||||
for (let i = 0; i < 5; i++) {
|
||||
recordAuthFailure(reqWithoutSocket);
|
||||
}
|
||||
expect(checkRateLimit(reqWithoutSocket).allowed).toBe(false);
|
||||
});
|
||||
|
||||
it("integrates with authorizeGatewayConnect", async () => {
|
||||
const req = mockReq("192.0.2.100");
|
||||
|
||||
// Fail auth 5 times
|
||||
for (let i = 0; i < 5; i++) {
|
||||
const res = await authorizeGatewayConnect({
|
||||
auth: { mode: "token", token: "secret", allowTailscale: false },
|
||||
connectAuth: { token: "wrong" },
|
||||
req,
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.reason).toBe("token_mismatch");
|
||||
}
|
||||
|
||||
// 6th attempt should be rate limited
|
||||
const res = await authorizeGatewayConnect({
|
||||
auth: { mode: "token", token: "secret", allowTailscale: false },
|
||||
connectAuth: { token: "correct" }, // Even correct token should be blocked
|
||||
req,
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
expect(res.reason).toBe("rate_limited");
|
||||
});
|
||||
});
|
||||
|
||||
@ -3,6 +3,76 @@ import type { IncomingMessage } from "node:http";
|
||||
import type { GatewayAuthConfig, GatewayTailscaleMode } from "../config/config.js";
|
||||
import { readTailscaleWhoisIdentity, type TailscaleWhoisIdentity } from "../infra/tailscale.js";
|
||||
import { isTrustedProxyAddress, parseForwardedForClientIp, resolveGatewayClientIp } from "./net.js";
|
||||
|
||||
// Rate limiting for brute-force protection
|
||||
type RateLimitEntry = {
|
||||
failures: number;
|
||||
lastAttempt: number;
|
||||
lockedUntil: number;
|
||||
};
|
||||
|
||||
const rateLimitMap = new Map<string, RateLimitEntry>();
|
||||
const MAX_FAILURES = 5;
|
||||
const LOCKOUT_MS = 60_000; // 1 minute
|
||||
const WINDOW_MS = 300_000; // 5 minutes
|
||||
const CLEANUP_INTERVAL_MS = 60_000;
|
||||
|
||||
// Cleanup stale entries to prevent memory leak
|
||||
let cleanupInterval: ReturnType<typeof setInterval> | null = null;
|
||||
function ensureCleanupInterval(): void {
|
||||
if (cleanupInterval) return;
|
||||
cleanupInterval = setInterval(() => {
|
||||
const now = Date.now();
|
||||
for (const [key, entry] of rateLimitMap) {
|
||||
if (now - entry.lastAttempt > WINDOW_MS) {
|
||||
rateLimitMap.delete(key);
|
||||
}
|
||||
}
|
||||
}, CLEANUP_INTERVAL_MS);
|
||||
// Don't keep process alive just for cleanup
|
||||
cleanupInterval.unref?.();
|
||||
}
|
||||
|
||||
function getRateLimitKey(req?: IncomingMessage): string {
|
||||
// Use socket address, not forwarded headers (spoofable)
|
||||
return req?.socket?.remoteAddress ?? "unknown";
|
||||
}
|
||||
|
||||
export function checkRateLimit(req?: IncomingMessage): { allowed: boolean; retryAfterMs?: number } {
|
||||
ensureCleanupInterval();
|
||||
const key = getRateLimitKey(req);
|
||||
const now = Date.now();
|
||||
const entry = rateLimitMap.get(key);
|
||||
|
||||
if (entry && now < entry.lockedUntil) {
|
||||
return { allowed: false, retryAfterMs: entry.lockedUntil - now };
|
||||
}
|
||||
|
||||
return { allowed: true };
|
||||
}
|
||||
|
||||
export function recordAuthFailure(req?: IncomingMessage): void {
|
||||
ensureCleanupInterval();
|
||||
const key = getRateLimitKey(req);
|
||||
const now = Date.now();
|
||||
const entry = rateLimitMap.get(key) ?? { failures: 0, lastAttempt: 0, lockedUntil: 0 };
|
||||
|
||||
entry.failures += 1;
|
||||
entry.lastAttempt = now;
|
||||
|
||||
if (entry.failures >= MAX_FAILURES) {
|
||||
entry.lockedUntil = now + LOCKOUT_MS;
|
||||
entry.failures = 0; // Reset after lockout
|
||||
}
|
||||
|
||||
rateLimitMap.set(key, entry);
|
||||
}
|
||||
|
||||
// Test helper
|
||||
export function resetRateLimiter(): void {
|
||||
rateLimitMap.clear();
|
||||
}
|
||||
|
||||
export type ResolvedGatewayAuthMode = "token" | "password";
|
||||
|
||||
export type ResolvedGatewayAuth = {
|
||||
@ -204,6 +274,13 @@ export async function authorizeGatewayConnect(params: {
|
||||
tailscaleWhois?: TailscaleWhoisLookup;
|
||||
}): Promise<GatewayAuthResult> {
|
||||
const { auth, connectAuth, req, trustedProxies } = params;
|
||||
|
||||
// Rate limit check - prevent brute-force attacks
|
||||
const rateLimit = checkRateLimit(req);
|
||||
if (!rateLimit.allowed) {
|
||||
return { ok: false, reason: "rate_limited" };
|
||||
}
|
||||
|
||||
const tailscaleWhois = params.tailscaleWhois ?? readTailscaleWhoisIdentity;
|
||||
const localDirect = isLocalDirectRequest(req, trustedProxies);
|
||||
|
||||
@ -219,6 +296,14 @@ export async function authorizeGatewayConnect(params: {
|
||||
user: tailscaleCheck.user.login,
|
||||
};
|
||||
}
|
||||
// Record auth failure for Tailscale attempts that had user headers but failed verification
|
||||
// (don't count missing headers as a "failure" since that's just falling through to token/password)
|
||||
if (
|
||||
tailscaleCheck.reason === "tailscale_user_mismatch" ||
|
||||
tailscaleCheck.reason === "tailscale_whois_failed"
|
||||
) {
|
||||
recordAuthFailure(req);
|
||||
}
|
||||
}
|
||||
|
||||
if (auth.mode === "token") {
|
||||
@ -229,6 +314,7 @@ export async function authorizeGatewayConnect(params: {
|
||||
return { ok: false, reason: "token_missing" };
|
||||
}
|
||||
if (!safeEqual(connectAuth.token, auth.token)) {
|
||||
recordAuthFailure(req);
|
||||
return { ok: false, reason: "token_mismatch" };
|
||||
}
|
||||
return { ok: true, method: "token" };
|
||||
@ -243,6 +329,7 @@ export async function authorizeGatewayConnect(params: {
|
||||
return { ok: false, reason: "password_missing" };
|
||||
}
|
||||
if (!safeEqual(password, auth.password)) {
|
||||
recordAuthFailure(req);
|
||||
return { ok: false, reason: "password_mismatch" };
|
||||
}
|
||||
return { ok: true, method: "password" };
|
||||
|
||||
@ -82,7 +82,29 @@ export function isFileLogLevelEnabled(level: LogLevel): boolean {
|
||||
}
|
||||
|
||||
function buildLogger(settings: ResolvedSettings): TsLogger<LogObj> {
|
||||
fs.mkdirSync(path.dirname(settings.file), { recursive: true });
|
||||
const logDir = path.dirname(settings.file);
|
||||
// Create log directory with secure permissions (owner-only access)
|
||||
try {
|
||||
if (!fs.existsSync(logDir)) {
|
||||
fs.mkdirSync(logDir, { recursive: true, mode: 0o700 });
|
||||
} else {
|
||||
// Ensure existing dir has secure permissions
|
||||
fs.chmodSync(logDir, 0o700);
|
||||
}
|
||||
} catch (err) {
|
||||
// Fall back to default umask if chmod fails (e.g., on Windows)
|
||||
// Log a warning since permissions may not be secure
|
||||
if (process.platform !== "win32") {
|
||||
console.warn(
|
||||
`[clawdbot] Could not set secure permissions on log directory ${logDir}: ${err}`,
|
||||
);
|
||||
}
|
||||
try {
|
||||
fs.mkdirSync(logDir, { recursive: true });
|
||||
} catch {
|
||||
// Directory may already exist, ignore
|
||||
}
|
||||
}
|
||||
// Clean up stale rolling logs when using a dated log filename.
|
||||
if (isRollingPath(settings.file)) {
|
||||
pruneOldRollingLogs(path.dirname(settings.file));
|
||||
|
||||
@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { fetchRemoteMedia } from "./fetch.js";
|
||||
import { fetchRemoteMedia, _isUrlAllowed } from "./fetch.js";
|
||||
|
||||
function makeStream(chunks: Uint8Array[]) {
|
||||
return new ReadableStream<Uint8Array>({
|
||||
@ -44,4 +44,128 @@ describe("fetchRemoteMedia", () => {
|
||||
}),
|
||||
).rejects.toThrow("exceeds maxBytes");
|
||||
});
|
||||
|
||||
it("blocks localhost URLs", async () => {
|
||||
await expect(fetchRemoteMedia({ url: "http://localhost:8080/file" })).rejects.toThrow(
|
||||
"URL not allowed",
|
||||
);
|
||||
|
||||
await expect(fetchRemoteMedia({ url: "http://localhost/file" })).rejects.toThrow(
|
||||
"URL not allowed",
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks private IP ranges", async () => {
|
||||
// 10.x.x.x
|
||||
await expect(fetchRemoteMedia({ url: "http://10.0.0.1/file" })).rejects.toThrow(
|
||||
"URL not allowed",
|
||||
);
|
||||
// 172.16.x.x - 172.31.x.x
|
||||
await expect(fetchRemoteMedia({ url: "http://172.16.0.1/file" })).rejects.toThrow(
|
||||
"URL not allowed",
|
||||
);
|
||||
// 192.168.x.x
|
||||
await expect(fetchRemoteMedia({ url: "http://192.168.1.1/file" })).rejects.toThrow(
|
||||
"URL not allowed",
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks loopback addresses", async () => {
|
||||
await expect(fetchRemoteMedia({ url: "http://127.0.0.1/file" })).rejects.toThrow(
|
||||
"URL not allowed",
|
||||
);
|
||||
await expect(fetchRemoteMedia({ url: "http://127.0.0.5/file" })).rejects.toThrow(
|
||||
"URL not allowed",
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks IPv6 loopback", async () => {
|
||||
await expect(fetchRemoteMedia({ url: "http://[::1]/file" })).rejects.toThrow("URL not allowed");
|
||||
});
|
||||
|
||||
it("blocks link-local addresses (AWS metadata)", async () => {
|
||||
await expect(
|
||||
fetchRemoteMedia({ url: "http://169.254.169.254/latest/meta-data/" }),
|
||||
).rejects.toThrow("URL not allowed");
|
||||
});
|
||||
|
||||
it("blocks redirects to private IPs", async () => {
|
||||
const fetchImpl = async () =>
|
||||
new Response(null, {
|
||||
status: 302,
|
||||
headers: { location: "http://127.0.0.1/admin" },
|
||||
});
|
||||
|
||||
await expect(
|
||||
fetchRemoteMedia({ url: "https://example.com/redirect", fetchImpl }),
|
||||
).rejects.toThrow("Redirect blocked");
|
||||
});
|
||||
|
||||
it("limits redirect count", async () => {
|
||||
let redirectCount = 0;
|
||||
const fetchImpl = async () => {
|
||||
redirectCount++;
|
||||
return new Response(null, {
|
||||
status: 302,
|
||||
headers: { location: `https://example.com/redirect${redirectCount}` },
|
||||
});
|
||||
};
|
||||
|
||||
await expect(fetchRemoteMedia({ url: "https://example.com/start", fetchImpl })).rejects.toThrow(
|
||||
"Too many redirects",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isUrlAllowed (SSRF protection)", () => {
|
||||
it("allows valid external URLs", () => {
|
||||
expect(_isUrlAllowed("https://example.com/file.jpg")).toBe(true);
|
||||
expect(_isUrlAllowed("http://cdn.example.org/image.png")).toBe(true);
|
||||
expect(_isUrlAllowed("https://8.8.8.8/file")).toBe(true);
|
||||
});
|
||||
|
||||
it("blocks non-http protocols", () => {
|
||||
expect(_isUrlAllowed("file:///etc/passwd")).toBe(false);
|
||||
expect(_isUrlAllowed("ftp://example.com/file")).toBe(false);
|
||||
expect(_isUrlAllowed("data:text/plain,hello")).toBe(false);
|
||||
});
|
||||
|
||||
it("blocks localhost variations", () => {
|
||||
expect(_isUrlAllowed("http://localhost/file")).toBe(false);
|
||||
expect(_isUrlAllowed("http://localhost:3000/api")).toBe(false);
|
||||
expect(_isUrlAllowed("http://sub.localhost/file")).toBe(false);
|
||||
});
|
||||
|
||||
it("blocks loopback IPs", () => {
|
||||
expect(_isUrlAllowed("http://127.0.0.1/file")).toBe(false);
|
||||
expect(_isUrlAllowed("http://127.0.0.255/file")).toBe(false);
|
||||
expect(_isUrlAllowed("http://[::1]/file")).toBe(false);
|
||||
});
|
||||
|
||||
it("blocks private IP ranges", () => {
|
||||
// Class A private
|
||||
expect(_isUrlAllowed("http://10.0.0.1/file")).toBe(false);
|
||||
expect(_isUrlAllowed("http://10.255.255.255/file")).toBe(false);
|
||||
// Class B private
|
||||
expect(_isUrlAllowed("http://172.16.0.1/file")).toBe(false);
|
||||
expect(_isUrlAllowed("http://172.31.255.255/file")).toBe(false);
|
||||
// Class C private
|
||||
expect(_isUrlAllowed("http://192.168.0.1/file")).toBe(false);
|
||||
expect(_isUrlAllowed("http://192.168.255.255/file")).toBe(false);
|
||||
});
|
||||
|
||||
it("blocks link-local addresses", () => {
|
||||
expect(_isUrlAllowed("http://169.254.169.254/latest/meta-data/")).toBe(false);
|
||||
expect(_isUrlAllowed("http://169.254.0.1/file")).toBe(false);
|
||||
});
|
||||
|
||||
it("blocks unspecified addresses", () => {
|
||||
expect(_isUrlAllowed("http://0.0.0.0/file")).toBe(false);
|
||||
expect(_isUrlAllowed("http://[::]/file")).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects invalid URLs", () => {
|
||||
expect(_isUrlAllowed("not-a-url")).toBe(false);
|
||||
expect(_isUrlAllowed("")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@ -1,7 +1,59 @@
|
||||
import path from "node:path";
|
||||
|
||||
import * as ipaddr from "ipaddr.js";
|
||||
|
||||
import { detectMime, extensionForMime } from "./mime.js";
|
||||
|
||||
/**
|
||||
* SSRF protection: validates that a URL is safe to fetch (not localhost, private IPs, etc.)
|
||||
*/
|
||||
function isUrlAllowed(url: string): boolean {
|
||||
let parsed: URL;
|
||||
try {
|
||||
parsed = new URL(url);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Only allow http/https
|
||||
if (!["http:", "https:"].includes(parsed.protocol)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const hostname = parsed.hostname;
|
||||
|
||||
// Block localhost variations
|
||||
if (hostname === "localhost" || hostname.endsWith(".localhost")) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check if hostname is an IP address
|
||||
try {
|
||||
// Handle IPv6 brackets [::1] -> ::1
|
||||
const cleanHostname = hostname.replace(/^\[|\]$/g, "");
|
||||
const addr = ipaddr.parse(cleanHostname);
|
||||
const range = addr.range();
|
||||
|
||||
// Block all private/special ranges
|
||||
const blockedRanges = [
|
||||
"loopback", // 127.0.0.0/8, ::1
|
||||
"private", // 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, fc00::/7
|
||||
"linkLocal", // 169.254.0.0/16, fe80::/10
|
||||
"uniqueLocal", // fc00::/7
|
||||
"unspecified", // 0.0.0.0, ::
|
||||
];
|
||||
|
||||
if (blockedRanges.includes(range)) {
|
||||
return false;
|
||||
}
|
||||
} catch {
|
||||
// Not a valid IP - it's a hostname, allow DNS resolution
|
||||
// Note: DNS rebinding is still possible but harder to exploit
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
type FetchMediaResult = {
|
||||
buffer: Buffer;
|
||||
contentType?: string;
|
||||
@ -63,23 +115,78 @@ async function readErrorBodySnippet(res: Response, maxChars = 200): Promise<stri
|
||||
}
|
||||
}
|
||||
|
||||
const MAX_REDIRECTS = 5;
|
||||
|
||||
export async function fetchRemoteMedia(options: FetchMediaOptions): Promise<FetchMediaResult> {
|
||||
const { url, fetchImpl, filePathHint, maxBytes } = options;
|
||||
|
||||
// SSRF protection: block private/local addresses
|
||||
if (!isUrlAllowed(url)) {
|
||||
throw new MediaFetchError("fetch_failed", `URL not allowed: blocked private/local address`);
|
||||
}
|
||||
|
||||
const fetcher: FetchLike | undefined = fetchImpl ?? globalThis.fetch;
|
||||
if (!fetcher) {
|
||||
throw new Error("fetch is not available");
|
||||
}
|
||||
|
||||
// Follow redirects manually to validate each redirect URL against SSRF rules
|
||||
let currentUrl = url;
|
||||
let res: Response;
|
||||
try {
|
||||
res = await fetcher(url);
|
||||
} catch (err) {
|
||||
throw new MediaFetchError("fetch_failed", `Failed to fetch media from ${url}: ${String(err)}`);
|
||||
let redirectCount = 0;
|
||||
|
||||
while (true) {
|
||||
try {
|
||||
res = await fetcher(currentUrl, { redirect: "manual" });
|
||||
} catch (err) {
|
||||
throw new MediaFetchError(
|
||||
"fetch_failed",
|
||||
`Failed to fetch media from ${currentUrl}: ${String(err)}`,
|
||||
);
|
||||
}
|
||||
|
||||
// Handle redirects (3xx status codes)
|
||||
if (res.status >= 300 && res.status < 400) {
|
||||
const location = res.headers.get("location");
|
||||
if (!location) {
|
||||
throw new MediaFetchError(
|
||||
"http_error",
|
||||
`Redirect from ${currentUrl} missing Location header`,
|
||||
);
|
||||
}
|
||||
|
||||
// Resolve relative redirects
|
||||
let redirectUrl: string;
|
||||
try {
|
||||
redirectUrl = new URL(location, currentUrl).href;
|
||||
} catch {
|
||||
throw new MediaFetchError("http_error", `Invalid redirect URL: ${location}`);
|
||||
}
|
||||
|
||||
// SSRF protection: validate redirect target
|
||||
if (!isUrlAllowed(redirectUrl)) {
|
||||
throw new MediaFetchError(
|
||||
"fetch_failed",
|
||||
`Redirect blocked: target URL not allowed (private/local address)`,
|
||||
);
|
||||
}
|
||||
|
||||
redirectCount++;
|
||||
if (redirectCount > MAX_REDIRECTS) {
|
||||
throw new MediaFetchError("fetch_failed", `Too many redirects (max ${MAX_REDIRECTS})`);
|
||||
}
|
||||
|
||||
currentUrl = redirectUrl;
|
||||
continue;
|
||||
}
|
||||
|
||||
// Not a redirect, exit loop
|
||||
break;
|
||||
}
|
||||
|
||||
if (!res.ok) {
|
||||
const statusText = res.statusText ? ` ${res.statusText}` : "";
|
||||
const redirected = res.url && res.url !== url ? ` (redirected to ${res.url})` : "";
|
||||
const redirected = currentUrl !== url ? ` (redirected to ${currentUrl})` : "";
|
||||
let detail = `HTTP ${res.status}${statusText}`;
|
||||
if (!res.body) {
|
||||
detail = `HTTP ${res.status}${statusText}; empty response body`;
|
||||
@ -184,3 +291,6 @@ async function readResponseWithLimit(res: Response, maxBytes: number): Promise<B
|
||||
total,
|
||||
);
|
||||
}
|
||||
|
||||
// Export for testing
|
||||
export { isUrlAllowed as _isUrlAllowed };
|
||||
|
||||
@ -6,7 +6,7 @@ import sharp from "sharp";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { optimizeImageToPng } from "../media/image-ops.js";
|
||||
import { loadWebMedia, optimizeImageToJpeg } from "./media.js";
|
||||
import { loadWebMedia, optimizeImageToJpeg, _isPathAllowed } from "./media.js";
|
||||
|
||||
const tmpFiles: string[] = [];
|
||||
|
||||
@ -262,3 +262,68 @@ describe("web media loading", () => {
|
||||
expect(result.buffer.length).toBeLessThanOrEqual(cap);
|
||||
});
|
||||
});
|
||||
|
||||
describe("path traversal protection", () => {
|
||||
it("allows paths within ~/.clawdbot", () => {
|
||||
const clawdbotPath = path.join(os.homedir(), ".clawdbot", "test.txt");
|
||||
expect(_isPathAllowed(clawdbotPath)).toBe(true);
|
||||
});
|
||||
|
||||
it("allows paths within tmpdir", async () => {
|
||||
// Create actual temp file to test real path resolution
|
||||
const tmpFile = path.join(
|
||||
os.tmpdir(),
|
||||
`clawdbot-test-${Date.now()}-${Math.random().toString(16).slice(2)}.txt`,
|
||||
);
|
||||
await fs.writeFile(tmpFile, "test");
|
||||
tmpFiles.push(tmpFile);
|
||||
|
||||
expect(_isPathAllowed(tmpFile)).toBe(true);
|
||||
});
|
||||
|
||||
it("allows non-existent paths within allowed directories", () => {
|
||||
const nonExistent = path.join(os.tmpdir(), "does-not-exist-12345.txt");
|
||||
// Non-existent paths within allowed directories return true (let the read fail with proper error)
|
||||
expect(_isPathAllowed(nonExistent)).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects non-existent paths outside allowed directories", () => {
|
||||
const nonExistent = "/etc/does-not-exist-12345.txt";
|
||||
// Non-existent paths outside allowed directories should be rejected
|
||||
expect(_isPathAllowed(nonExistent)).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects file:// URLs to restricted paths via loadWebMedia", async () => {
|
||||
// Use /bin/ls which exists on macOS and is definitely outside allowed roots
|
||||
await expect(loadWebMedia("file:///bin/ls", 1024 * 1024)).rejects.toThrow(
|
||||
/Access denied.*outside allowed directories/,
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects local paths to restricted areas via loadWebMedia", async () => {
|
||||
// Use /bin/ls which exists on macOS and is definitely outside allowed roots
|
||||
await expect(loadWebMedia("/bin/ls", 1024 * 1024)).rejects.toThrow(
|
||||
/Access denied.*outside allowed directories/,
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks symlink escapes", async () => {
|
||||
// Create a symlink in tmpdir that points to home directory (which is outside allowed roots)
|
||||
const symlinkPath = path.join(
|
||||
os.tmpdir(),
|
||||
`clawdbot-symlink-test-${Date.now()}-${Math.random().toString(16).slice(2)}`,
|
||||
);
|
||||
|
||||
try {
|
||||
await fs.symlink(os.homedir(), symlinkPath);
|
||||
tmpFiles.push(symlinkPath);
|
||||
|
||||
// The symlink is in tmpdir, but its target (home dir) is not in allowed roots
|
||||
// isPathAllowed should block this because realpathSync resolves to home dir
|
||||
expect(_isPathAllowed(symlinkPath)).toBe(false);
|
||||
} catch {
|
||||
// If we can't create symlinks (permissions, Windows), skip this test
|
||||
console.log("Skipping symlink test (cannot create symlink)");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@ -1,4 +1,6 @@
|
||||
import fsSync from "node:fs";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
@ -30,6 +32,53 @@ const HEIC_MIME_RE = /^image\/hei[cf]$/i;
|
||||
const HEIC_EXT_RE = /\.(heic|heif)$/i;
|
||||
const MB = 1024 * 1024;
|
||||
|
||||
/**
|
||||
* Path traversal protection: validates that a file path is within allowed directories.
|
||||
* Follows symlinks to ensure the real path is within bounds.
|
||||
*/
|
||||
function isPathAllowed(filePath: string): boolean {
|
||||
// Resolve the input path (handles ..)
|
||||
const resolved = path.resolve(filePath);
|
||||
|
||||
// Allowlist of safe roots (resolve them too since macOS /tmp is a symlink)
|
||||
const allowedRoots = [path.join(os.homedir(), ".clawdbot"), os.tmpdir()].map((root) => {
|
||||
try {
|
||||
return fsSync.realpathSync(root);
|
||||
} catch {
|
||||
return root;
|
||||
}
|
||||
});
|
||||
|
||||
// Helper to check if a path is within allowed roots
|
||||
const isWithinAllowedRoots = (pathToCheck: string): boolean => {
|
||||
return allowedRoots.some(
|
||||
(root) => pathToCheck === root || pathToCheck.startsWith(root + path.sep),
|
||||
);
|
||||
};
|
||||
|
||||
// Follow symlinks to get real path
|
||||
let realPath: string;
|
||||
try {
|
||||
realPath = fsSync.realpathSync(resolved);
|
||||
} catch {
|
||||
// Path doesn't exist - try to resolve the parent directory to handle symlinks
|
||||
// (e.g., /var -> /private/var on macOS)
|
||||
const parentDir = path.dirname(resolved);
|
||||
try {
|
||||
const realParent = fsSync.realpathSync(parentDir);
|
||||
const fileName = path.basename(resolved);
|
||||
realPath = path.join(realParent, fileName);
|
||||
} catch {
|
||||
// Parent also doesn't exist - check raw resolved path against allowed roots
|
||||
// This is more restrictive but safe
|
||||
return isWithinAllowedRoots(resolved);
|
||||
}
|
||||
}
|
||||
|
||||
// Check REAL path (after symlink resolution) is within allowed roots
|
||||
return isWithinAllowedRoots(realPath);
|
||||
}
|
||||
|
||||
function formatMb(bytes: number, digits = 2): string {
|
||||
return (bytes / MB).toFixed(digits);
|
||||
}
|
||||
@ -119,6 +168,10 @@ async function loadWebMediaInternal(
|
||||
} catch {
|
||||
throw new Error(`Invalid file:// URL: ${mediaUrl}`);
|
||||
}
|
||||
// Path traversal protection: ensure file is within allowed directories
|
||||
if (!isPathAllowed(mediaUrl)) {
|
||||
throw new Error("Access denied: file path outside allowed directories");
|
||||
}
|
||||
}
|
||||
|
||||
const optimizeAndClampImage = async (
|
||||
@ -200,6 +253,11 @@ async function loadWebMediaInternal(
|
||||
mediaUrl = resolveUserPath(mediaUrl);
|
||||
}
|
||||
|
||||
// Path traversal protection for local paths
|
||||
if (!isPathAllowed(mediaUrl)) {
|
||||
throw new Error("Access denied: file path outside allowed directories");
|
||||
}
|
||||
|
||||
// Local path
|
||||
const data = await fs.readFile(mediaUrl);
|
||||
const mime = await detectMime({ buffer: data, filePath: mediaUrl });
|
||||
@ -302,3 +360,6 @@ export async function optimizeImageToJpeg(
|
||||
}
|
||||
|
||||
export { optimizeImageToPng };
|
||||
|
||||
// Export for testing
|
||||
export { isPathAllowed as _isPathAllowed };
|
||||
|
||||
Loading…
Reference in New Issue
Block a user