[Security] Proto-access control bypass via Map Symbol.toStringTag spoofing + HTML escape bypass

[eddieran]

Summary

Three security findings in Handlebars.js v5.0.0-alpha.1:

Finding 1: Proto-Access Control Bypass via Fake Map (Medium)

File: lib/handlebars/runtime.js:119-122

lookupProperty completely skips proto-access controls for Map objects. Map detection uses Object.prototype.toString which is spoofable via Symbol.toStringTag:

const fakeMap = { [Symbol.toStringTag]: 'Map', get(key) { return key === 'constructor' ? Function : undefined; } };
// lookupProperty(fakeMap, 'constructor') returns Function, bypassing the blocklist

Fix: Use instanceof Map instead of toString tag check.

Finding 2: HTML Escaping Bypass via toHTML Duck-Typing (Medium)

File: lib/handlebars/utils.js:61-66

escapeExpression treats any object with a toHTML property as SafeString (no instanceof check). Attacker-controlled data with toHTML bypasses HTML escaping in {{}} expressions.

Fix: Use instanceof SafeString check.

Finding 3: AST Injection via Direct AST Input (Medium)

File: lib/handlebars/compiler/compiler.js:476-485

compile() accepts pre-parsed AST objects without structural validation, enabling code injection via crafted AST nodes through the Function() constructor.

Fix: Validate AST structure before compilation.

[jaylinski]

Thanks for the report! I will also test this for our stable 4.x branch and do a backport if necessary. A few notes:

Finding 1: this vulnerability only affects applications that use user-controlled Map-objects as template input.
Finding 2: same preconditions as in finding 2.
Finding 3: this was recently patched in version 4.7.9 (upstream is pending #2143). Do you have concrete PoC that I can test? Maybe you found a variant I didn't think of in the previous security fixes.

[eddieran]

Thanks for the detailed response, @jaylinski.

Finding 3 (AST injection): You're right — I reviewed the v4.7.9 changes and PR #2143, and the compiler-level sanitization (sanitizeDepth, sanitizeParts, type coercion for literals) is a solid fix. Handling it at the compiler boundary rather than via input AST validation is arguably more robust since it covers all code paths. I don't have a variant that bypasses the new mitigations.

Findings 1 and 2: Agreed on the preconditions — these require attacker-controlled objects in the template context. That said, this is a common pattern in web applications where template data is constructed from user input (e.g., form submissions, API responses deserialized into objects). An attacker who can control a property like Symbol.toStringTag or inject a toHTML method on a context object can bypass proto-access controls or HTML escaping respectively.

I had submitted a fix for these two as PR #2147 but it was inadvertently closed. I can re-submit the PR if that would be helpful — the changes are straightforward:

• Finding 1: instanceof Map/instanceof Set instead of Object.prototype.toString tag check
• Finding 2: instanceof SafeString instead of duck-typing on toHTML

Happy to help however is most useful for the 4.x backport.

[eddieran]

Thanks @jaylinski — quick status update on the three findings:

Finding 1 (Map spoofing via Symbol.toStringTag) — Agreed, and only exploitable when user-controlled objects reach lookupProperty. PR #2148 now contains just this one fix: instanceof Map / instanceof Set in place of the tag check. CI is green and the change is pure defensive hardening — no API impact.

Finding 2 (toHTML duck-typing escape bypass) — Walked this back: @copilot-pull-request-reviewer flagged it correctly as a breaking change to the documented duck-typing contract where any object with .toHTML() is treated as safe. The "attack" requires the attacker to already control template structure, not just values — a much bigger compromise than what Handlebars' threat model covers. Removed from the PR.

Finding 3 (AST injection via pre-parsed AST input) — You're right that this was patched in 4.7.9. I hit it on v5.0.0-alpha.1 while poking at compile(ast) and didn't notice the backport landed upstream. I'll re-test against current master; if the 4.7.9 fix is fully ported to the 5.x line, this one drops off entirely. If you do a 4.x backport of PR #2148, let me know if you'd like me to open a parallel PR against the 4.x branch.