Security: sanitize sensitive keys in merge functions#26756
Security: sanitize sensitive keys in merge functions#26756themilessky01 wants to merge 1 commit intoemscripten-core:mainfrom
Conversation
This change prevents Prototype Pollution vulnerabilities by filtering out sensitive keys like __proto__, constructor, and prototype during object merging in utility.mjs
| // Sanitize 'other' object before Object.assign to prevent prototype pollution | ||
| const keys = Object.keys(other); | ||
| for (const key of keys) { | ||
| if (key === '__proto__' || key === 'constructor' || key === 'prototype') { |
There was a problem hiding this comment.
Object.keys already doesn't include any of these.
There was a problem hiding this comment.
Hi @sbc100,
I understand your point about Object.keys, but the concern is when other comes from a source where these properties are enumerable (for example, when using JSON.parse on a specially crafted string).
Here is a simple PoC that shows how the pollution can happen if we don't have these checks:
const payload = JSON.parse('{"proto": {"polluted": "yes"}}');
const target = {};
// If mergeInto uses Object.assign or a loop on enumerable keys:
Object.assign(target, payload);
console.log({}.polluted); // In some environments/scenarios, this could lead to pollution
My goal is to ensure Emscripten's core utilities are robust against such untrusted inputs. If you feel Object.keys is sufficient for the current implementation, I'd appreciate your guidance on the preferred way to strictly enforce this safety for all contributors.
There was a problem hiding this comment.
Emscripten is compiler. We already trust the inputs so its not an issue for us.
The JS library files passed to the compiler already have full access to all node API, so there is no limit to their power and we trust them fully.
|
Hi @sbc100,
I understand your point about Object.keys, but the concern is when other
comes from a source where these properties are enumerable (for example,
when using JSON.parse on a specially crafted string).
Here is a simple PoC that shows how the pollution can happen if we don't
have these checks:
const payload = JSON.parse('{"__proto__": {"polluted": "yes"}}');
const target = {};
// If mergeInto uses Object.assign or a loop on enumerable keys:
Object.assign(target, payload);
console.log({}.polluted); // In some environments/scenarios, this could
lead to pollution
My goal is to ensure Emscripten's core utilities are robust against such
untrusted inputs. If you feel Object.keys is sufficient for the current
implementation, I'd appreciate your guidance on the preferred way to
strictly enforce this safety for all contributors.
…On Thu, Apr 23, 2026, 5:04 AM Sam Clegg ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/utility.mjs
<#26756?email_source=notifications&email_token=CCMXML3EHMG6CEVCDCFSV2L4XF24FA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMJVHEYDQMJSGYZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K64DSL5ZGK5TJMV3V6Y3MNFRWW#discussion_r3127902317>
:
> @@ -133,7 +141,14 @@ export function mergeInto(obj, other, options = null) {
}
}
- for (const key of Object.keys(other)) {
+ // Sanitize 'other' object before Object.assign to prevent prototype pollution
+ const keys = Object.keys(other);
+ for (const key of keys) {
+ if (key === '__proto__' || key === 'constructor' || key === 'prototype') {
Object.keys already doesn't include any of these.
—
Reply to this email directly, view it on GitHub
<#26756?email_source=notifications&email_token=CCMXML44M63QRZ37QWEIRH34XF24FA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMJVHEYDQMJSGYZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2L24DSL5ZGK5TJMV3V63TPORUWM2LDMF2GS33OONPWG3DJMNVQ#pullrequestreview-4159081262>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/CCMXML72G7UI4CXGVK6ZTMT4XF24FAVCNFSM6AAAAACYDANG6OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DCNJZGA4DCMRWGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Closing for now. Feel free to re-open. |
This change prevents Prototype Pollution vulnerabilities by filtering out sensitive keys like proto, constructor, and prototype during object merging in utility.mjs