diff --git a/src/web/public/webview-tabs.js b/src/web/public/webview-tabs.js index a232f794..98d40188 100644 --- a/src/web/public/webview-tabs.js +++ b/src/web/public/webview-tabs.js @@ -264,10 +264,17 @@ Object.assign(CodemanApp.prototype, { if (recent.length >= 5) return; recent.push(now); this._webviewRecoveries.set(id, recent); - // Path only, never an origin: a `//host/x` here would jump the frame off - // the proxy (resolveUpstreamUrl refuses it server-side as well). - const path = data.path.replace(/^\/+/, '/'); - void this.openWebview(id, { path: path.startsWith('/') && !path.startsWith('//') ? path : '/' }); + // Path only, never an origin. Three spellings would resolve to a foreign + // origin (in direct mode `new URL(path, src)` is the frame's src, so the + // frame would remount there): the protocol-relative `//host/x`; a + // backslash, which the WHATWG parser treats as `/` for http(s), so + // `/\host/x` too; and an ASCII tab or newline, which the parser deletes + // before it looks at anything, so `//host/x` IS `//host/x` by the time + // it resolves. Drop the invisible ones, collapse the leading separators to + // one `/`, and refuse whatever still opens a second one. The proxied form + // is refused server-side as well (resolveUpstreamUrl). + const path = data.path.replace(/[\t\n\r]/g, '').replace(/^[/\\]+/, '/'); + void this.openWebview(id, { path: path.startsWith('/') && !/^\/[/\\]/.test(path) ? path : '/' }); return; } }; diff --git a/test/webview-loopback-links.test.ts b/test/webview-loopback-links.test.ts index 7c29d43e..0208be9e 100644 --- a/test/webview-loopback-links.test.ts +++ b/test/webview-loopback-links.test.ts @@ -385,4 +385,31 @@ describe('lost-frame recovery', () => { expect(opens).toBeLessThanOrEqual(5); expect(opens).toBeGreaterThan(0); }); + + /** + * The proxied form above cannot leave the prefix whatever the path says. A + * DIRECT-mode tab is the reachable case: `POST /open` returns no embedUrl, so + * the recovered path is resolved with `new URL(path, src)` and becomes the + * frame's src. The WHATWG parser reads a backslash as `/` for http(s), and + * deletes ASCII tab and newline before parsing, so `/\host/x` and `//host/x` + * both mean `//host/x` there: a page in such a tab could remount its own frame + * on a foreign origin. Not an escalation (the page can already navigate itself + * anywhere), but the handler promises "path only, never an origin". + */ + it('keeps a direct-mode frame on its own origin whatever separator the path opens with', async () => { + const { win, app } = boot(); + app._installWebviewLostListener(); + await app.openWebview('direct'); + expect(frameSrc(win, 'direct')).toBe('https://localhost:9443/'); + const attempts = ['/\\evil.example/x', '\\\\evil.example/x', '/\t/evil.example/x', '/\n/evil.example/x']; + for (const path of attempts) { + lost(win, frameOf(win, 'direct').contentWindow, path); + await vi.waitFor(() => expect(frameSrc(win, 'direct')).toBe('https://localhost:9443/evil.example/x')); + // Reset for the next spelling so the assertion above cannot pass on a stale + // src; through openWebview rather than another lost message, which is + // capped at five per minute per frame. + await app.openWebview('direct', { path: '/' }); + expect(frameSrc(win, 'direct')).toBe('https://localhost:9443/'); + } + }); });