Skip to content

bound arraybuffer access in javy.io readSync and writeSync - #1309

Open
nabeel-dev21 wants to merge 1 commit into
bytecodealliance:mainfrom
nabeel-dev21:javyio-bound-arraybuffer
Open

nabeel-dev21 wants to merge 1 commit into
bytecodealliance:mainfrom
nabeel-dev21:javyio-bound-arraybuffer

Conversation

@nabeel-dev21

Copy link
Copy Markdown

Description of the change

Javy.IO.readSync and Javy.IO.writeSync take an ArrayBuffer plus an offset and a length that come straight from JavaScript numbers, then slice the buffer with data[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. writeSync panics on the index; readSync went further and built a &mut [u8] over engine-owned memory through raw JS_GetArrayBuffer and slice::from_raw_parts_mut, then indexed that the same way.

Both functions now obtain the buffer through rquickjs' typed as_array_buffer/as_raw accessor and resolve the range with checked_add plus get/get_mut against the buffer's own length, returning a JS error for a bad range. That also drops the raw-FFI slice in readSync, so the single remaining unsafe there follows the same SAFETY contract writeSync already documents.

Why am I making this change?

The readSync path 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

  • I've updated the default plugin import namespace and incremented the major version of javy-plugin-api if the QuickJS bytecode has changed.
  • I've updated the relevant CHANGELOG files if necessary. Changes to javy-cli, javy-plugin, and javy-plugin-processing do not require updating CHANGELOG files.
  • I've updated the relevant crate versions if necessary. Versioning policy for library crates
  • I've updated documentation including crate documentation if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant