Description
vm2 3.10.5 introduced a severe performance regression for workloads where a host-side library is frozen into the VM and its methods return objects with deep cross-references (e.g. DOM wrappers, XML parsers).
The root cause is containsDangerousConstructor() in lib/bridge.js, added in the security fixes between 3.10.4 and 3.10.5. It is called in thisFromOtherWithFactory() on every object crossing the bridge, and it recursively walks all reachable own properties of each object. For DOM-like objects where every node references ownerDocument (which in turn references the entire document tree), a single bridge crossing triggers a full walk of thousands of objects.
Benchmark
Tested against a real-world XML transformation script using @xmldom/xmldom-style objects (every node holds a reference to ownerDocument, making ~12,500 objects reachable from a single node):
| Version |
Mean per run |
| 3.10.4 |
~6ms |
| 3.10.5 |
~1,100ms |
In production workloads with larger documents, we measured ~43,000ms vs ~329ms (~133x regression).
Reproduction
A self-contained reproduction script is available here:
https://gist.github.com/bglick/3f83a9530058df4cb2e9988431b9524e
git clone https://github.com/patriksimek/vm2
cd vm2
npm install
# copy perf-repro.js from the gist into the vm2 directory
node perf-repro.js # 3.10.5: ~1,100ms/run
git checkout v3.10.4 -- lib/bridge.js
node perf-repro.js # 3.10.4: ~6ms/run
Root Cause
In thisFromOtherWithFactory (lib/bridge.js), the security fix added:
const dangerous = !isHost && containsDangerousConstructor(other);
containsDangerousConstructor creates a new WeakMap on every call and recursively walks all reachable objects. There is no caching between calls, so the same prototype chain is scanned from scratch for every new instance of the same class that crosses the bridge.
Proposed Fix
Cache scan results by prototype. Prototypes are long-lived and their shape is stable, so the result is safe to cache in a WeakMap. Instance own properties only need a shallow (non-recursive) check — nested objects get their own scan when they individually cross the bridge via thisFromOtherWithFactory, preserving the same security invariant.
We have a working patch on our fork that passes all 144 existing tests including every sandbox escape test:
Happy to open a PR if you'd like to review it.
Description
vm23.10.5 introduced a severe performance regression for workloads where a host-side library is frozen into the VM and its methods return objects with deep cross-references (e.g. DOM wrappers, XML parsers).The root cause is
containsDangerousConstructor()inlib/bridge.js, added in the security fixes between 3.10.4 and 3.10.5. It is called inthisFromOtherWithFactory()on every object crossing the bridge, and it recursively walks all reachable own properties of each object. For DOM-like objects where every node referencesownerDocument(which in turn references the entire document tree), a single bridge crossing triggers a full walk of thousands of objects.Benchmark
Tested against a real-world XML transformation script using
@xmldom/xmldom-style objects (every node holds a reference toownerDocument, making ~12,500 objects reachable from a single node):In production workloads with larger documents, we measured ~43,000ms vs ~329ms (~133x regression).
Reproduction
A self-contained reproduction script is available here:
https://gist.github.com/bglick/3f83a9530058df4cb2e9988431b9524e
Root Cause
In
thisFromOtherWithFactory(lib/bridge.js), the security fix added:containsDangerousConstructorcreates a newWeakMapon every call and recursively walks all reachable objects. There is no caching between calls, so the same prototype chain is scanned from scratch for every new instance of the same class that crosses the bridge.Proposed Fix
Cache scan results by prototype. Prototypes are long-lived and their shape is stable, so the result is safe to cache in a
WeakMap. Instance own properties only need a shallow (non-recursive) check — nested objects get their own scan when they individually cross the bridge viathisFromOtherWithFactory, preserving the same security invariant.We have a working patch on our fork that passes all 144 existing tests including every sandbox escape test:
Happy to open a PR if you'd like to review it.