fix: prevent prototype pollution in extend() and objExtend() via __proto__ key filtering#2735
Open
hectorhdzg wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Prevents prototype pollution vulnerabilities in extend() and objExtend() when merging untrusted input containing __proto__, and adds unit tests to verify the fix.
Changes:
- Skip merging
__proto__keys inobjExtend()andextend() - Add unit tests covering shallow/deep merges and nested cases
- Validate safe properties still merge when malicious keys are present
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| shared/AppInsightsCore/src/utils/HelperFuncs.ts | Adds __proto__ key filtering to objExtend() loop to block prototype pollution |
| shared/AppInsightsCore/src/ext/extUtils.ts | Adds __proto__ key filtering inside extend() iteration to prevent deep-merge pollution |
| shared/AppInsightsCore/Tests/Unit/src/ext/UtilsTest.ts | Adds extend() regression tests for shallow/deep merges with __proto__ input |
| shared/AppInsightsCore/Tests/Unit/src/ai/HelperFunc.Tests.ts | Adds objExtend() regression tests including nested __proto__ scenarios |
bde7722 to
405173e
Compare
MSNev
reviewed
May 19, 2026
| var obj = theArgs[i]; | ||
| objForEachKey(obj, (prop, value) => { | ||
| // Prevent prototype pollution by skipping unsafe keys | ||
| if (prop === "__proto__" || prop === "constructor" || prop === "prototype") { |
Collaborator
There was a problem hiding this comment.
Should use objForEachKeySafe or the new isUnsafePropKey helper instead of repeating the "proto" and other strings multiple times in the code base
MSNev
reviewed
May 19, 2026
| let isArgObj = isObject(arg); | ||
| for (let prop in arg) { | ||
| // Prevent prototype pollution by skipping unsafe keys | ||
| if (prop === "__proto__" || prop === "constructor" || prop === "prototype") { |
Collaborator
There was a problem hiding this comment.
Should use the new isUnsafePropKey helper instead of repeating the "proto" and other strings multiple times in the code base
…fe key filtering Both extend() in extUtils.ts and objExtend() in HelperFuncs.ts were vulnerable to prototype pollution when processing untrusted input containing __proto__, constructor, or prototype keys (e.g. from JSON.parse). In deep extend mode, target['__proto__'] resolves to Object.prototype via the getter, causing recursive merge to directly mutate Object.prototype and pollute all objects globally. - Filter __proto__, constructor, and prototype keys in extend() objForEachKey callback - Filter __proto__, constructor, and prototype keys in objExtend() for...in loop - Add prototype pollution unit tests with try/finally cleanup guards for both functions
405173e to
43e8190
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both extend() in extUtils.ts and objExtend() in HelperFuncs.ts were vulnerable to prototype pollution when processing untrusted input containing proto keys (e.g. from JSON.parse). In deep extend mode, target['proto'] resolves to Object.prototype via the getter, causing recursive merge to directly mutate Object.prototype and pollute all objects globally.
Add proto key guard in extend() objForEachKey callback
Add proto key guard in objExtend() for...in loop
Add prototype pollution unit tests for both functions