From c37107d624b1e894fa257286044a49344fa59be1 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Thu, 1 Oct 2026 12:28:33 +0000 Subject: [PATCH 1/3] Strip unreadable and oddly-spaced candidates from the offer under Local networks The offer filter matched candidates with a single-space regex and split lines only on CRLF, so a candidate with a doubled space or tab before its address, or one after a bare LF or CR, kept its off-network address and reached the native stack, which parses it. Refs #873 --- docs/specs/remote-network.md | 2 +- lib/src/host/remote/local-networks.test.ts | 25 +++++++++++++++++ lib/src/host/remote/local-networks.ts | 31 +++++++++++++--------- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/docs/specs/remote-network.md b/docs/specs/remote-network.md index e1488f7df..41e98b988 100644 --- a/docs/specs/remote-network.md +++ b/docs/specs/remote-network.md @@ -39,7 +39,7 @@ Under `local` each one-time runtime is held to the networks allowed at its open; - **The attempt's UDP socket binds the one allowed address when exactly one is present**: a single interface holds every address in the allowed networks, loopback and link-local aside, and exactly one in its preferred family, IPv4 over IPv6. **Otherwise it listens on every interface** (`docs/specs/remote-security-model.md` -> "Direct path"), and the level restricts the path, not the listener. Chosen per attempt (rationale). - **Must strip every candidate outside the allowed networks from the Burrow's answer**, and send a default address outside them as `0.0.0.0`. **An answer left with no candidate refuses the attempt.** -- **Must strip the phone's offer the same way before the Burrow applies it**, a hostname or mDNS name included, so its ICE agent sends no check and makes no lookup toward an address the level does not hold. **An offer left with no candidate is still answered**: the phone's checks reach the answer's candidates, and the pair forms peer-reflexive (rationale). +- **Must strip the phone's offer the same way before the Burrow applies it**, a hostname, mDNS name, or unreadable candidate included, so its ICE agent sends no check and makes no lookup toward an address the level does not hold. **An offer left with no candidate is still answered**: the phone's checks reach the answer's candidates, and the pair forms peer-reflexive (rationale). - **Must check the selected candidate pair on the Burrow before its channel reports open**, and again while it is open and `connected` — on every ICE or connection state change, and every `DIRECT_PATH_RECHECK_MS` (rationale): both ends parse as IP addresses — IPv4-mapped IPv6 matching its IPv4 range — each inside an allowed CIDR. **A hostname, an mDNS name, or a pair the stack will not report refuses**, and a frame arriving before the open is checked first; once open, a reading with no pair is left to the connection's own state (rationale). - **Never trust SDP candidates, Hosted-observed addresses, or Client claims** as path evidence; only the Burrow's own ICE agent answers (rationale). - **A refusal is a violation**: it ends the connection `network-not-allowed` (`docs/specs/one-time.md` -> "Burrow runtime"), switched or not. diff --git a/lib/src/host/remote/local-networks.test.ts b/lib/src/host/remote/local-networks.test.ts index 7a98309f8..5237afb33 100644 --- a/lib/src/host/remote/local-networks.test.ts +++ b/lib/src/host/remote/local-networks.test.ts @@ -174,6 +174,31 @@ describe('localNetworksPath', () => { expect(candidatesIn(localNetworksPath(['10.0.0.0/8']).acceptRemote(offer))).toEqual([]); }); + it('strips an offer’s off-network candidate however its whitespace and line endings are written', () => { + // Each of these is a candidate toward 203.0.113.7 that the native stack still parses. + const offer = [ + 'v=0', + 'c=IN IP4\t203.0.113.7', + 'a=candidate:1 1 udp 2113937151 192.168.86.23 51234 typ host', + 'a=candidate:2 1 udp 2113937151 203.0.113.7 51236 typ host', + 'a=candidate:3 1 udp 2113937151\t203.0.113.7 51237 typ host', + 'a=candidate:4 1 udp 2113937151 192.168.86.23 51238 typ host\na=candidate:5 1 udp 2113937151 203.0.113.7 51239 typ host', + 'a=candidate:6 1 udp 2113937151 192.168.86.23 51240 typ host\ra=candidate:7 1 udp 2113937151 203.0.113.7 51241 typ host', + 'a=candidate:8 1 udp', + '', + ].join('\r\n'); + + const accepted = localNetworksPath(LAN).acceptRemote(offer); + expect(accepted).not.toContain('203.0.113.7'); + expect(accepted).not.toContain('candidate:8'); + expect(candidatesIn(accepted)).toEqual([ + 'a=candidate:1 1 udp 2113937151 192.168.86.23 51234 typ host', + 'a=candidate:4 1 udp 2113937151 192.168.86.23 51238 typ host', + 'a=candidate:6 1 udp 2113937151 192.168.86.23 51240 typ host', + ]); + expect(accepted).toContain('\r\nc=IN IP4 0.0.0.0\r\n'); + }); + it('allows a pair whose two ends are both on allowed networks', () => { const path = localNetworksPath([...LAN, ...TAILNET]); expect(path.refusal({ local: '192.168.86.160', remote: '192.168.86.23' })).toBeNull(); diff --git a/lib/src/host/remote/local-networks.ts b/lib/src/host/remote/local-networks.ts index b3ef624b5..42eab902e 100644 --- a/lib/src/host/remote/local-networks.ts +++ b/lib/src/host/remote/local-networks.ts @@ -34,26 +34,31 @@ export function bindAddressFor( return family.length === 1 ? family[0]! : null; } -/** `c=` and `a=candidate` lines, with the address each names. */ -const CONNECTION_LINE = /^c=IN IP[46] (\S+)$/; -const CANDIDATE_LINE = /^a=candidate:\S+ \S+ \S+ \S+ (\S+) /; - /** - * `sdp` with every candidate outside `inAllowed` removed and a default address - * outside it written as `0.0.0.0`, and how many candidates are left. + * `sdp` with every candidate outside `inAllowed`, or with no address to read, + * removed and a default address outside it written as `0.0.0.0`, and how many + * candidates are left. Lines end at any of CRLF, LF, or CR and fields at any + * whitespace, so no line the native parser reads as a candidate escapes this + * one; lines rejoin with the first ending found. */ function keepAllowed(sdp: string, inAllowed: (address: string) => boolean): { sdp: string; candidates: number } { - const eol = sdp.includes('\r\n') ? '\r\n' : '\n'; + const eol = /\r\n|\r|\n/.exec(sdp)?.[0] ?? '\n'; let candidates = 0; const lines: string[] = []; - for (const line of sdp.split(eol)) { - const candidate = CANDIDATE_LINE.exec(line); - if (candidate) { - if (!inAllowed(candidate[1]!)) continue; + for (const line of sdp.split(/\r\n|\r|\n/)) { + const fields = line.trim().split(/\s+/); + if (/^a=candidate:/i.test(fields[0]!)) { + const address = fields[4]; + if (!address || !inAllowed(address)) continue; candidates += 1; + } else if (/^c=/i.test(fields[0]!)) { + const address = fields[2]; + if (!address || !inAllowed(address)) { + lines.push('c=IN IP4 0.0.0.0'); + continue; + } } - const connection = CONNECTION_LINE.exec(line); - lines.push(connection && !inAllowed(connection[1]!) ? 'c=IN IP4 0.0.0.0' : line); + lines.push(line); } return { sdp: lines.join(eol), candidates }; } From 769fae20d5ed53fe1d88f443b1f9bc453f5f6e31 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Thu, 1 Oct 2026 12:33:41 +0000 Subject: [PATCH 2/3] fix: split candidate fields on the native parser's whitespace, not JavaScript's \s libdatachannel reads candidate fields with std::istringstream, which splits only on C-locale whitespace. JavaScript's \s also matches U+00A0 and other Unicode spaces, so a foundation carrying them shifted the field this filter read as the address away from the one the native stack applies. --- lib/src/host/remote/local-networks.test.ts | 2 ++ lib/src/host/remote/local-networks.ts | 13 +++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/src/host/remote/local-networks.test.ts b/lib/src/host/remote/local-networks.test.ts index 5237afb33..78b8134f2 100644 --- a/lib/src/host/remote/local-networks.test.ts +++ b/lib/src/host/remote/local-networks.test.ts @@ -185,6 +185,8 @@ describe('localNetworksPath', () => { 'a=candidate:4 1 udp 2113937151 192.168.86.23 51238 typ host\na=candidate:5 1 udp 2113937151 203.0.113.7 51239 typ host', 'a=candidate:6 1 udp 2113937151 192.168.86.23 51240 typ host\ra=candidate:7 1 udp 2113937151 203.0.113.7 51241 typ host', 'a=candidate:8 1 udp', + // U+00A0 is not whitespace to the native parser, which reads 203.0.113.7 as this address. + 'a=candidate:9\u00a0x\u00a0y\u00a0z\u00a0192.168.86.23 1 udp 2113937151 203.0.113.7 51242 typ host', '', ].join('\r\n'); diff --git a/lib/src/host/remote/local-networks.ts b/lib/src/host/remote/local-networks.ts index 42eab902e..cd66c2c18 100644 --- a/lib/src/host/remote/local-networks.ts +++ b/lib/src/host/remote/local-networks.ts @@ -37,21 +37,22 @@ export function bindAddressFor( /** * `sdp` with every candidate outside `inAllowed`, or with no address to read, * removed and a default address outside it written as `0.0.0.0`, and how many - * candidates are left. Lines end at any of CRLF, LF, or CR and fields at any - * whitespace, so no line the native parser reads as a candidate escapes this - * one; lines rejoin with the first ending found. + * candidates are left. Lines end at any of CRLF, LF, or CR and fields at the + * C-locale whitespace libdatachannel splits on, never JavaScript's wider `\s`, + * so no line the native parser reads as a candidate escapes this one; lines + * rejoin with the first ending found. */ function keepAllowed(sdp: string, inAllowed: (address: string) => boolean): { sdp: string; candidates: number } { const eol = /\r\n|\r|\n/.exec(sdp)?.[0] ?? '\n'; let candidates = 0; const lines: string[] = []; for (const line of sdp.split(/\r\n|\r|\n/)) { - const fields = line.trim().split(/\s+/); - if (/^a=candidate:/i.test(fields[0]!)) { + const fields = line.split(/[ \t\v\f]+/).filter((field) => field !== ''); + if (/^a=candidate:/i.test(fields[0] ?? '')) { const address = fields[4]; if (!address || !inAllowed(address)) continue; candidates += 1; - } else if (/^c=/i.test(fields[0]!)) { + } else if (/^c=/i.test(fields[0] ?? '')) { const address = fields[2]; if (!address || !inAllowed(address)) { lines.push('c=IN IP4 0.0.0.0'); From c05a5d56eac18a4e8215e39bebe17160139b12af Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Thu, 1 Oct 2026 14:10:10 -0700 Subject: [PATCH 3/3] Keep diagnostic documentation attached to its function --- lib/src/host/remote/local-networks.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/host/remote/local-networks.ts b/lib/src/host/remote/local-networks.ts index fa968b08d..a029ba5c5 100644 --- a/lib/src/host/remote/local-networks.ts +++ b/lib/src/host/remote/local-networks.ts @@ -83,13 +83,13 @@ const isPrivateAddress = allowedAddressTest([ 'fe80::/10', ]); +const CANDIDATE_LINE = /^a=candidate:\S+ \S+ \S+ \S+ (\S+) /; + /** * The first candidate of `sdp` that is an IP literal outside every private * range — a phone's server-reflexive candidate, most often — and outside * `skip`, or `null`. */ -const CANDIDATE_LINE = /^a=candidate:\S+ \S+ \S+ \S+ (\S+) /; - export function firstPublicCandidate(sdp: string, skip: (address: string) => boolean = () => false): string | null { for (const line of sdp.split(/\r?\n/)) { const address = CANDIDATE_LINE.exec(line)?.[1];