Repository navigation
fix: zero backing bytes outside copied ArrayBuffer views - #18
Merged
Merged
Conversation
Copying a view also copied its whole backing ArrayBuffer, so small pooled Node Buffers leaked the rest of the pool into the receiving isolate (and vice versa on implicit copies). After reconstruction, audit the copied graph without running JavaScript and zero bytes outside the union of the views' ranges, unless the backing ArrayBuffer itself is part of the graph. SharedArrayBuffers are untouched. - Live objects inserted via derefInto() are opaque leaves and are never audited or modified (held as Globals: ReadValue closes its own scope). - Plain records are recognised by intrinsic prototype identity, so receiver prototype pollution cannot make copies fail. - transferIn consumes external storage only after sanitization succeeds, so a failed audit no longer destroys the ExternalCopy. - Audit work is bounded to 1,048,576 edges/roots.
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.
Overview
Copying an
ArrayBufferview across an isolate boundary copied its whole backing buffer, so a small pooled NodeBufferexposed the rest of the 8 KB pool through.buffer. That pool can hold data from unrelated requests. This change zeroes every byte that no copied view covers before the result reaches the receiver, closing the heap disclosure reported in VMB-542 at the isolated-vm layer.Risk Estimate
Changes
ExternalCopy, implicit copies, callback/closure argument vectors), audit the reconstructed graph without running JavaScript, then zero bytes outside the union of view ranges. Offsets, buffer size and aliasing between views are preserved.ArrayBufferexplicitly anywhere in the graph keeps all of its bytes. SharedArrayBuffers are never touched, and neither is reusableExternalCopystorage.derefInto()are opaque leaves, never audited or modified. They're held asv8::GlobalbecauseValueDeserializer::ReadValuecloses its own handle scope.Object.prototypeidentity, so receiver prototype pollution can't make copies fail. Proxies, accessors and unknown object types are rejected, and the audit is capped at 1,048,576 edges/roots.transferIn, external storage is consumed only after sanitization succeeds (CopyIntoPrivate+Consume), so a failed audit leaves theExternalCopyintact.tests/array-buffer-sanitization.jscovers aliasing/offsets, explicit buffer inclusion, every view type, cycles, SAB, reusable vs transferred storage, memory-limit charging, implicit copy paths in both directions, multi-argument vectors, prototype pollution,derefInto(), and atransferInaudit failure.tests/array-buffer-copy.jsis updated for the zeroed bytes. Full suite: 85 passed.Project Link
Jira issue