Skip to content

fix: reject __proto__/constructor/prototype filter keys (prototype pollution) - #172

Open
BrianWillows wants to merge 1 commit into
loris:masterfrom
BrianWillows:fix/prototype-pollution
Open

fix: reject __proto__/constructor/prototype filter keys (prototype pollution)#172
BrianWillows wants to merge 1 commit into
loris:masterfrom
BrianWillows:fix/prototype-pollution

Conversation

@BrianWillows

Copy link
Copy Markdown

Summary

The filter reducer assigns into an object using the query-string parameter
name
as the key, with no guard on prototype-walking names:

.reduce((result, { prefix, key, op, value }) => {
  if (!result[key]) { result[key] = {}; }
  ...
  } else { result[key][op] = value; }   // key === '__proto__' -> Object.prototype[op] = value

When key is __proto__, result['__proto__'] resolves through the getter to
Object.prototype, so the assignment writes an attacker-controlled, enumerable
property onto the prototype of every object in the process (CWE-1321 prototype
pollution).

Reproduced on 6.1.0:

aqp('__proto__>5');
({}).$gt        // => 5

The library's intended input is exactly an untrusted query string
(aqp(req.query) in an Express/Mongo API), so this is reachable directly from a
request. No configuration is needed — blacklist defaults to empty and
whitelist is undefined.

The polluted keys are limited to Mongo operators ($gt, $gte, $lt, $lte,
$ne, $nin), so this is not an arbitrary-gadget pollution, but the value is
fully attacker-controlled and the property is enumerable — every for (k in obj)
in the process then yields it, which corrupts query shapes and object iteration
generally.

Fix

Reject the three prototype-walking key names in the same filter that already
applies blacklist/whitelist. They are not usable Mongo field names, so no
legitimate query is affected.

const FORBIDDEN_KEYS = ['__proto__', 'constructor', 'prototype'];
...
!FORBIDDEN_KEYS.includes(key) &&
!options.blacklist.includes(key) && ...

Verification

  • Existing test suite passes unchanged (ava, 65 tests passed).
  • aqp('__proto__>5') now returns {"filter":{}} and ({}).$gt stays
    undefined; the constructor, prototype and !=a,b/$nin variants are
    blocked too.
  • Normal queries are unchanged, e.g.
    aqp('status=sent&price>100&sort=-date&limit=5') still yields
    {"sort":{"date":-1},"limit":5,"filter":{"status":"sent","price":{"$gt":100}}}.

Notes

Found and fixed with AI assistance (Claude). Happy to add a regression test to
the suite if you'd like one.

…llution)

The filter reducer assigns result[key][op] = value where key comes straight
from the query-string parameter name. For key '__proto__', result['__proto__']
resolves via the getter to Object.prototype, so the assignment writes an
attacker-controlled, enumerable property onto Object.prototype for the whole
process (CWE-1321).

aqp('__proto__>5') was enough to set Object.prototype.$gt = 5, and the
library's intended input is exactly an untrusted query string.

Reject the three prototype-walking key names in the same filter that already
applies blacklist/whitelist. They are not usable Mongo field names, so no
legitimate query is affected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

2 participants