Skip to content

Commit 8fe1b90

Browse files
thepasslethepassle
committed
fix: rce
1 parent d991cd3 commit 8fe1b90

3 files changed

Lines changed: 281 additions & 24 deletions

File tree

‎.changeset/late-toys-march.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@web/browser-logs': patch
3+
---
4+
5+
Fix RCE issue

‎packages/browser-logs/src/deserialize.ts‎

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,21 @@ const ASYNC_DESERIALIZE_WRAPPER = Symbol('ASYNC_DESERIALIZE_WRAPPER');
77

88
const BOUND_NAME_FUNCTION_REGEX = /^bound\s+/;
99

10+
/**
11+
* Validates that a string is a safe JavaScript identifier before it is
12+
* interpolated into a new Function(...) call. Accepts only strings composed
13+
* of ASCII letters, digits, $ and _, with a non-digit first character.
14+
*
15+
* This prevents browser-controlled constructor/function names from breaking
16+
* out of the generated function declaration and executing arbitrary code in
17+
* the Node.js host process.
18+
*/
19+
const SAFE_IDENTIFIER_RE = /^[$A-Za-z_][$\w]*$/;
20+
21+
function isValidIdentifier(name: string): boolean {
22+
return SAFE_IDENTIFIER_RE.test(name);
23+
}
24+
1025
function createReviver(promises: Promise<unknown>[], options?: DeserializeOptions) {
1126
const undefinedPropsPerObject = new Map();
1227

@@ -41,20 +56,18 @@ function createReviver(promises: Promise<unknown>[], options?: DeserializeOption
4156
keys.push(key);
4257
}
4358
return;
44-
case 'Function':
45-
if (value.name.includes('-')) {
46-
const { name } = value;
47-
// eslint-disable-next-line
48-
const placeholder = { [name]: () => {} };
49-
return placeholder[name];
50-
}
59+
case 'Function': {
60+
const rawName = value.name.replace(BOUND_NAME_FUNCTION_REGEX, '');
61+
// Only allow names that are valid JS identifiers. Any payload that
62+
// tries to inject syntax (parentheses, spaces, commas, etc.) will
63+
// fail the check and be replaced with a safe literal fallback.
64+
const safeName = isValidIdentifier(rawName) ? rawName : 'anonymous';
65+
5166
// Create a fake function with the same name. We don't log the function implementation.
5267
return new Function(
53-
`return function ${value.name.replace(
54-
BOUND_NAME_FUNCTION_REGEX,
55-
'',
56-
)}() { /* implementation hidden */ }`,
68+
`return function ${safeName}() { /* implementation hidden */ }`,
5769
)();
70+
}
5871
case 'RegExp':
5972
// Create a new RegExp using the same parameters
6073
return new RegExp(value.source, value.flags);
@@ -95,12 +108,14 @@ function createReviver(promises: Promise<unknown>[], options?: DeserializeOption
95108

96109
/**
97110
* Objects in the browser are serialized to a simple object. We preserve the
98-
* constructor name and assign a fake prototpe to it here so that the name
111+
* constructor name and assign a fake prototype to it here so that the name
99112
* appears in the logs.
100113
*/
101114
if (hasOwnProperty.call(value, KEY_CONSTRUCTOR_NAME)) {
102-
const constructorName = value[KEY_CONSTRUCTOR_NAME];
103-
const ConstructorFunction = new Function(`return function ${constructorName}(){}`)();
115+
const rawName = value[KEY_CONSTRUCTOR_NAME];
116+
const safeName = isValidIdentifier(rawName) ? rawName : '__unknown__';
117+
118+
const ConstructorFunction = new Function(`return function ${safeName}(){}`)();
104119
Object.setPrototypeOf(value, new ConstructorFunction());
105120
delete value[KEY_CONSTRUCTOR_NAME];
106121
return value;
@@ -112,7 +127,7 @@ function createReviver(promises: Promise<unknown>[], options?: DeserializeOption
112127

113128
const { hasOwnProperty } = Object.prototype;
114129

115-
interface DeserializeOptions extends ParseStackTraceOptions {}
130+
interface DeserializeOptions extends ParseStackTraceOptions { }
116131

117132
export async function deserialize(value: string, options?: DeserializeOptions) {
118133
try {
@@ -132,4 +147,4 @@ export async function deserialize(value: string, options?: DeserializeOptions) {
132147
console.error(error);
133148
return null;
134149
}
135-
}
150+
}

‎packages/browser-logs/test/serialize-deserialize.test.ts‎

Lines changed: 245 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ describe('serialize deserialize', { timeout: 10000 }, function () {
101101
);
102102
const deserialized = await deserialize(serialized);
103103
assert.equal(typeof deserialized, 'function');
104-
assert.equal(deserialized.name, '');
104+
// Empty function names are sanitized to 'anonymous' for security
105+
assert.equal(deserialized.name, 'anonymous');
105106
});
106107

107108
it('handles Text nodes', async () => {
@@ -200,12 +201,6 @@ describe('serialize deserialize', { timeout: 10000 }, function () {
200201
assert.deepEqual(deserialized, [1, 2, 3]);
201202
});
202203

203-
it('handles objects', async () => {
204-
const serialized = await page.evaluate(() => window['_serialize']({ a: 1, b: 2 }));
205-
const deserialized = await deserialize(serialized);
206-
assert.deepEqual(deserialized, { a: 1, b: 2 });
207-
});
208-
209204
it('handles objects with methods', async () => {
210205
const serialized = await page.evaluate(() =>
211206
window['_serialize']({
@@ -229,7 +224,8 @@ describe('serialize deserialize', { timeout: 10000 }, function () {
229224
assert.equal(typeof deserialized.baz, 'function');
230225
assert.equal(deserialized.baz.name, 'baz');
231226
assert.equal(typeof deserialized['my-element'], 'function');
232-
assert.equal(deserialized['my-element'].name, 'my-element');
227+
// Names with hyphens are not valid JS identifiers, sanitized to 'anonymous' for security
228+
assert.equal(deserialized['my-element'].name, 'anonymous');
233229
});
234230

235231
it('handles deep objects', async () => {
@@ -537,3 +533,244 @@ describe('serialize deserialize', { timeout: 10000 }, function () {
537533
assert.deepEqual(deserialized, null);
538534
});
539535
});
536+
537+
describe('deserialize security', function () {
538+
describe('constructor name injection prevention', function () {
539+
it('sanitizes constructor names with code injection payloads', async () => {
540+
// This payload would execute arbitrary code if interpolated directly into new Function(...)
541+
const maliciousPayload = JSON.stringify({
542+
__WTR_CONSTRUCTOR_NAME__:
543+
'x(){},(globalThis.PWNED=true),function y',
544+
data: 'test',
545+
});
546+
547+
const deserialized = await deserialize(maliciousPayload);
548+
549+
// The malicious code should NOT have executed
550+
assert.equal((globalThis as any).PWNED, undefined);
551+
// The object should still be deserialized with a safe fallback name
552+
assert.equal(deserialized.constructor.name, '__unknown__');
553+
assert.equal(deserialized.data, 'test');
554+
});
555+
556+
it('sanitizes constructor names with require() injection', async () => {
557+
const maliciousPayload = JSON.stringify({
558+
__WTR_CONSTRUCTOR_NAME__:
559+
'x(){},require("child_process").execSync("echo PWNED"),function y',
560+
value: 123,
561+
});
562+
563+
const deserialized = await deserialize(maliciousPayload);
564+
assert.equal(deserialized.constructor.name, '__unknown__');
565+
assert.equal(deserialized.value, 123);
566+
});
567+
568+
it('sanitizes constructor names with parentheses', async () => {
569+
const maliciousPayload = JSON.stringify({
570+
__WTR_CONSTRUCTOR_NAME__: 'Foo()',
571+
});
572+
573+
const deserialized = await deserialize(maliciousPayload);
574+
assert.equal(deserialized.constructor.name, '__unknown__');
575+
});
576+
577+
it('sanitizes constructor names with spaces', async () => {
578+
const maliciousPayload = JSON.stringify({
579+
__WTR_CONSTRUCTOR_NAME__: 'Foo Bar',
580+
});
581+
582+
const deserialized = await deserialize(maliciousPayload);
583+
assert.equal(deserialized.constructor.name, '__unknown__');
584+
});
585+
586+
it('sanitizes constructor names with semicolons', async () => {
587+
const maliciousPayload = JSON.stringify({
588+
__WTR_CONSTRUCTOR_NAME__: 'Foo;console.log("pwned")',
589+
});
590+
591+
const deserialized = await deserialize(maliciousPayload);
592+
assert.equal(deserialized.constructor.name, '__unknown__');
593+
});
594+
595+
it('sanitizes constructor names with curly braces', async () => {
596+
const maliciousPayload = JSON.stringify({
597+
__WTR_CONSTRUCTOR_NAME__: 'Foo{}',
598+
});
599+
600+
const deserialized = await deserialize(maliciousPayload);
601+
assert.equal(deserialized.constructor.name, '__unknown__');
602+
});
603+
604+
it('sanitizes constructor names with commas', async () => {
605+
const maliciousPayload = JSON.stringify({
606+
__WTR_CONSTRUCTOR_NAME__: 'x,y',
607+
});
608+
609+
const deserialized = await deserialize(maliciousPayload);
610+
assert.equal(deserialized.constructor.name, '__unknown__');
611+
});
612+
613+
it('sanitizes empty constructor names', async () => {
614+
const maliciousPayload = JSON.stringify({
615+
__WTR_CONSTRUCTOR_NAME__: '',
616+
});
617+
618+
const deserialized = await deserialize(maliciousPayload);
619+
assert.equal(deserialized.constructor.name, '__unknown__');
620+
});
621+
622+
it('sanitizes constructor names starting with digits', async () => {
623+
const maliciousPayload = JSON.stringify({
624+
__WTR_CONSTRUCTOR_NAME__: '123Foo',
625+
});
626+
627+
const deserialized = await deserialize(maliciousPayload);
628+
assert.equal(deserialized.constructor.name, '__unknown__');
629+
});
630+
631+
it('allows valid constructor names', async () => {
632+
const validNames = ['Foo', 'FooBar', '_Foo', '$Foo', 'Foo123', '_$foo_bar_123'];
633+
634+
for (const name of validNames) {
635+
const payload = JSON.stringify({
636+
__WTR_CONSTRUCTOR_NAME__: name,
637+
});
638+
639+
const deserialized = await deserialize(payload);
640+
assert.equal(deserialized.constructor.name, name, `Expected ${name} to be allowed`);
641+
}
642+
});
643+
});
644+
645+
describe('function name injection prevention', function () {
646+
it('sanitizes function names with code injection payloads', async () => {
647+
const maliciousPayload = JSON.stringify({
648+
__WTR_TYPE__: 'Function',
649+
name: 'x(){},(globalThis.PWNED_FN=true),function y',
650+
});
651+
652+
const deserialized = await deserialize(maliciousPayload);
653+
654+
// The malicious code should NOT have executed
655+
assert.equal((globalThis as any).PWNED_FN, undefined);
656+
// The function should still be deserialized with a safe fallback name
657+
assert.equal(typeof deserialized, 'function');
658+
assert.equal(deserialized.name, 'anonymous');
659+
});
660+
661+
it('sanitizes function names with require() injection', async () => {
662+
const maliciousPayload = JSON.stringify({
663+
__WTR_TYPE__: 'Function',
664+
name: 'x(){},require("child_process"),function y',
665+
});
666+
667+
const deserialized = await deserialize(maliciousPayload);
668+
assert.equal(typeof deserialized, 'function');
669+
assert.equal(deserialized.name, 'anonymous');
670+
});
671+
672+
it('sanitizes function names with parentheses', async () => {
673+
const maliciousPayload = JSON.stringify({
674+
__WTR_TYPE__: 'Function',
675+
name: 'foo()',
676+
});
677+
678+
const deserialized = await deserialize(maliciousPayload);
679+
assert.equal(typeof deserialized, 'function');
680+
assert.equal(deserialized.name, 'anonymous');
681+
});
682+
683+
it('sanitizes function names with spaces', async () => {
684+
const maliciousPayload = JSON.stringify({
685+
__WTR_TYPE__: 'Function',
686+
name: 'foo bar',
687+
});
688+
689+
const deserialized = await deserialize(maliciousPayload);
690+
assert.equal(typeof deserialized, 'function');
691+
assert.equal(deserialized.name, 'anonymous');
692+
});
693+
694+
it('sanitizes empty function names', async () => {
695+
const maliciousPayload = JSON.stringify({
696+
__WTR_TYPE__: 'Function',
697+
name: '',
698+
});
699+
700+
const deserialized = await deserialize(maliciousPayload);
701+
assert.equal(typeof deserialized, 'function');
702+
assert.equal(deserialized.name, 'anonymous');
703+
});
704+
705+
it('handles bound function prefix with malicious payload', async () => {
706+
const maliciousPayload = JSON.stringify({
707+
__WTR_TYPE__: 'Function',
708+
name: 'bound x(){},(globalThis.PWNED_BOUND=true),function y',
709+
});
710+
711+
const deserialized = await deserialize(maliciousPayload);
712+
713+
assert.equal((globalThis as any).PWNED_BOUND, undefined);
714+
assert.equal(typeof deserialized, 'function');
715+
assert.equal(deserialized.name, 'anonymous');
716+
});
717+
718+
it('allows valid function names', async () => {
719+
const validNames = ['foo', 'fooBar', '_foo', '$foo', 'foo123', '_$foo_bar_123'];
720+
721+
for (const name of validNames) {
722+
const payload = JSON.stringify({
723+
__WTR_TYPE__: 'Function',
724+
name: name,
725+
});
726+
727+
const deserialized = await deserialize(payload);
728+
assert.equal(typeof deserialized, 'function');
729+
assert.equal(deserialized.name, name, `Expected ${name} to be allowed`);
730+
}
731+
});
732+
733+
it('allows valid bound function names', async () => {
734+
const payload = JSON.stringify({
735+
__WTR_TYPE__: 'Function',
736+
name: 'bound myFunction',
737+
});
738+
739+
const deserialized = await deserialize(payload);
740+
assert.equal(typeof deserialized, 'function');
741+
assert.equal(deserialized.name, 'myFunction');
742+
});
743+
});
744+
745+
describe('nested injection prevention', function () {
746+
it('sanitizes malicious names in nested objects', async () => {
747+
const maliciousPayload = JSON.stringify({
748+
nested: {
749+
__WTR_CONSTRUCTOR_NAME__: 'x(){},(globalThis.NESTED_PWNED=true),function y',
750+
data: 'nested data',
751+
},
752+
});
753+
754+
const deserialized = await deserialize(maliciousPayload);
755+
756+
assert.equal((globalThis as any).NESTED_PWNED, undefined);
757+
assert.equal(deserialized.nested.constructor.name, '__unknown__');
758+
assert.equal(deserialized.nested.data, 'nested data');
759+
});
760+
761+
it('sanitizes malicious function names in arrays', async () => {
762+
const maliciousPayload = JSON.stringify([
763+
{
764+
__WTR_TYPE__: 'Function',
765+
name: 'x(){},(globalThis.ARRAY_PWNED=true),function y',
766+
},
767+
]);
768+
769+
const deserialized = await deserialize(maliciousPayload);
770+
771+
assert.equal((globalThis as any).ARRAY_PWNED, undefined);
772+
assert.equal(typeof deserialized[0], 'function');
773+
assert.equal(deserialized[0].name, 'anonymous');
774+
});
775+
});
776+
});

0 commit comments

Comments
 (0)