Repository navigation
fix(genie-ui): defeat DNS rebinding on the ws PTY trust boundary - #2621
Conversation
…inding verifyClient trusted new URL(origin).host === host unconditionally. both Origin and Host are browser-set, so DNS rebinding (evil.com -> 127.0.0.1) presents Origin === Host === evil.com:PORT and passed the check, opening MSG.INPUT -> proc.write() into a live login shell (RCE). the same-origin branch now additionally requires the real Host hostname to be a loopback identity; non-loopback hosts fall through to the existing GENIE_UI_ALLOWED_ORIGINS allowlist. adds a named rebinding regression test, inverts the old any-LAN-host test, and corrects the README trust-boundary claim. found by ultracode review of PR #2619.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
1 similar comment
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What
The genie-ui PTY WebSocket auth (
verifyClient) trusted its same-origin branch (new URL(origin).host === host) unconditionally. BothOriginandHostare browser-set, so a DNS-rebinding attack (evil.com → 127.0.0.1) presentsOrigin === Host === evil.com:PORTand passed the check — letting a malicious page sendMSG.INPUTframes that flowhandleClientMsg → manager.write → proc.write()into a live login shell (RCE as the operator). The loopback bind is no defense because the browser itself is the loopback client.Fix
The same-origin branch now additionally requires the real Host hostname to be a loopback identity (
localhost/127.0.0.1/::1). After a rebind the Host is stillevil.com(non-loopback), so it falls through to the existingGENIE_UI_ALLOWED_ORIGINSallowlist and is rejected. The allowlist escape hatch for legitimate LAN/remote browsers is unchanged.Tests
Origin === Host === evil.com→ rejected) — permanently owns the scenario.accepts any LAN hostnametest (which enshrined the hole) → non-loopback same-origin now rejected.[::1]loopback cases; retained cross-origin / mismatched-port / malformed / no-Origin cases.bun test packages/genie-ui/server/index.test.ts→ 10 pass / 0 fail. Biome + typecheck clean.Provenance
Found by an ultracode review of PR #2619 (dev→main promotion). One confirmed HIGH; adversarially verified (rebinding closed, escape hatch intact, no parsing bypass — IP-normalization tricks fail closed, credential/fragment/subdomain smuggling all rejected). Reviewed post-fix by an independent reviewer: SHIP.
Rated HIGH not CRITICAL because
genie-uiisprivate: true, unpublished, absent from the CLI dist bundle, and manually launched — vulnerable source shipped, but no always-on exposure. Landing before genie-ui is recommended to anyone or exposed as a product surface.