Fix missing sandbox escape defenses (#562)#563
Open
eddieran wants to merge 1 commit intopatriksimek:mainfrom
Open
Fix missing sandbox escape defenses (#562)#563eddieran wants to merge 1 commit intopatriksimek:mainfrom
eddieran wants to merge 1 commit intopatriksimek:mainfrom
Conversation
Two critical defenses documented as present in docs/ATTACKS.md were absent
from the actual code, leaving exploitable gaps:
1. Array Species Self-Return Defense (Category 18):
- Add neutralizeArraySpecies() to set constructor=undefined on host arrays
before/after every host function call in BaseHandler.apply trap
- Intercept constructor writes to host arrays in BaseHandler.set trap
(stores on proxy target instead of host array)
- Intercept constructor defineProperty on host arrays in defineProperty trap
- Add SPECIES_ATTACK_SENTINEL symbol for tamper detection
2. Error.prepareStackTrace Safe Default (Category 19):
- Add defaultSandboxPrepareStackTrace() that safely handles Symbol names,
Proxy objects, and exotic types without throwing
- When user sets Error.prepareStackTrace = undefined, reset to safe default
instead of allowing undefined (which triggers V8 fallback to host's
prepareStackTraceCallback, enabling host-realm TypeError generation)
Fixes patriksimek#562
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two critical defenses documented in
docs/ATTACKS.mdas present ("NOW FIXED") were absent from the actual code, leaving exploitable gaps for sandbox escapes:Array Species Self-Return Defense (Category 18): V8's
ArraySpeciesCreatereadsconstructor[Symbol.species]on raw host arrays, bypassing proxy traps entirely. If an attacker setsconstructorto a species-returning function on a host array, methods likemap/filter/slicestore raw host values directly into that array, bypassing bridge sanitization. This addsneutralizeArraySpecies()in theapplytrap (setsconstructor = undefinedon host arrays before/after every host call), plus constructor interception in thesetanddefinePropertytraps.Error.prepareStackTrace Safe Default (Category 19): When
Error.prepareStackTraceisundefinedin the sandbox, V8 falls back to Node.js's host-sideprepareStackTraceCallback. If that host code throws (e.g., whenerror.nameis aSymbol), theTypeErroris a host-realm error usable for escape. This adds adefaultSandboxPrepareStackTracefunction that safely handles Symbol names and exotic types, and resets to this safe default when the user setsError.prepareStackTrace = undefined.Changes
lib/bridge.jsSPECIES_ATTACK_SENTINELsymbol and cacheSymbol.speciesneutralizeArraySpecies()/neutralizeArraySpeciesArgs()-- setsconstructor = undefinedon host arrays viaotherReflectDefinePropertyBaseHandler.set: interceptconstructorwrites to host arrays (store on proxy target)BaseHandler.defineProperty: interceptconstructordefineProperty on host arraysBaseHandler.apply: callneutralizeArraySpeciesArgsbefore and after host function callslib/setup-sandbox.jsdefaultSandboxPrepareStackTrace()-- safe stack formatter that handles Symbol names, Proxy objects, and exotic types without throwingprepareStackTracesetter: when user setsundefined/non-function, reset to safe default instead of storingundefinedtest/vm.jsTest plan
Fixes #562