From e24c2cf5328af017f10d42ccfadb2e03d2c5aba2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 30 May 2026 13:23:50 -0700 Subject: [PATCH] fix: drop executable grab urls --- src/main/browser/browser-grab-payload.test.ts | 26 +++++++++++++++++++ src/main/browser/browser-grab-payload.ts | 8 ++++++ src/main/browser/grab-guest-script.test.ts | 7 +++++ src/main/browser/grab-guest-script.ts | 8 ++++++ 4 files changed, 49 insertions(+) diff --git a/src/main/browser/browser-grab-payload.test.ts b/src/main/browser/browser-grab-payload.test.ts index 390e35056fa..575d67375a9 100644 --- a/src/main/browser/browser-grab-payload.test.ts +++ b/src/main/browser/browser-grab-payload.test.ts @@ -122,4 +122,30 @@ describe('clampGrabPayload', () => { expect(payload?.target.reactComponents?.length).toBeLessThanOrEqual(512) expect(payload?.target.sourceFile).toBe('src/Button.tsx:12:4') }) + + it('drops executable and embedded URL schemes from page and attribute URLs', () => { + const payload = clampGrabPayload( + makeRawPayload({ + page: { + ...(makeRawPayload().page as Record), + sanitizedUrl: 'javascript:alert(1)' + }, + target: { + ...(makeRawPayload().target as Record), + attributes: { + href: 'javascript:alert(1)', + src: 'data:text/html,', + action: 'vbscript:msgbox(1)', + title: 'Safe label' + } + } + }) + ) + + expect(payload?.page.sanitizedUrl).toBe('') + expect(payload?.target.attributes.href).toBe('') + expect(payload?.target.attributes.src).toBe('') + expect(payload?.target.attributes.action).toBe('') + expect(payload?.target.attributes.title).toBe('Safe label') + }) }) diff --git a/src/main/browser/browser-grab-payload.ts b/src/main/browser/browser-grab-payload.ts index 46815ed8008..8c92670efde 100644 --- a/src/main/browser/browser-grab-payload.ts +++ b/src/main/browser/browser-grab-payload.ts @@ -5,6 +5,8 @@ import { GRAB_SECRET_PATTERNS } from '../../shared/browser-grab-types' +const SAFE_GRAB_URL_PROTOCOLS = new Set(['http:', 'https:', 'file:']) + /** * Re-validate and clamp all string, array, and budget fields in a grab payload * before forwarding to the renderer. This is the main-side safety net: even if @@ -66,6 +68,12 @@ export function clampGrabPayload(raw: unknown): BrowserGrabPayload | null { } try { const url = new URL(str) + if (url.protocol === 'about:') { + return url.toString() === 'about:blank' ? 'about:blank' : '' + } + if (!SAFE_GRAB_URL_PROTOCOLS.has(url.protocol)) { + return '' + } url.search = '' url.hash = '' return url.toString() diff --git a/src/main/browser/grab-guest-script.test.ts b/src/main/browser/grab-guest-script.test.ts index e8705c0edd8..4455b1d0420 100644 --- a/src/main/browser/grab-guest-script.test.ts +++ b/src/main/browser/grab-guest-script.test.ts @@ -121,6 +121,13 @@ describe('buildGuestOverlayScript', () => { expect(script).toContain("return '';") }) + it('arm script rejects executable and embedded URL schemes', () => { + const script = buildGuestOverlayScript('arm') + + expect(script).toContain('SAFE_URL_PROTOCOLS') + expect(script).toContain('!SAFE_URL_PROTOCOLS.has(u.protocol)') + }) + it('arm script slices text nodes before normalizing bounded text', () => { const script = buildGuestOverlayScript('arm') diff --git a/src/main/browser/grab-guest-script.ts b/src/main/browser/grab-guest-script.ts index 9817bc187ac..a07f4b70892 100644 --- a/src/main/browser/grab-guest-script.ts +++ b/src/main/browser/grab-guest-script.ts @@ -93,6 +93,8 @@ const ARM_SCRIPT = `(function() { 'secret', 'password', 'passwd' ]; + var SAFE_URL_PROTOCOLS = new Set(['http:', 'https:', 'file:']); + var STYLE_PROPS = [ 'display', 'position', 'width', 'height', 'margin', 'padding', 'color', 'backgroundColor', 'border', 'borderRadius', 'fontFamily', @@ -118,6 +120,12 @@ const ARM_SCRIPT = `(function() { function sanitizeUrl(url) { try { var u = new URL(url); + if (u.protocol === 'about:') { + return u.toString() === 'about:blank' ? 'about:blank' : ''; + } + if (!SAFE_URL_PROTOCOLS.has(u.protocol)) { + return ''; + } u.search = ''; u.hash = ''; return u.toString();