Repository navigation
bound arraybuffer access in javy.io readSync and writeSync - #1309
Open
nabeel-dev21 wants to merge 1 commit into
Open
nabeel-dev21 wants to merge 1 commit into
nabeel-dev21 wants to merge 1 commit into
Conversation
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.
Description of the change
Javy.IO.readSyncandJavy.IO.writeSynctake anArrayBufferplus an offset and a length that come straight from JavaScript numbers, then slice the buffer withdata[offset..(offset + length)]. Neither value is bounds-checked against the buffer and the addition is unguarded, so an out-of-range or overflowing pair reaches the slice.writeSyncpanics on the index;readSyncwent further and built a&mut [u8]over engine-owned memory through rawJS_GetArrayBufferandslice::from_raw_parts_mut, then indexed that the same way.Both functions now obtain the buffer through rquickjs' typed
as_array_buffer/as_rawaccessor and resolve the range withchecked_addplusget/get_mutagainst the buffer's own length, returning a JS error for a bad range. That also drops the raw-FFI slice inreadSync, so the single remainingunsafethere follows the same SAFETY contractwriteSyncalready documents.Why am I making this change?
The
readSyncpath carried a comment noting the raw slice should be revisited to make it safe, and its soundness rested on the JavaScript wrapper always handing over an offset and length consistent with the buffer. Doing the validation inside the host function removes that reliance and keeps the slicing sound regardless of what the caller passes. Behaviour is unchanged for valid inputs.Checklist
javy-plugin-apiif the QuickJS bytecode has changed.javy-cli,javy-plugin, andjavy-plugin-processingdo not require updating CHANGELOG files.