diff --git a/.changeset/late-toys-march.md b/.changeset/late-toys-march.md new file mode 100644 index 0000000000..ab52ddc59c --- /dev/null +++ b/.changeset/late-toys-march.md @@ -0,0 +1,5 @@ +--- +'@web/browser-logs': patch +--- + +Fix RCE issue diff --git a/packages/browser-logs/src/deserialize.ts b/packages/browser-logs/src/deserialize.ts index b483842ac7..ec39ae4508 100644 --- a/packages/browser-logs/src/deserialize.ts +++ b/packages/browser-logs/src/deserialize.ts @@ -7,6 +7,21 @@ const ASYNC_DESERIALIZE_WRAPPER = Symbol('ASYNC_DESERIALIZE_WRAPPER'); const BOUND_NAME_FUNCTION_REGEX = /^bound\s+/; +/** + * Validates that a string is a safe JavaScript identifier before it is + * interpolated into a new Function(...) call. Accepts only strings composed + * of ASCII letters, digits, $ and _, with a non-digit first character. + * + * This prevents browser-controlled constructor/function names from breaking + * out of the generated function declaration and executing arbitrary code in + * the Node.js host process. + */ +const SAFE_IDENTIFIER_RE = /^[$A-Za-z_][$\w]*$/; + +function isValidIdentifier(name: string): boolean { + return SAFE_IDENTIFIER_RE.test(name); +} + function createReviver(promises: Promise[], options?: DeserializeOptions) { const undefinedPropsPerObject = new Map(); @@ -41,20 +56,16 @@ function createReviver(promises: Promise[], options?: DeserializeOption keys.push(key); } return; - case 'Function': - if (value.name.includes('-')) { - const { name } = value; - // eslint-disable-next-line - const placeholder = { [name]: () => {} }; - return placeholder[name]; - } + case 'Function': { + const rawName = value.name.replace(BOUND_NAME_FUNCTION_REGEX, ''); + // Only allow names that are valid JS identifiers. Any payload that + // tries to inject syntax (parentheses, spaces, commas, etc.) will + // fail the check and be replaced with a safe literal fallback. + const safeName = isValidIdentifier(rawName) ? rawName : 'anonymous'; + // Create a fake function with the same name. We don't log the function implementation. - return new Function( - `return function ${value.name.replace( - BOUND_NAME_FUNCTION_REGEX, - '', - )}() { /* implementation hidden */ }`, - )(); + return new Function(`return function ${safeName}() { /* implementation hidden */ }`)(); + } case 'RegExp': // Create a new RegExp using the same parameters return new RegExp(value.source, value.flags); @@ -95,12 +106,14 @@ function createReviver(promises: Promise[], options?: DeserializeOption /** * Objects in the browser are serialized to a simple object. We preserve the - * constructor name and assign a fake prototpe to it here so that the name + * constructor name and assign a fake prototype to it here so that the name * appears in the logs. */ if (hasOwnProperty.call(value, KEY_CONSTRUCTOR_NAME)) { - const constructorName = value[KEY_CONSTRUCTOR_NAME]; - const ConstructorFunction = new Function(`return function ${constructorName}(){}`)(); + const rawName = value[KEY_CONSTRUCTOR_NAME]; + const safeName = isValidIdentifier(rawName) ? rawName : '__unknown__'; + + const ConstructorFunction = new Function(`return function ${safeName}(){}`)(); Object.setPrototypeOf(value, new ConstructorFunction()); delete value[KEY_CONSTRUCTOR_NAME]; return value; diff --git a/packages/browser-logs/test/serialize-deserialize.test.ts b/packages/browser-logs/test/serialize-deserialize.test.ts index e380cc0609..8bcaf05706 100644 --- a/packages/browser-logs/test/serialize-deserialize.test.ts +++ b/packages/browser-logs/test/serialize-deserialize.test.ts @@ -101,7 +101,8 @@ describe('serialize deserialize', { timeout: 10000 }, function () { ); const deserialized = await deserialize(serialized); assert.equal(typeof deserialized, 'function'); - assert.equal(deserialized.name, ''); + // Empty function names are sanitized to 'anonymous' for security + assert.equal(deserialized.name, 'anonymous'); }); it('handles Text nodes', async () => { @@ -200,12 +201,6 @@ describe('serialize deserialize', { timeout: 10000 }, function () { assert.deepEqual(deserialized, [1, 2, 3]); }); - it('handles objects', async () => { - const serialized = await page.evaluate(() => window['_serialize']({ a: 1, b: 2 })); - const deserialized = await deserialize(serialized); - assert.deepEqual(deserialized, { a: 1, b: 2 }); - }); - it('handles objects with methods', async () => { const serialized = await page.evaluate(() => window['_serialize']({ @@ -229,7 +224,8 @@ describe('serialize deserialize', { timeout: 10000 }, function () { assert.equal(typeof deserialized.baz, 'function'); assert.equal(deserialized.baz.name, 'baz'); assert.equal(typeof deserialized['my-element'], 'function'); - assert.equal(deserialized['my-element'].name, 'my-element'); + // Names with hyphens are not valid JS identifiers, sanitized to 'anonymous' for security + assert.equal(deserialized['my-element'].name, 'anonymous'); }); it('handles deep objects', async () => { @@ -537,3 +533,243 @@ describe('serialize deserialize', { timeout: 10000 }, function () { assert.deepEqual(deserialized, null); }); }); + +describe('deserialize security', function () { + describe('constructor name injection prevention', function () { + it('sanitizes constructor names with code injection payloads', async () => { + // This payload would execute arbitrary code if interpolated directly into new Function(...) + const maliciousPayload = JSON.stringify({ + __WTR_CONSTRUCTOR_NAME__: 'x(){},(globalThis.PWNED=true),function y', + data: 'test', + }); + + const deserialized = await deserialize(maliciousPayload); + + // The malicious code should NOT have executed + assert.equal((globalThis as any).PWNED, undefined); + // The object should still be deserialized with a safe fallback name + assert.equal(deserialized.constructor.name, '__unknown__'); + assert.equal(deserialized.data, 'test'); + }); + + it('sanitizes constructor names with require() injection', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_CONSTRUCTOR_NAME__: + 'x(){},require("child_process").execSync("echo PWNED"),function y', + value: 123, + }); + + const deserialized = await deserialize(maliciousPayload); + assert.equal(deserialized.constructor.name, '__unknown__'); + assert.equal(deserialized.value, 123); + }); + + it('sanitizes constructor names with parentheses', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_CONSTRUCTOR_NAME__: 'Foo()', + }); + + const deserialized = await deserialize(maliciousPayload); + assert.equal(deserialized.constructor.name, '__unknown__'); + }); + + it('sanitizes constructor names with spaces', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_CONSTRUCTOR_NAME__: 'Foo Bar', + }); + + const deserialized = await deserialize(maliciousPayload); + assert.equal(deserialized.constructor.name, '__unknown__'); + }); + + it('sanitizes constructor names with semicolons', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_CONSTRUCTOR_NAME__: 'Foo;console.log("pwned")', + }); + + const deserialized = await deserialize(maliciousPayload); + assert.equal(deserialized.constructor.name, '__unknown__'); + }); + + it('sanitizes constructor names with curly braces', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_CONSTRUCTOR_NAME__: 'Foo{}', + }); + + const deserialized = await deserialize(maliciousPayload); + assert.equal(deserialized.constructor.name, '__unknown__'); + }); + + it('sanitizes constructor names with commas', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_CONSTRUCTOR_NAME__: 'x,y', + }); + + const deserialized = await deserialize(maliciousPayload); + assert.equal(deserialized.constructor.name, '__unknown__'); + }); + + it('sanitizes empty constructor names', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_CONSTRUCTOR_NAME__: '', + }); + + const deserialized = await deserialize(maliciousPayload); + assert.equal(deserialized.constructor.name, '__unknown__'); + }); + + it('sanitizes constructor names starting with digits', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_CONSTRUCTOR_NAME__: '123Foo', + }); + + const deserialized = await deserialize(maliciousPayload); + assert.equal(deserialized.constructor.name, '__unknown__'); + }); + + it('allows valid constructor names', async () => { + const validNames = ['Foo', 'FooBar', '_Foo', '$Foo', 'Foo123', '_$foo_bar_123']; + + for (const name of validNames) { + const payload = JSON.stringify({ + __WTR_CONSTRUCTOR_NAME__: name, + }); + + const deserialized = await deserialize(payload); + assert.equal(deserialized.constructor.name, name, `Expected ${name} to be allowed`); + } + }); + }); + + describe('function name injection prevention', function () { + it('sanitizes function names with code injection payloads', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_TYPE__: 'Function', + name: 'x(){},(globalThis.PWNED_FN=true),function y', + }); + + const deserialized = await deserialize(maliciousPayload); + + // The malicious code should NOT have executed + assert.equal((globalThis as any).PWNED_FN, undefined); + // The function should still be deserialized with a safe fallback name + assert.equal(typeof deserialized, 'function'); + assert.equal(deserialized.name, 'anonymous'); + }); + + it('sanitizes function names with require() injection', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_TYPE__: 'Function', + name: 'x(){},require("child_process"),function y', + }); + + const deserialized = await deserialize(maliciousPayload); + assert.equal(typeof deserialized, 'function'); + assert.equal(deserialized.name, 'anonymous'); + }); + + it('sanitizes function names with parentheses', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_TYPE__: 'Function', + name: 'foo()', + }); + + const deserialized = await deserialize(maliciousPayload); + assert.equal(typeof deserialized, 'function'); + assert.equal(deserialized.name, 'anonymous'); + }); + + it('sanitizes function names with spaces', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_TYPE__: 'Function', + name: 'foo bar', + }); + + const deserialized = await deserialize(maliciousPayload); + assert.equal(typeof deserialized, 'function'); + assert.equal(deserialized.name, 'anonymous'); + }); + + it('sanitizes empty function names', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_TYPE__: 'Function', + name: '', + }); + + const deserialized = await deserialize(maliciousPayload); + assert.equal(typeof deserialized, 'function'); + assert.equal(deserialized.name, 'anonymous'); + }); + + it('handles bound function prefix with malicious payload', async () => { + const maliciousPayload = JSON.stringify({ + __WTR_TYPE__: 'Function', + name: 'bound x(){},(globalThis.PWNED_BOUND=true),function y', + }); + + const deserialized = await deserialize(maliciousPayload); + + assert.equal((globalThis as any).PWNED_BOUND, undefined); + assert.equal(typeof deserialized, 'function'); + assert.equal(deserialized.name, 'anonymous'); + }); + + it('allows valid function names', async () => { + const validNames = ['foo', 'fooBar', '_foo', '$foo', 'foo123', '_$foo_bar_123']; + + for (const name of validNames) { + const payload = JSON.stringify({ + __WTR_TYPE__: 'Function', + name: name, + }); + + const deserialized = await deserialize(payload); + assert.equal(typeof deserialized, 'function'); + assert.equal(deserialized.name, name, `Expected ${name} to be allowed`); + } + }); + + it('allows valid bound function names', async () => { + const payload = JSON.stringify({ + __WTR_TYPE__: 'Function', + name: 'bound myFunction', + }); + + const deserialized = await deserialize(payload); + assert.equal(typeof deserialized, 'function'); + assert.equal(deserialized.name, 'myFunction'); + }); + }); + + describe('nested injection prevention', function () { + it('sanitizes malicious names in nested objects', async () => { + const maliciousPayload = JSON.stringify({ + nested: { + __WTR_CONSTRUCTOR_NAME__: 'x(){},(globalThis.NESTED_PWNED=true),function y', + data: 'nested data', + }, + }); + + const deserialized = await deserialize(maliciousPayload); + + assert.equal((globalThis as any).NESTED_PWNED, undefined); + assert.equal(deserialized.nested.constructor.name, '__unknown__'); + assert.equal(deserialized.nested.data, 'nested data'); + }); + + it('sanitizes malicious function names in arrays', async () => { + const maliciousPayload = JSON.stringify([ + { + __WTR_TYPE__: 'Function', + name: 'x(){},(globalThis.ARRAY_PWNED=true),function y', + }, + ]); + + const deserialized = await deserialize(maliciousPayload); + + assert.equal((globalThis as any).ARRAY_PWNED, undefined); + assert.equal(typeof deserialized[0], 'function'); + assert.equal(deserialized[0].name, 'anonymous'); + }); + }); +});