diff --git a/CHANGELOG.md b/CHANGELOG.md index ee4cdc59c20..d5f8382dd5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ Docs: https://docs.openclaw.ai ### Fixes -- Security/Net: harden SSRF IPv4 literal parsing to block octal/hex/short/packed legacy forms (for example `0177.0.0.1`, `127.1`, `2130706433`) in pre-DNS guard checks. +- Security/Net: enforce strict dotted-decimal IPv4 literals in SSRF checks and fail closed on unsupported legacy forms (octal/hex/short/packed, for example `0177.0.0.1`, `127.1`, `2130706433`) before DNS lookup. - Security/Discord: enforce trusted-sender guild permission checks for moderation actions (`timeout`, `kick`, `ban`) and ignore untrusted `senderUserId` params to prevent privilege escalation in tool-driven flows. Thanks @aether-ai-agent for reporting. - Security/ACP: harden ACP bridge session management with duplicate-session refresh, idle-session reaping, oldest-idle soft-cap eviction, and burst rate limiting on session creation to reduce local DoS risk without disrupting normal IDE usage. - Security/Plugins/Hooks: add optional `--pin` for npm plugin/hook installs, persist resolved npm metadata (`name`, `version`, `spec`, integrity, shasum, timestamp), warn/confirm on integrity drift during updates, and extend `openclaw security audit` to flag unpinned specs, missing integrity metadata, and install-record version drift. diff --git a/src/infra/net/fetch-guard.ssrf.test.ts b/src/infra/net/fetch-guard.ssrf.test.ts index fe6e60a59df..9971f07b981 100644 --- a/src/infra/net/fetch-guard.ssrf.test.ts +++ b/src/infra/net/fetch-guard.ssrf.test.ts @@ -33,6 +33,17 @@ describe("fetchWithSsrFGuard hardening", () => { expect(fetchImpl).not.toHaveBeenCalled(); }); + it("blocks unsupported packed-hex loopback literal URLs before fetch", async () => { + const fetchImpl = vi.fn(); + await expect( + fetchWithSsrFGuard({ + url: "http://0x7f000001/internal", + fetchImpl, + }), + ).rejects.toThrow(/private|internal|blocked/i); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + it("blocks redirect chains that hop to private hosts", async () => { const lookupFn = vi.fn(async () => [ { address: "93.184.216.34", family: 4 }, diff --git a/src/infra/net/ssrf.pinning.test.ts b/src/infra/net/ssrf.pinning.test.ts index 14ee88bc49e..b9e04df3c8a 100644 --- a/src/infra/net/ssrf.pinning.test.ts +++ b/src/infra/net/ssrf.pinning.test.ts @@ -133,6 +133,17 @@ describe("ssrf pinning", () => { expect(lookup).not.toHaveBeenCalled(); }); + it("blocks unsupported short-form IPv4 literals before DNS lookup", async () => { + const lookup = vi.fn(async () => [ + { address: "93.184.216.34", family: 4 }, + ]) as unknown as LookupFn; + + await expect(resolvePinnedHostnameWithPolicy("8.8.2056", { lookupFn: lookup })).rejects.toThrow( + SsrFBlockedError, + ); + expect(lookup).not.toHaveBeenCalled(); + }); + it("allows ISATAP embedded private IPv4 when private network is explicitly enabled", async () => { const lookup = vi.fn(async () => [ { address: "2001:db8:1234::5efe:127.0.0.1", family: 6 }, diff --git a/src/infra/net/ssrf.test.ts b/src/infra/net/ssrf.test.ts index f2f571c3708..716cc21ebca 100644 --- a/src/infra/net/ssrf.test.ts +++ b/src/infra/net/ssrf.test.ts @@ -26,12 +26,6 @@ const privateIpCases = [ "fec0::1", "2001:db8:1234::5efe:127.0.0.1", "2001:db8:1234:1:200:5efe:7f00:1", - "0177.0.0.1", - "0x7f.0.0.1", - "127.1", - "2130706433", - "0x7f000001", - "017700000001", ]; const publicIpCases = [ @@ -44,12 +38,25 @@ const publicIpCases = [ "2001:0000:0:0:0:0:f7f7:f7f7", "2001:db8:1234::5efe:8.8.8.8", "2001:db8:1234:1:1111:5efe:7f00:1", - "8.8.2056", - "0x08080808", ]; const malformedIpv6Cases = ["::::", "2001:db8::gggg"]; -const malformedIpv4Cases = ["08.0.0.1", "0x7g.0.0.1", "127.0.0.1.", "127..0.1"]; +const unsupportedLegacyIpv4Cases = [ + "0177.0.0.1", + "0x7f.0.0.1", + "127.1", + "2130706433", + "0x7f000001", + "017700000001", + "8.8.2056", + "0x08080808", + "08.0.0.1", + "0x7g.0.0.1", + "127..0.1", + "999.1.1.1", +]; + +const nonIpHostnameCases = ["example.com", "abc.123.example", "1password.com", "0x.example.com"]; describe("ssrf ip classification", () => { it.each(privateIpCases)("classifies %s as private", (address) => { @@ -64,8 +71,15 @@ describe("ssrf ip classification", () => { expect(isPrivateIpAddress(address)).toBe(true); }); - it.each(malformedIpv4Cases)("treats malformed IPv4 literal %s as non-IP", (address) => { - expect(isPrivateIpAddress(address)).toBe(false); + it.each(unsupportedLegacyIpv4Cases)( + "fails closed for unsupported legacy IPv4 literal %s", + (address) => { + expect(isPrivateIpAddress(address)).toBe(true); + }, + ); + + it.each(nonIpHostnameCases)("does not treat hostname %s as an IP literal", (hostname) => { + expect(isPrivateIpAddress(hostname)).toBe(false); }); }); @@ -90,6 +104,7 @@ describe("isBlockedHostnameOrIp", () => { it("blocks legacy IPv4 literal representations", () => { expect(isBlockedHostnameOrIp("0177.0.0.1")).toBe(true); + expect(isBlockedHostnameOrIp("8.8.2056")).toBe(true); expect(isBlockedHostnameOrIp("127.1")).toBe(true); expect(isBlockedHostnameOrIp("2130706433")).toBe(true); }); diff --git a/src/infra/net/ssrf.ts b/src/infra/net/ssrf.ts index d3d621b2542..2cd7790883f 100644 --- a/src/infra/net/ssrf.ts +++ b/src/infra/net/ssrf.ts @@ -68,76 +68,77 @@ function matchesHostnameAllowlist(hostname: string, allowlist: string[]): boolea return allowlist.some((pattern) => isHostnameAllowedByPattern(hostname, pattern)); } +function parseStrictIpv4Octet(part: string): number | null { + if (!/^[0-9]+$/.test(part)) { + return null; + } + const value = Number.parseInt(part, 10); + if (Number.isNaN(value) || value < 0 || value > 255) { + return null; + } + // Accept only canonical decimal octets (no leading zeros, no alternate radices). + if (part !== String(value)) { + return null; + } + return value; +} + function parseIpv4(address: string): number[] | null { const parts = address.split("."); - if (parts.length < 1 || parts.length > 4) { + if (parts.length !== 4) { return null; } - - const numbers: number[] = []; for (const part of parts) { - if (!part) { + if (parseStrictIpv4Octet(part) === null) { return null; } - const lower = part.toLowerCase(); - let value: number; - if (lower.startsWith("0x")) { - const hex = lower.slice(2); - if (!hex || !/^[0-9a-f]+$/i.test(hex)) { - return null; - } - value = Number.parseInt(hex, 16); - } else if (part.length > 1 && part.startsWith("0")) { - const octal = part.slice(1); - if (!/^[0-7]+$/.test(octal)) { - return null; - } - value = Number.parseInt(octal, 8); - } else { - if (!/^[0-9]+$/.test(part)) { - return null; - } - value = Number.parseInt(part, 10); - } - if (!Number.isFinite(value) || value < 0) { - return null; - } - numbers.push(value); + } + return parts.map((part) => Number.parseInt(part, 10)); +} + +function classifyIpv4Part(part: string): "decimal" | "hex" | "invalid-hex" | "non-numeric" { + if (/^0x[0-9a-f]+$/i.test(part)) { + return "hex"; + } + if (/^0x/i.test(part)) { + return "invalid-hex"; + } + if (/^[0-9]+$/.test(part)) { + return "decimal"; + } + return "non-numeric"; +} + +function isUnsupportedLegacyIpv4Literal(address: string): boolean { + const parts = address.split("."); + if (parts.length === 0 || parts.length > 4) { + return false; + } + if (parts.some((part) => part.length === 0)) { + return true; } - let ipv4Number: number; - if (numbers.length === 1) { - if (numbers[0] > 0xffffffff) { - return null; - } - ipv4Number = numbers[0]; - } else if (numbers.length === 2) { - if (numbers[0] > 0xff || numbers[1] > 0xffffff) { - return null; - } - ipv4Number = numbers[0] * 0x1000000 + numbers[1]; - } else if (numbers.length === 3) { - if (numbers[0] > 0xff || numbers[1] > 0xff || numbers[2] > 0xffff) { - return null; - } - ipv4Number = numbers[0] * 0x1000000 + numbers[1] * 0x10000 + numbers[2]; - } else { - if (numbers.some((value) => value > 0xff)) { - return null; - } - ipv4Number = numbers[0] * 0x1000000 + numbers[1] * 0x10000 + numbers[2] * 0x100 + numbers[3]; + const partKinds = parts.map(classifyIpv4Part); + if (partKinds.some((kind) => kind === "non-numeric")) { + return false; + } + if (partKinds.some((kind) => kind === "invalid-hex")) { + return true; } - if (!Number.isSafeInteger(ipv4Number) || ipv4Number < 0 || ipv4Number > 0xffffffff) { - return null; + if (parts.length !== 4) { + return true; } - - return [ - Math.floor(ipv4Number / 0x1000000) & 0xff, - Math.floor(ipv4Number / 0x10000) & 0xff, - Math.floor(ipv4Number / 0x100) & 0xff, - ipv4Number & 0xff, - ]; + for (const part of parts) { + if (/^0x/i.test(part)) { + return true; + } + const value = Number.parseInt(part, 10); + if (Number.isNaN(value) || value > 255 || part !== String(value)) { + return true; + } + } + return false; } function stripIpv6ZoneId(address: string): string { @@ -365,10 +366,14 @@ export function isPrivateIpAddress(address: string): boolean { } const ipv4 = parseIpv4(normalized); - if (!ipv4) { - return false; + if (ipv4) { + return isPrivateIpv4(ipv4); } - return isPrivateIpv4(ipv4); + // Reject non-canonical IPv4 literal forms (octal/hex/short/packed) by default. + if (isUnsupportedLegacyIpv4Literal(normalized)) { + return true; + } + return false; } export function isBlockedHostname(hostname: string): boolean { diff --git a/src/shared/net/ipv4.ts b/src/shared/net/ipv4.ts index 3c511823c77..0019a006791 100644 --- a/src/shared/net/ipv4.ts +++ b/src/shared/net/ipv4.ts @@ -1,4 +1,4 @@ -export function validateIPv4AddressInput(value: string | undefined): string | undefined { +export function validateDottedDecimalIPv4Input(value: string | undefined): string | undefined { if (!value) { return "IP address is required for custom bind mode"; } @@ -17,3 +17,8 @@ export function validateIPv4AddressInput(value: string | undefined): string | un } return "Invalid IPv4 address (each octet must be 0-255)"; } + +// Backward-compatible alias for callers using the old helper name. +export function validateIPv4AddressInput(value: string | undefined): string | undefined { + return validateDottedDecimalIPv4Input(value); +}