diff --git a/CHANGELOG.md b/CHANGELOG.md index 936ff55f561..521e28fcb10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -129,6 +129,7 @@ Docs: https://docs.openclaw.ai - Security/macOS app beta: enforce path-only `system.run` allowlist matching (drop basename matches like `echo`), migrate legacy basename entries to last resolved paths when available, and harden shell-chain handling to fail closed on unsafe parse/control syntax (including quoted command substitution/backticks). This is an optional allowlist-mode feature; default installs remain deny-by-default. This ships in the next npm release. Thanks @tdjackey for reporting. - Security/Agents: auto-generate and persist a dedicated `commands.ownerDisplaySecret` when `commands.ownerDisplay=hash`, remove gateway token fallback from owner-ID prompt hashing across CLI and embedded agent runners, and centralize owner-display secret resolution in one shared helper. This ships in the next npm release. Thanks @aether-ai-agent for reporting. - Security/SSRF: expand IPv4 fetch guard blocking to include RFC special-use/non-global ranges (including benchmarking, TEST-NET, multicast, and reserved/broadcast blocks), centralize range checks into a single CIDR policy table, and reuse one shared host/IP classifier across literal + DNS checks to reduce classifier drift. This ships in the next npm release. Thanks @princeeismond-dot for reporting. +- Security/SSRF: block RFC2544 benchmarking range (`198.18.0.0/15`) across direct and embedded-IP paths, and normalize IPv6 dotted-quad transition literals (for example `::127.0.0.1`, `64:ff9b::8.8.8.8`) in shared IP parsing/classification. - Security/Archive: block zip symlink escapes during archive extraction. - Security/Media sandbox: keep tmp media allowance for absolute tmp paths only and enforce symlink-escape checks before sandbox-validated reads, preventing tmp symlink exfiltration and relative `../` sandbox escapes when sandboxes live under tmp. (#17892) Thanks @dashed. - Browser/Upload: accept canonical in-root upload paths when the configured uploads directory is a symlink alias (for example `/tmp` -> `/private/tmp` on macOS), so browser upload validation no longer rejects valid files during client->server revalidation. (#23300, #23222, #22848) Thanks @bgaither4, @parkerati, and @Nabsku. diff --git a/src/infra/net/ssrf.dispatcher.test.ts b/src/infra/net/ssrf.dispatcher.test.ts index 2a7f79d2ddf..0dfb816aa00 100644 --- a/src/infra/net/ssrf.dispatcher.test.ts +++ b/src/infra/net/ssrf.dispatcher.test.ts @@ -14,7 +14,7 @@ import { createPinnedDispatcher, type PinnedHostname } from "./ssrf.js"; describe("createPinnedDispatcher", () => { it("enables network family auto-selection for pinned lookups", () => { - const lookup = vi.fn(); + const lookup = vi.fn() as unknown as PinnedHostname["lookup"]; const pinned: PinnedHostname = { hostname: "api.telegram.org", addresses: ["149.154.167.220"], diff --git a/src/shared/net/ip.ts b/src/shared/net/ip.ts index 91c71fdad88..21770a20e29 100644 --- a/src/shared/net/ip.ts +++ b/src/shared/net/ip.ts @@ -28,6 +28,7 @@ const PRIVATE_OR_LOOPBACK_IPV6_RANGES = new Set([ "linkLocal", "uniqueLocal", ]); +const RFC2544_BENCHMARK_PREFIX: [ipaddr.IPv4, number] = [ipaddr.IPv4.parse("198.18.0.0"), 15]; const EMBEDDED_IPV4_SENTINEL_RULES: Array<{ matches: (parts: number[]) => boolean; @@ -79,6 +80,32 @@ function stripIpv6Brackets(value: string): string { return value; } +function isNumericIpv4LiteralPart(value: string): boolean { + return /^[0-9]+$/.test(value) || /^0x[0-9a-f]+$/i.test(value); +} + +function parseIpv6WithEmbeddedIpv4(raw: string): ipaddr.IPv6 | undefined { + if (!raw.includes(":") || !raw.includes(".")) { + return undefined; + } + const match = /^(.*:)([^:%]+(?:\.[^:%]+){3})(%[0-9A-Za-z]+)?$/i.exec(raw); + if (!match) { + return undefined; + } + const [, prefix, embeddedIpv4, zoneSuffix = ""] = match; + if (!ipaddr.IPv4.isValidFourPartDecimal(embeddedIpv4)) { + return undefined; + } + const octets = embeddedIpv4.split(".").map((part) => Number.parseInt(part, 10)); + const high = ((octets[0] << 8) | octets[1]).toString(16); + const low = ((octets[2] << 8) | octets[3]).toString(16); + const normalizedIpv6 = `${prefix}${high}:${low}${zoneSuffix}`; + if (!ipaddr.IPv6.isValid(normalizedIpv6)) { + return undefined; + } + return ipaddr.IPv6.parse(normalizedIpv6); +} + export function isIpv4Address(address: ParsedIpAddress): address is ipaddr.IPv4 { return address.kind() === "ipv4"; } @@ -115,7 +142,7 @@ export function parseCanonicalIpAddress(raw: string | undefined): ParsedIpAddres if (ipaddr.IPv6.isValid(normalized)) { return ipaddr.IPv6.parse(normalized); } - return undefined; + return parseIpv6WithEmbeddedIpv4(normalized); } export function parseLooseIpAddress(raw: string | undefined): ParsedIpAddress | undefined { @@ -127,10 +154,10 @@ export function parseLooseIpAddress(raw: string | undefined): ParsedIpAddress | if (!normalized) { return undefined; } - if (!ipaddr.isValid(normalized)) { - return undefined; + if (ipaddr.isValid(normalized)) { + return ipaddr.parse(normalized); } - return ipaddr.parse(normalized); + return parseIpv6WithEmbeddedIpv4(normalized); } export function normalizeIpAddress(raw: string | undefined): string | undefined { @@ -163,7 +190,20 @@ export function isLegacyIpv4Literal(raw: string | undefined): boolean { if (!normalized || normalized.includes(":")) { return false; } - return ipaddr.IPv4.isValid(normalized) && !ipaddr.IPv4.isValidFourPartDecimal(normalized); + if (isCanonicalDottedDecimalIPv4(normalized)) { + return false; + } + const parts = normalized.split("."); + if (parts.length === 0 || parts.length > 4) { + return false; + } + if (parts.some((part) => part.length === 0)) { + return false; + } + if (!parts.every((part) => isNumericIpv4LiteralPart(part))) { + return false; + } + return true; } export function isLoopbackIpAddress(raw: string | undefined): boolean { @@ -208,7 +248,9 @@ export function isCarrierGradeNatIpv4Address(raw: string | undefined): boolean { } export function isBlockedSpecialUseIpv4Address(address: ipaddr.IPv4): boolean { - return BLOCKED_IPV4_SPECIAL_USE_RANGES.has(address.range()); + return ( + BLOCKED_IPV4_SPECIAL_USE_RANGES.has(address.range()) || address.match(RFC2544_BENCHMARK_PREFIX) + ); } function decodeIpv4FromHextets(high: number, low: number): ipaddr.IPv4 { @@ -276,7 +318,13 @@ export function isIpInCidr(ip: string, cidr: string): boolean { return false; } try { - return comparableIp.match([comparableBase, prefixLength]); + if (isIpv4Address(comparableIp) && isIpv4Address(comparableBase)) { + return comparableIp.match([comparableBase, prefixLength]); + } + if (isIpv6Address(comparableIp) && isIpv6Address(comparableBase)) { + return comparableIp.match([comparableBase, prefixLength]); + } + return false; } catch { return false; } diff --git a/src/telegram/monitor.test.ts b/src/telegram/monitor.test.ts index 5a1899b3ab6..49fbcc13155 100644 --- a/src/telegram/monitor.test.ts +++ b/src/telegram/monitor.test.ts @@ -28,7 +28,7 @@ const { initSpy, runSpy, loadConfig } = vi.hoisted(() => ({ runSpy: vi.fn(() => ({ task: () => Promise.resolve(), stop: vi.fn(), - isRunning: () => false, + isRunning: (): boolean => false, })), loadConfig: vi.fn(() => ({ agents: { defaults: { maxConcurrent: 2 } }, @@ -214,10 +214,12 @@ describe("monitorTelegramProvider (grammY)", () => { .mockImplementationOnce(() => ({ task: () => Promise.reject(networkError), stop: vi.fn(), + isRunning: (): boolean => false, })) .mockImplementationOnce(() => ({ task: () => Promise.resolve(), stop: vi.fn(), + isRunning: (): boolean => false, })); await monitorTelegramProvider({ token: "tok" }); @@ -331,6 +333,7 @@ describe("monitorTelegramProvider (grammY)", () => { runSpy.mockImplementationOnce(() => ({ task: () => Promise.reject(new Error("bad token")), stop: vi.fn(), + isRunning: (): boolean => false, })); await expect(monitorTelegramProvider({ token: "tok" })).rejects.toThrow("bad token"); diff --git a/src/telegram/monitor.ts b/src/telegram/monitor.ts index 17cc864b5d6..54a24c96ff5 100644 --- a/src/telegram/monitor.ts +++ b/src/telegram/monitor.ts @@ -167,13 +167,14 @@ export async function monitorTelegramProvider(opts: MonitorTelegramOpts = {}) { abortSignal: opts.abortSignal, publicUrl: opts.webhookUrl, }); - if (opts.abortSignal && !opts.abortSignal.aborted) { + const abortSignal = opts.abortSignal; + if (abortSignal && !abortSignal.aborted) { await new Promise((resolve) => { const onAbort = () => { - opts.abortSignal?.removeEventListener("abort", onAbort); + abortSignal.removeEventListener("abort", onAbort); resolve(); }; - opts.abortSignal.addEventListener("abort", onAbort, { once: true }); + abortSignal.addEventListener("abort", onAbort, { once: true }); }); } return;