From ac3626a023fe197f3ab18152aede2372542d5c0b Mon Sep 17 00:00:00 2001
From: Janrupf <werbung.janrupf@t-online.de>
Date: Mon, 29 Jan 2024 11:32:28 +0100
Subject: [PATCH] fix(napi): Fix buffer corruption and soundness issues (#1923)

* fix(napi): Fix buffer corruption and soundness issues

* test: fix tests to conform to buffer API
---
 crates/napi/src/env.rs                  | 30 ++++++++++++++++++++-----
 examples/napi-compat-mode/src/buffer.rs | 12 +++++-----
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/crates/napi/src/env.rs b/crates/napi/src/env.rs
index 352d2235..31526e76 100644
--- a/crates/napi/src/env.rs
+++ b/crates/napi/src/env.rs
@@ -313,9 +313,18 @@ impl Env {
   /// Provided `finalize_callback` will be called when `Buffer` got dropped.
   ///
   /// You can pass in `noop_finalize` if you have nothing to do in finalize phase.
+  ///
+  /// # Notes
+  ///
+  /// JavaScript may mutate the data passed in to this buffer when writing the buffer.
+  /// However, some JavaScript runtimes do not support external buffers (notably electron!)
+  /// in which case modifications may be lost.
+  ///
+  /// If you need to support these runtimes, you should create a buffer by other means and then
+  /// later copy the data back out.
   pub unsafe fn create_buffer_with_borrowed_data<Hint, Finalize>(
     &self,
-    mut data: *const u8,
+    mut data: *mut u8,
     length: usize,
     hint: Hint,
     finalize_callback: Finalize,
@@ -324,7 +333,7 @@ impl Env {
     Finalize: FnOnce(Hint, Env),
   {
     let mut raw_value = ptr::null_mut();
-    if data.is_null() || data == EMPTY_VEC.as_ptr() {
+    if data.is_null() || data as *const u8 == EMPTY_VEC.as_ptr() {
       return Err(Error::new(
         Status::InvalidArg,
         "Borrowed data should not be null".to_owned(),
@@ -370,7 +379,7 @@ impl Env {
         value: raw_value,
         value_type: ValueType::Object,
       }),
-      mem::ManuallyDrop::new(unsafe { Vec::from_raw_parts(data as *mut u8, length, length) }),
+      mem::ManuallyDrop::new(unsafe { Vec::from_raw_parts(data, length, length) }),
     ))
   }
 
@@ -461,7 +470,7 @@ impl Env {
           let mut underlying_data = ptr::null_mut();
           let status =
             sys::napi_create_arraybuffer(self.0, length, &mut underlying_data, &mut raw_value);
-          ptr::copy_nonoverlapping(underlying_data.cast(), data_ptr, length);
+          ptr::copy_nonoverlapping(data_ptr, underlying_data.cast(), length);
           status
         } else {
           status
@@ -487,9 +496,18 @@ impl Env {
   /// Provided `finalize_callback` will be called when `Buffer` got dropped.
   ///
   /// You can pass in `noop_finalize` if you have nothing to do in finalize phase.
+  ///
+  /// # Notes
+  ///
+  /// JavaScript may mutate the data passed in to this buffer when writing the buffer.
+  /// However, some JavaScript runtimes do not support external buffers (notably electron!)
+  /// in which case modifications may be lost.
+  ///
+  /// If you need to support these runtimes, you should create a buffer by other means and then
+  /// later copy the data back out.
   pub unsafe fn create_arraybuffer_with_borrowed_data<Hint, Finalize>(
     &self,
-    mut data: *const u8,
+    data: *mut u8,
     length: usize,
     hint: Hint,
     finalize_callback: Finalize,
@@ -527,7 +545,7 @@ impl Env {
         let mut underlying_data = ptr::null_mut();
         let status =
           sys::napi_create_arraybuffer(self.0, length, &mut underlying_data, &mut raw_value);
-        data = underlying_data.cast();
+        ptr::copy_nonoverlapping(data, underlying_data.cast(), length);
         finalize(hint, *self);
         check_status!(status)?;
       } else {
diff --git a/examples/napi-compat-mode/src/buffer.rs b/examples/napi-compat-mode/src/buffer.rs
index 9c5fa9d5..9746e8f5 100644
--- a/examples/napi-compat-mode/src/buffer.rs
+++ b/examples/napi-compat-mode/src/buffer.rs
@@ -28,8 +28,8 @@ pub fn copy_buffer(ctx: CallContext) -> Result<JsBuffer> {
 
 #[contextless_function]
 pub fn create_borrowed_buffer_with_noop_finalize(env: Env) -> ContextlessResult<JsBuffer> {
-  let data = vec![1, 2, 3];
-  let data_ptr = data.as_ptr();
+  let mut data = vec![1, 2, 3];
+  let data_ptr = data.as_mut_ptr();
   let length = data.len();
   let manually_drop = ManuallyDrop::new(data);
 
@@ -39,8 +39,8 @@ pub fn create_borrowed_buffer_with_noop_finalize(env: Env) -> ContextlessResult<
 
 #[contextless_function]
 pub fn create_borrowed_buffer_with_finalize(env: Env) -> ContextlessResult<JsBuffer> {
-  let data = vec![1, 2, 3];
-  let data_ptr = data.as_ptr();
+  let mut data = vec![1, 2, 3];
+  let data_ptr = data.as_mut_ptr();
   let length = data.len();
   let manually_drop = ManuallyDrop::new(data);
 
@@ -59,8 +59,8 @@ pub fn create_borrowed_buffer_with_finalize(env: Env) -> ContextlessResult<JsBuf
 
 #[contextless_function]
 pub fn create_empty_borrowed_buffer_with_finalize(env: Env) -> ContextlessResult<JsBuffer> {
-  let data = vec![];
-  let data_ptr = data.as_ptr();
+  let mut data = vec![];
+  let data_ptr = data.as_mut_ptr();
   let length = data.len();
   let manually_drop = ManuallyDrop::new(data);