Skip to content

reject prototype-polluting path segments in Path.set - #5756

Open
zayeem06 wants to merge 1 commit into
Polymer:masterfrom
zayeem06:path-set-proto-guard
Open

zayeem06 wants to merge 1 commit into
Polymer:masterfrom
zayeem06:path-set-proto-guard

Conversation

@zayeem06

@zayeem06 zayeem06 commented Jul 1, 2026

Copy link
Copy Markdown

Reference Issue

N/A

Path.set walks a dotted path and assigns straight through each segment, so a path whose segments include __proto__, prototype, or constructor (for example __proto__.polluted or constructor.prototype.polluted) writes onto Object.prototype rather than the target object. This helper backs the element set()/notifyPath public API and the two-way binding change path, so an app that forwards an externally influenced path into set ends up polluting every object. The fix rejects those segments inside set and returns without writing, which keeps the pollution guard in the shared helper instead of at each call site. Legitimate paths are unaffected, and I added a regression case to test/unit/path.html that asserts the magic keys no-op while ordinary nested and top-level sets still work.

@zayeem06
zayeem06 requested a review from kevinpschaaf as a code owner July 1, 2026 06:33
@zayeem06

Copy link
Copy Markdown
Author

any update?

@ghost

ghost commented Oct 10, 2026

Copy link
Copy Markdown

Independently re-derived and confirmed this exact mechanism (not aware of this PR beforehand — found it while auditing lib/utils/path.js for a Patch Rewards submission, then found this PR already covers it on dedup-checking before writing anything up).

Traced by hand: for path = "obj.__proto__.polluted" on a target whose obj is a plain object, the loop in set() dereferences prop = prop["obj"] then prop = prop["__proto__"] — the second step resolves via the inherited Object.prototype.__proto__ getter to the real, global Object.prototype. The final prop[last] = value then writes directly onto it. Confirmed this is genuinely global pollution, not a single-instance hijack — every object in the page's JS environment is affected afterward, not just the original target. Since set()/notifyPath is Polymer's primary public data-binding API, this is reachable in any app that forwards an externally-influenced path string into it (e.g. building a dynamic path from a form-field name or API-sourced key).

Given it's been open since July and the severity, wanted to leave independent confirmation to help with prioritizing review/merge. The fix here (reject __proto__/prototype/constructor segments inside set()) looks correct and sufficient for this specific primitive.

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.

1 participant