How prototype pollution happens in JavaScript AST traversal and how to fix it

Answer Summary

This is a prototype pollution vulnerability (CWE-1321) in the JavaScript AST traversal module src/traverse/estraverse at line 358. The Controller.prototype.__initialize function merged caller-supplied visitor keys using Object.assign(Object.create(this.__keys), visitor.keys); since Object.assign performs [[Set]], a source key named __proto__ triggered the inherited Object.prototype.__proto__ setter and replaced the key map's prototype with an attacker-controlled object. The fix uses an object spread — this.__keys = visitor.keys ? { ...VisitorKeys, ...visitor.keys } : VisitorKeys — because spread uses CreateDataPropertyOrThrow, which defines own properties and never invokes inherited setters.

Vulnerability at a Glance

Introduction

The src/traverse/estraverse file is the engine that walks an ESTree abstract syntax tree. To know which properties of a node contain child nodes, it consults a lookup table called VisitorKeys — Program has body, BinaryExpression has left and right, and so on. Because real-world tools invent their own node types (JSX, TypeScript, template languages, custom codemod nodes), the traversal Controller lets callers pass a keys option to extend that table.

That extension point is where the bug lived. In Controller.prototype.__initialize, the merge was written like this:

this.__keys = VisitorKeys
if (visitor.keys) {
  this.__keys = Object.assign(Object.create(this.__keys), visitor.keys)
}

The intent is elegant: create a fresh object whose prototype is the shared VisitorKeys table, then layer the caller's overrides on top. Defaults are inherited, overrides are own properties, and the shared VisitorKeys object is never mutated.

The problem is the interaction between Object.create and Object.assign. The new object still inherits from Object.prototype, which carries a legacy accessor property named __proto__. And Object.assign copies values by assignment, not by definition. So if visitor.keys contains an own enumerable property literally named __proto__, the merge doesn't store a key list — it calls a setter that rewrites the prototype chain of this.__keys.

This matters far beyond one library. Any Node.js package that accepts an options object and merges it with Object.assign into an object that inherits Object.prototype has the same latent primitive.

The Vulnerability Explained

Why Object.assign is not a safe merge

Object.assign(target, source) is specified to perform Set(target, key, value, true) for each own enumerable key of the source. Set walks the prototype chain looking for an accessor. Object.prototype defines:

Object.getOwnPropertyDescriptor(Object.prototype, '__proto__')
// { get: [Function: get __proto__], set: [Function: set __proto__], ... }

So target.__proto__ = value never creates a data property — it invokes that setter, which replaces target's internal prototype.

Contrast that with object spread. { ...source } is specified using CopyDataProperties, which calls CreateDataPropertyOrThrow — a [[DefineOwnProperty]] operation that ignores the prototype chain entirely. The two look interchangeable in everyday code; they are not interchangeable in the presence of untrusted keys.

Reaching the sink

The keys option is public API. In practice it arrives from places the library author does not control:

• a linter/codemod configuration file parsed with JSON.parse
• a plugin manifest downloaded from a registry
• a web service that lets users describe custom node types
• an options object forwarded verbatim from a caller several layers up

JSON.parse is the classic amplifier here, because it happily creates a genuine own enumerable property named __proto__:

const parsed = JSON.parse('{"__proto__": {"Program": ["hacked"]}}')
Object.keys(parsed)                       // [ '__proto__' ]  ← own, enumerable
Object.getPrototypeOf(parsed) === Object.prototype  // true — not polluted yet

Nothing bad has happened yet. The pollution happens on the merge.

Step 1 — rewiring the key map's prototype

const { traverse } = require('abstract-syntax-tree')

// Attacker-influenced config, e.g. read from a JSON plugin manifest
const keys = JSON.parse('{"__proto__": {"Program": ["evil"], "Identifier": ["evil"]}}')

traverse(ast, {
  keys,
  enter (node) { /* downstream tool logic */ }
})

Inside __initialize:

this.__keys = Object.assign(Object.create(VisitorKeys), keys)

Object.create(VisitorKeys) produces an object with the chain {} → VisitorKeys → Object.prototype. Object.assign then assigns __proto__, the inherited setter fires, and the chain becomes {} → { Program: ['evil'], Identifier: ['evil'] } → Object.prototype.

VisitorKeys has been unlinked from the chain entirely. The traversal's default child-key table is gone, replaced by an object the attacker authored — and note that not a single own property was added, so a defensive check like Object.keys(this.__keys).includes('__proto__') after the merge would see nothing suspicious.

Step 2 — steering the traversal

Every child lookup in the traversal loop resolves through that chain:

candidates = this.__keys[node.type]   // now answered by the attacker's object

The attacker now controls, for every node type, which properties the traverser treats as child nodes. That yields several concrete outcomes:

1. Traversal into non-AST properties. Declaring {"Identifier": ["constructor"]} makes the walker descend into node.constructor, so Identifier.prototype.constructor (or Object) gets passed to enter/leave as if it were an AST node. Downstream consumers routinely write to the nodes they visit — node.name = ..., Object.assign(node, replacement) — turning a read primitive into a write primitive against shared objects and prototypes. That is the second step of the chain.
2. Getter-triggered execution. If the attacker's replacement prototype is built with accessor properties, merely reading this.__keys[node.type] runs attacker code inside the traversal, in whatever privilege context the build tool or server runs.
3. Silent traversal blinding. Returning [] for security-relevant node types (CallExpression, ImportDeclaration, MemberExpression) makes the walker skip them. For a security linter or dependency scanner built on this traversal, that is an analysis bypass: dangerous code passes review because the analyzer never visited it.

Real-world impact

This module is a Node.js library, so the blast radius is downstream consumers: bundlers, linters, minifiers, codemods, and any service that parses or transforms user-submitted JavaScript. Those processes typically run with filesystem and network access in CI. A prototype-level write inside a build step is a supply-chain foothold, and a traversal that can be told to skip node types undermines every security tool layered on top of it. Hence the critical rating even though exploitation requires a caller that forwards untrusted keys.

The Fix

The patch is one statement in Controller.prototype.__initialize.

Before (src/traverse/estraverse:358):

this.__keys = VisitorKeys
if (visitor.keys) {
  this.__keys = Object.assign(Object.create(this.__keys), visitor.keys)
}

After:

// Visitor keys override the defaults; any node type the visitor does not
// mention keeps its default child keys. Spreading defines each key rather
// than assigning it, so a key named __proto__ is stored like any other
// instead of reaching the setter on Object.prototype.
this.__keys = visitor.keys ? { ...VisitorKeys, ...visitor.keys } : VisitorKeys

Three things change, and each one matters:

1. Define instead of assign. The spread of visitor.keys runs through CreateDataPropertyOrThrow, which never consults the prototype chain. A malicious {"__proto__": {...}} now becomes an inert own data property on this.__keys:

const merged = { ...VisitorKeys, ...JSON.parse('{"__proto__":{"Program":["evil"]}}') }
Object.getPrototypeOf(merged) === Object.prototype  // true — chain intact
merged.Program                                      // the real default, e.g. [ 'body' ]

Since no ESTree node type is named __proto__, the stored key is simply never looked up. The exploit primitive is gone without any filtering, allow-listing, or extra runtime checks.

2. Defaults become own properties. { ...VisitorKeys, ...visitor.keys } flattens the two-level prototype chain into a single object with own properties. That removes the inheritance trick that made the old code fragile in the first place: there is no longer a prototype-linked layer for a setter to hijack. Lookup order is preserved because later spreads win — visitor.keys still overrides VisitorKeys for the node types it mentions.

3. VisitorKeys is still not mutated. The spread builds a brand-new object, so the shared module-level VisitorKeys table is untouched — the same safety property the original Object.create was reaching for. And when no keys option is supplied, the code short-circuits to VisitorKeys directly, avoiding a needless copy on the hot path.

Tests that lock in the behavior

Because the fix changes how defaults are reachable (inherited → own), the PR adds regression tests in test/traverse.js to prove the observable contract did not change:

test("it visits the children named by a custom key", () => {
  const { traverse } = AbstractSyntaxTree
  const visited = []
  const tree = {
    type: "CustomNode",
    children: [
      { type: "Identifier", name: "a" },
      { type: "Identifier", name: "b" },
    ],
  }
  traverse(tree, {
    keys: { CustomNode: ["children"] },
    enter(node) {
      visited.push(node.name || node.type)
    },
  })
  assert.deepEqual(visited, ["CustomNode", "a", "b"])
})

test("it keeps the default keys for node types the visitor does not mention", () => {
  const { parse, traverse } = AbstractSyntaxTree
  const visited = []
  const tree = parse("var a = 1")
  traverse(tree, {
    keys: { CustomNode: ["children"] },
    // ...asserts the standard VariableDeclaration/VariableDeclarator/Identifier
    // children are still visited
  })
})

The first test proves custom keys still drive traversal; the second proves the spread did not drop the built-in VisitorKeys entries for node types the visitor never mentions. Together they pin down exactly the behavior that a naive "just delete `__