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
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@rushstack/node-core-library",
"comment": "Prevent prototype pollution when Objects.mergeWith() processes attacker-controlled properties.",
"type": "patch"
}
],
"packageName": "@rushstack/node-core-library",
"email": "mojazayeri@users.noreply.github.com"
}
21 changes: 18 additions & 3 deletions libraries/node-core-library/src/objects/mergeWith.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,31 @@ export function mergeWith<TTarget extends object, TSource extends object>(
const targetRecord: Record<string, unknown> = target as unknown as Record<string, unknown>;
const sourceRecord: Record<string, unknown> = source as unknown as Record<string, unknown>;
for (const [key, srcValue] of Object.entries(sourceRecord)) {
const objValue: unknown = targetRecord[key];
const objValue: unknown = Object.hasOwnProperty.call(targetRecord, key)
? targetRecord[key]
: undefined;
const customized: unknown = customizer?.(objValue, srcValue, key);
if (customized !== undefined) {
targetRecord[key] = customized;
_setProperty(targetRecord, key, customized);
} else if (isRecord(srcValue) && isRecord(objValue)) {
mergeWith(objValue, srcValue, customizer);
} else {
targetRecord[key] = srcValue;
_setProperty(targetRecord, key, srcValue);
}
}

return target;
}

function _setProperty(target: Record<string, unknown>, key: string, value: unknown): void {
if (key === '__proto__') {
Object.defineProperty(target, key, {
configurable: true,
enumerable: true,
value,
writable: true
});
} else {
target[key] = value;
}
}
22 changes: 22 additions & 0 deletions libraries/node-core-library/src/objects/test/mergeWith.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,28 @@ describe(mergeWith.name, () => {
mergeWith(target, { a: 1 });
expect(target).toEqual({ a: 1 });
});

it('does not merge into inherited target properties', () => {
const prototype: { settings: Record<string, unknown> } = { settings: { inherited: true } };
const target: Record<string, unknown> = Object.create(prototype);

mergeWith(target, { settings: { own: true } });

expect(prototype.settings).toEqual({ inherited: true });
expect(target.settings).toEqual({ own: true });
expect(Object.hasOwnProperty.call(target, 'settings')).toBe(true);
});

it('does not pollute Object.prototype through __proto__', () => {
const source: Record<string, unknown> = JSON.parse('{"__proto__":{"polluted":true}}');
const target: Record<string, unknown> = {};

mergeWith(target, source);

expect((Object.prototype as Record<string, unknown>).polluted).toBeUndefined();
expect(Object.hasOwnProperty.call(target, '__proto__')).toBe(true);
expect(target.__proto__).toEqual({ polluted: true });
});
});

describe('customizer behavior', () => {
Expand Down