Security: Prototype Pollution via Custom Attributes in BearerTokenType

[tomaioo]

Summary

Security: Prototype Pollution via Custom Attributes in BearerTokenType

Problem

Severity: Medium | File: lib/token-types/bearer-token-type.js:L58

In BearerTokenType.valueOf(), custom attributes are iterated using for...in without proper prototype chain protection. While Object.prototype.hasOwnProperty.call is used, if customAttributes itself is an object with a polluted prototype (e.g., via __proto__ or constructor), properties from the prototype chain could be included in the token response. Additionally, the constructor allows customAttributes to be any object, which could lead to information disclosure or unexpected behavior if malicious properties are passed.

Solution

Use Object.keys(this.customAttributes) instead of for...in to avoid prototype chain iteration, or ensure customAttributes is created with Object.create(null). Also validate that customAttributes only contains expected types (strings, numbers, booleans) to prevent object injection attacks.

Changes

• lib/token-types/bearer-token-type.js (modified)

[jankapunkt]

Dear @tomaioo if you think this is related to a reproducible vulnerability then please open a new advisory: https://github.com/node-oauth/node-oauth2-server/security/advisories/new

[dhensby]

Thanks for the report @tomaioo, and for thinking about token-response safety. I dug into this with a PoC against the current code and I don't think there's a vulnerability here — and unfortunately the proposed change breaks token issuance. Details:

No prototype pollution is possible. valueOf() already guards with Object.prototype.hasOwnProperty.call(...), so inherited/prototype-chain properties are never copied into the response. There's also no recursive merge anywhere, so Object.prototype can't be polluted. Even feeding a __proto__ payload straight in (JSON.parse('{"__proto__":{"polluted":1},...}')), Object.prototype.polluted stays undefined and polluted never appears in the response's own keys. In the realistic flow through TokenModel, the __proto__ key doesn't even survive into customAttributes.

The change is equivalent for the stated concern — Object.keys() returns the same own-enumerable key set as for...in + hasOwnProperty, so it changes nothing about which keys are copied.

It also regresses the common path. When allowExtendedTokenAttributes is off, customAttributes is undefined; for...in (undefined) is a no-op, but Object.keys(undefined) throws TypeError: Cannot convert undefined or null to object. That's the 22 failing CI checks here — every token response without extended attributes would 500.

Since customAttributes is sourced from the operator's own saveToken() result (and gated behind allowExtendedTokenAttributes) rather than from untrusted request input, I'm going to close this. If you'd like to modernise the loop for readability (e.g. Object.hasOwn), happy to take that as a separate non-security PR — just guard the undefined case.

---

One process note, kindly meant: for anything you suspect is a security vulnerability, please report it privately rather than opening a public PR. The preferred channel is GitHub's private vulnerability reporting — the "Report a vulnerability" button on this repo's Security tab (it's enabled here, and it's how we already handle advisories). That keeps the report — and any description of how to exploit it — private until a fix can be prepared and released, which is exactly what coordinated/responsible disclosure exists to protect. A public PR effectively discloses the suspected issue to everyone before we can ship a fix. There's no actual vulnerability here so no harm done, but for next time the private advisory flow is the safe route — and we're always happy to collaborate on a fix there.

Thanks again for the contribution and for keeping an eye on this. 🙏