fix: Refactor extend() to handle prototype pollution cases in object merging #1125
Conversation
glen-testing
left a comment
glen-testing
left a comment
There was a problem hiding this comment.
Security approves because we asked for this as a result of a bugbounty report. We didn't confirm any functionality isn't affected.
jaissica12
left a comment
jaissica12
left a comment
There was a problem hiding this comment.
Approving this PR since the prototype pollution fix looks solid and the tests are comprehensive. We can add a couple more tests and clean up some SonarCloud noise, but nothing blocking.
| describe('extend() - Prototype Pollution Prevention', () => { | ||
| it('should block __proto__ in shallow merge', () => { | ||
| const malicious = JSON.parse('{"__proto__": {"isAdmin": true}}'); | ||
| const result = helpers.extend({}, malicious); |
There was a problem hiding this comment.
To reduce the unused variable noise we can add minimal assertions like expect(result).toBeDefined() or expect(typeof result).toBe('object') .
This satisfies the linter while keeping the tests focused on pollution blocking
| @@ -0,0 +1,201 @@ | |||
| import Helpers from '../../src/helpers'; | |||
|
|
|||
| describe('Helpers - Prototype Pollution Protection', () => { | |||
There was a problem hiding this comment.
We can add tests for:
(1) objects with null prototype (Object.create(null)) to handle prototype-less objects
(2) null/undefined source arguments (extend({}, obj1, null, obj2, undefined)) to ensure extend() handles these edge cases correctly.
|
Background
Previous version of jQuery and lodash had a prototype pollution issue in their
extendfunctions. It allows bad actors to pass__proto__,prototype, orconstructorto create unintended behavior for the SDK, which uses theextendhelper function throughout the SDK to merge configuration objects and options.What Has Changed
Added validation to skip reserved object properties (proto, constructor, prototype) during object extension operations, following JavaScript best practices for property iteration.
Screenshots/Video
Checklist
Reference Issue (For employees only. Ignore if you are an outside contributor)