Skip to content

fix: prevent prototype pollution in extend() and objExtend() via __proto__ key filtering#2735

Open
hectorhdzg wants to merge 1 commit into
microsoft:mainfrom
hectorhdzg:fix/prototype-pollution-extend-unsafe-keys
Open

fix: prevent prototype pollution in extend() and objExtend() via __proto__ key filtering#2735
hectorhdzg wants to merge 1 commit into
microsoft:mainfrom
hectorhdzg:fix/prototype-pollution-extend-unsafe-keys

Conversation

@hectorhdzg
Copy link
Copy Markdown
Member

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

@hectorhdzg hectorhdzg requested a review from a team as a code owner May 19, 2026 21:02
Copilot AI review requested due to automatic review settings May 19, 2026 21:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 in objExtend() and extend()
  • 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

Comment thread shared/AppInsightsCore/src/ext/extUtils.ts
Comment thread shared/AppInsightsCore/src/utils/HelperFuncs.ts Outdated
Comment thread shared/AppInsightsCore/Tests/Unit/src/ext/UtilsTest.ts Outdated
Comment thread shared/AppInsightsCore/Tests/Unit/src/ai/HelperFunc.Tests.ts Outdated
@hectorhdzg hectorhdzg force-pushed the fix/prototype-pollution-extend-unsafe-keys branch 2 times, most recently from bde7722 to 405173e Compare May 19, 2026 22:55
var obj = theArgs[i];
objForEachKey(obj, (prop, value) => {
// Prevent prototype pollution by skipping unsafe keys
if (prop === "__proto__" || prop === "constructor" || prop === "prototype") {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use objForEachKeySafe or the new isUnsafePropKey helper instead of repeating the "proto" and other strings multiple times in the code base

let isArgObj = isObject(arg);
for (let prop in arg) {
// Prevent prototype pollution by skipping unsafe keys
if (prop === "__proto__" || prop === "constructor" || prop === "prototype") {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
@hectorhdzg hectorhdzg force-pushed the fix/prototype-pollution-extend-unsafe-keys branch from 405173e to 43e8190 Compare May 19, 2026 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants