Repository navigation
fix(utils): prevent prototype pollution in deepExtend - #3287
lahirumaramba wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a security check to the deepExtend function in src/utils/deep-copy.ts to prevent prototype pollution by skipping __proto__, constructor, and prototype properties. The feedback suggests adding unit tests to verify that these prototype pollution vectors are successfully blocked and to prevent future regressions.
| if (prop === '__proto__' || prop === 'constructor' || prop === 'prototype') { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
While the prototype pollution check is correct, there are no unit tests included in this pull request to verify that deepExtend successfully blocks prototype pollution attempts (e.g., using __proto__, constructor, or prototype). Adding unit tests for these edge cases is highly recommended to prevent future regressions. For example, you can add tests like:\n\ndescribe('deepExtend prototype pollution', () => {\n it('should not pollute prototype via proto', () => {\n const payload = JSON.parse('{"proto": {"polluted": true}}');\n deepExtend({}, payload);\n expect((Object.prototype as any).polluted).to.be.undefined;\n });\n\n it('should not pollute prototype via constructor', () => {\n const payload = JSON.parse('{"constructor": {"prototype": {"polluted": true}}}');\n deepExtend({}, payload);\n expect((Object.prototype as any).polluted).to.be.undefined;\n });\n});
Prevent prototype pollution in
deepExtend