Skip to content

fix: guard filterXSSWithResult option copy against prototype pollution - #301

Open
mahirhir wants to merge 1 commit into
leizongmin:masterfrom
mahirhir:fix/proto-pollution-filterxsswithresult
Open

fix: guard filterXSSWithResult option copy against prototype pollution#301
mahirhir wants to merge 1 commit into
leizongmin:masterfrom
mahirhir:fix/proto-pollution-filterxsswithresult

Conversation

@mahirhir

Copy link
Copy Markdown

#298 added Object.prototype.hasOwnProperty guards to every place the library copies config out of a for...in loop (shallowCopyObject, keysToLowerCase, the FilterXSS option handling and the exports setup) so a polluted Object.prototype can't inject sanitizer config.

filterXSSWithResult, added in #297 in the same release, copies the caller's options with its own for...in loop that didn't get that guard:

for (var key in options) {
  opts[key] = options[key];
}

It copies inherited enumerable properties onto opts as own properties, so they survive the now-guarded shallowCopyObject inside FilterXSS and become the effective config. A polluted prototype therefore still reaches the sanitizer through this entry point when an options object is passed.

Repro on master (assuming Object.prototype was polluted elsewhere in the process, the same threat model as #298):

Object.prototype.escapeHtml = () => '<img src=x onerror=alert(1)>';
Object.prototype.whiteList = { script: ['src'] };

xss('<script src="evil.js"></script>', {});
// safe: "&lt;script src=\"evil.js\"&gt;&lt;/script&gt;"

xss.filterXSSWithResult('<script src="evil.js"></script>', {}).html;
// "<img src=x onerror=alert(1)><script src=...></script>"  -> polluted config used

The fix mirrors shallowCopyObject:

for (var key in options) {
  if (Object.prototype.hasOwnProperty.call(options, key)) {
    opts[key] = options[key];
  }
}

This is the same threat model as #298 (hardening against an externally polluted Object.prototype, not a new vector from HTML input) — it just completes that fix for the one config-copy path that was missed.

Added a regression test in test/test_filter_xss_with_result.js (fails before this change, passes after) and rebuilt dist/ to match. Full test suite (43 tests) passes.

leizongmin#298 added hasOwnProperty guards to every config-copy loop. The
filterXSSWithResult helper added in leizongmin#297 has its own option-copy loop
that was missed, so a polluted Object.prototype still injects config
through that entry point. Mirror the shallowCopyObject guard.
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