Conversation
…ant callback JS_PrintValue's write callback can re-enter the context and detach the backing ArrayBuffer (JS_DetachArrayBuffer) or mutate a fast array while js_print_object() is iterating with a cached element count. The element pointer/value was then read after the callback returned, dereferencing freed storage. - typed arrays: recheck the live element count each iteration and copy the element into a local before emitting output through the callback - fast arrays: same live-count recheck; hold a JSValue reference to the element across the output so it cannot be freed mid-print Reproduced with an embedder write callback that detaches the ArrayBuffer after the first element: ASan reported a heap-use-after-free read in js_print_object(); after the fix the print stops cleanly for all 12 typed array classes and for shrunk fast arrays. make test passes. Fixes bellard#558
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
1 participant
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.
Issue
Fixes #558 —
JS_PrintValue()reads stale/freed TypedArray storage when an embedder-supplied write callback re-enters the context and detaches the backingArrayBuffermid-print (heap-use-after-free / NULL-deref, reported by Sn0wyDay with an ASan trace).Root cause
In
js_print_object(), the TypedArray print loop cached the element count inlen1and materializedptrbefore callingjs_print_comma(), which invokes the embedder's write callback. If that callback callsJS_DetachArrayBuffer()(or otherwise mutates the value being printed), the subsequent*ptrdereference touched freed storage.The fast-array print loop directly above has the same reentrancy flaw:
p->u.array.u.values[i]was read after the output callback, so a callback that shrinks or frees the array (e.g. viaJS_SetPropertyonlength) left a dangling element access.Fix
Both loops now treat the live object state as volatile across every output call:
p->u.array.countis rechecked each iteration (it drops to 0 on detach), and the element is copied into a local variable beforejs_print_comma()runs — the value printed is the one read while the buffer was still valid. Output then proceeds from the copy, so no stale pointer is ever dereferenced.JS_DupValueRT()/JS_FreeValueRT()around the output so a reentrant shrink cannot free the element mid-print.The change is confined to the two element loops; the header/footer prints and all other object classes are untouched.
Verification
Reproducer: embedder write callback that calls
JS_DetachArrayBuffer()once the first element has been emitted.04be246): ASanheap-use-after-freeread injs_print_object()(freed by JS_DetachArrayBuffer).Uint8Array(5) [ 1, 2 ]— the element copied before the detaching callback is still emitted, then the loop terminates cleanly. No sanitizer findings.Uint8Clamped…Float64, incl.BigInt64/BigUint64/Float16) verified with the same trigger.a.lengthmid-print):[ 1, 2, <3 empty items> ], no crash.make test: all suites pass.