Skip to content

Commit 95b2d03

Browse files
fix(auth): reject control chars in redirect target
Browsers strip C0 control characters (TAB/LF/CR) from URLs before navigating, so `/\t/evil.example` and `/\n//evil.example` slipped past the slash checks and then normalised to a protocol-relative, off-origin URL. Reject any C0 control char (U+0000-U+001F) up front, before the slash checks. Add regression tests for the TAB/LF/CR variants. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a0b1e5c commit 95b2d03

2 files changed

Lines changed: 33 additions & 4 deletions

File tree

src/routes/auth/login/safe-redirect.ts

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,33 @@
66

77
export const DEFAULT_REDIRECT = '/portal/dashboard';
88

9+
/**
10+
* True if `value` contains any C0 control character (U+0000–U+001F, incl.
11+
* TAB/LF/CR). Browsers strip these from URLs before navigating, so a value
12+
* like `/\t/evil.example` or `/\n//evil.example` would slip past the slash
13+
* checks below and then normalise to a protocol-relative, off-origin URL.
14+
* Strip-then-parse is the classic open-redirect bypass, so we reject any such
15+
* value up front. (Expressed as a char-code scan to keep literal control
16+
* characters out of the source.)
17+
*/
18+
function hasControlChar(value: string): boolean {
19+
for (let i = 0; i < value.length; i++) {
20+
if (value.charCodeAt(i) <= 0x1f) return true;
21+
}
22+
return false;
23+
}
24+
925
/**
1026
* Return a safe post-login redirect target. A value is only accepted when it
1127
* begins with a single `/` (a same-origin absolute path). Everything else —
12-
* empty/missing values, absolute URLs (`https://…`), protocol-relative URLs
13-
* (`//host`) and the backslash variant browsers normalise to them (`/\host`)
14-
* — falls back to {@link DEFAULT_REDIRECT}.
28+
* empty/missing values, values containing control characters, absolute URLs
29+
* (`https://…`), protocol-relative URLs (`//host`) and the backslash variant
30+
* browsers normalise to them (`/\host`) — falls back to {@link DEFAULT_REDIRECT}.
1531
*/
1632
export function safeRedirect(target: string | null | undefined): string {
17-
if (!target || !target.startsWith('/')) return DEFAULT_REDIRECT;
33+
if (!target) return DEFAULT_REDIRECT;
34+
if (hasControlChar(target)) return DEFAULT_REDIRECT;
35+
if (!target.startsWith('/')) return DEFAULT_REDIRECT;
1836
// Reject protocol-relative URLs. Browsers treat `\` as `/`, so `/\host`
1937
// escapes the origin just like `//host` does.
2038
if (target[1] === '/' || target[1] === '\\') return DEFAULT_REDIRECT;

tests/unit/safe-redirect.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,15 @@ describe('safeRedirect', () => {
3232
expect(safeRedirect('/\\evil.example')).toBe(DEFAULT_REDIRECT);
3333
expect(safeRedirect('/\\/evil.example')).toBe(DEFAULT_REDIRECT);
3434
});
35+
36+
it('rejects control chars browsers strip before navigating (TAB/LF/CR)', () => {
37+
// Browsers drop these, so `/\t/evil.example` etc. would normalise to
38+
// `//evil.example` (off-origin) if they slipped past the slash checks.
39+
expect(safeRedirect('/\t/evil.example')).toBe(DEFAULT_REDIRECT);
40+
expect(safeRedirect('/\n/evil.example')).toBe(DEFAULT_REDIRECT);
41+
expect(safeRedirect('/\r/evil.example')).toBe(DEFAULT_REDIRECT);
42+
expect(safeRedirect('/\t\\evil.example')).toBe(DEFAULT_REDIRECT);
43+
expect(safeRedirect('\t//evil.example')).toBe(DEFAULT_REDIRECT);
44+
expect(safeRedirect('/foo\nbar')).toBe(DEFAULT_REDIRECT);
45+
});
3546
});

0 commit comments

Comments
 (0)