From 8fe30bb32b132b1fba010698014d7ddaed49901e Mon Sep 17 00:00:00 2001 From: Alex Nazaruk Date: Fri, 3 Jul 2026 00:37:10 +0200 Subject: [PATCH] fix(api): validate OAuth redirect + require non-empty state (session leak / login CSRF) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in the CoinPay OAuth flow (services/api/src/index.ts): 1. Open redirect / session-token exfiltration (HIGH). The 'redirect' query param was stored and, after login, the fresh session token was appended to it (`${redirect}#token=${sess}`) and the browser redirected there — with no validation. ?redirect=https://evil.com leaks the victim's session token to an attacker host. Add safeRedirect() (new redirect.ts) that only honors same-origin absolute URLs or site-relative paths; everything else falls back to the safe default. 2. OAuth state CSRF (MEDIUM). The callback checked `state !== cookie`, but a missing cookie AND missing state param both read as undefined, so `undefined !== undefined` is false and the check was bypassed. Require a non-empty state that matches the cookie. Adds redirect.test.ts (same-origin allowed; external / protocol-relative / javascript: / cross-scheme rejected). Logic verified standalone. --- services/api/src/index.ts | 12 ++++++++++-- services/api/src/redirect.test.ts | 32 +++++++++++++++++++++++++++++++ services/api/src/redirect.ts | 24 +++++++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 services/api/src/redirect.test.ts create mode 100644 services/api/src/redirect.ts diff --git a/services/api/src/index.ts b/services/api/src/index.ts index 03e367f..3a14fdf 100644 --- a/services/api/src/index.ts +++ b/services/api/src/index.ts @@ -12,6 +12,7 @@ import { sendEmail } from './email.js'; import { store } from './store/routes.js'; import { swarmRoutes } from './swarm.js'; import { dnsRoutes } from './dns.js'; +import { safeRedirect } from './redirect.js'; const CP = { clientId: process.env.COINPAY_CLIENT_ID || '', @@ -76,8 +77,15 @@ app.get('/api/auth/coinpay/login', (c) => { app.get('/api/auth/coinpay/callback', async (c) => { const code = c.req.query('code'); const state = c.req.query('state'); - if (!code || state !== getCookie(c, 'cp_state')) return c.text('invalid oauth state', 400); - const redirect = getCookie(c, 'cp_redirect') || ''; + const expectedState = getCookie(c, 'cp_state'); + // Require a non-empty state that matches the cookie. Without the `!state`/ + // `!expectedState` guards, a missing cookie AND missing state param both read + // as undefined, so `undefined !== undefined` is false and the CSRF check is + // silently bypassed (login CSRF). + if (!code || !state || !expectedState || state !== expectedState) { + return c.text('invalid oauth state', 400); + } + const redirect = safeRedirect(getCookie(c, 'cp_redirect'), APP_URL); const basic = Buffer.from(`${CP.clientId}:${CP.clientSecret}`).toString('base64'); const tokRes = await fetch(CP.tokenUrl, { diff --git a/services/api/src/redirect.test.ts b/services/api/src/redirect.test.ts new file mode 100644 index 0000000..f0ca18c --- /dev/null +++ b/services/api/src/redirect.test.ts @@ -0,0 +1,32 @@ +import { describe, it, expect } from 'vitest'; +import { safeRedirect } from './redirect.js'; + +const APP = 'https://tronbrowser.dev'; + +describe('safeRedirect', () => { + it('allows site-relative paths (resolved against our origin)', () => { + expect(safeRedirect('/dashboard', APP)).toBe('https://tronbrowser.dev/dashboard'); + expect(safeRedirect('/a/b?x=1', APP)).toBe('https://tronbrowser.dev/a/b?x=1'); + }); + + it('allows absolute same-origin URLs', () => { + expect(safeRedirect('https://tronbrowser.dev/x', APP)).toBe('https://tronbrowser.dev/x'); + }); + + it('rejects external hosts (prevents session-token exfiltration)', () => { + expect(safeRedirect('https://evil.com', APP)).toBeUndefined(); + expect(safeRedirect('https://tronbrowser.dev.evil.com/x', APP)).toBeUndefined(); + }); + + it('rejects protocol-relative, scheme and cross-scheme tricks', () => { + expect(safeRedirect('//evil.com', APP)).toBeUndefined(); + expect(safeRedirect('javascript:alert(1)', APP)).toBeUndefined(); + expect(safeRedirect('http://tronbrowser.dev/x', APP)).toBeUndefined(); // http != https origin + }); + + it('rejects empty / nullish input', () => { + expect(safeRedirect('', APP)).toBeUndefined(); + expect(safeRedirect(undefined, APP)).toBeUndefined(); + expect(safeRedirect(null, APP)).toBeUndefined(); + }); +}); diff --git a/services/api/src/redirect.ts b/services/api/src/redirect.ts new file mode 100644 index 0000000..8fc7e52 --- /dev/null +++ b/services/api/src/redirect.ts @@ -0,0 +1,24 @@ +/** + * Validate a post-login redirect target. + * + * The OAuth/login flow appends the freshly minted session token to the + * redirect URL's fragment (`${redirect}#token=...`), so an unvalidated + * redirect (`?redirect=https://evil.com`) lets an attacker exfiltrate the + * victim's session token. Only honor targets that stay on our own origin — + * an absolute same-origin URL or a site-relative path; everything else + * (external hosts, protocol-relative `//evil.com`, `javascript:` URLs, + * unparseable input) is rejected so the caller falls back to a safe default. + */ +export function safeRedirect( + redirect: string | undefined | null, + appUrl: string, +): string | undefined { + if (!redirect) return undefined; + try { + const target = new URL(redirect, appUrl); + if (target.origin === new URL(appUrl).origin) return target.toString(); + } catch { + /* not a parseable URL — reject */ + } + return undefined; +}