From 319d967a06126a1640f7630af749a9e232b719e3 Mon Sep 17 00:00:00 2001 From: Omkar Chebale Date: Sat, 3 Oct 2026 00:32:12 +0530 Subject: [PATCH] Refuse a malformed IP range in an egress rule instead of reading it as /0 parseCidr read an empty prefix as 0 ("10.0.0.5/" allowed every IPv4 address), took hex and extra segments, and accepted IPv6 zone ids that BlockList then threw on inside the filter. Parse the prefix as 1-3 decimal digits, refuse extra segments and zone ids. --- CHANGELOG.md | 8 ++++++++ agent-computer/src/egress.ts | 6 +++++- agent-computer/tests/egress-policy.test.ts | 11 +++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9febb9a50..344484d83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A malformed IP range in an egress rule is refused instead of widening the rule + +An egress `cidr` rule written `10.0.0.5/` was stored as `10.0.0.5/0`, which is every IPv4 +address, so a typo for one host opened an allow-list to all of them. `/0x8` and `10.0.0.0/8/9` were +accepted the same way. A zone id such as `fe80::1%eth0` was accepted too, and then threw from the +filter on the first connection that policy judged. Each is now refused when the rule is saved, with +the sentence a malformed range already got. A rule like this saved earlier matches nothing. + ### A playground component's Published switch publishes its source too The Published switch on an admin component page called the generic publication endpoint for every diff --git a/agent-computer/src/egress.ts b/agent-computer/src/egress.ts index 7fff9bc3c..14a961a26 100644 --- a/agent-computer/src/egress.ts +++ b/agent-computer/src/egress.ts @@ -238,7 +238,11 @@ function normalizeHost(host: string): string { function parseCidr( value: string, ): { address: string; prefix: number; family: "ipv4" | "ipv6" } | null { - const [address = "", prefixText] = value.trim().split("/"); + const [address = "", prefixText, ...rest] = value.trim().split("/"); + // `Number` reads "" as 0 and takes "0x8", so `10.0.0.5/` would allow every address. A zone id + // passes `isIP` but `BlockList` throws on it when the rule is first used. + if (rest.length > 0 || address.includes("%")) return null; + if (prefixText !== undefined && !/^\d{1,3}$/.test(prefixText)) return null; const version = isIP(address); if (version === 0) return null; const max = version === 4 ? 32 : 128; diff --git a/agent-computer/tests/egress-policy.test.ts b/agent-computer/tests/egress-policy.test.ts index ed3c75c04..d5baa4bcb 100644 --- a/agent-computer/tests/egress-policy.test.ts +++ b/agent-computer/tests/egress-policy.test.ts @@ -102,6 +102,17 @@ describe("the network policy rules", () => { .ok, ).toBe(false); expect(parseEgressPolicy({ mode: "sometimes", rules: [] }).ok).toBe(false); + // Each of these once read as some other range: "" and "0x0" as /0, which is every address. + for (const value of [ + "10.0.0.5/", + "10.0.0.0/0x8", + "10.0.0.0/8/9", + "10.0.0.0/ 8", + "fe80::1%eth0", + "fe80::%eth0/64", + ]) { + expect(parseEgressRules([{ type: "cidr", value }]).ok).toBe(false); + } expect( parseEgressRules([ { type: "domain", value: "Example.COM" },