fix(napi): Read reference ownership before calling finalizer to avoid…

… crash (#24203)

Fixes #23493.

What was happening here was that napi-rs was freeing the napi reference
([here](https://github.com/napi-rs/napi-rs/blob/19e3488efcbc601afa1f11a979372eb6c5ea6130/crates/napi/src/bindgen_runtime/mod.rs#L62))
during its finalize callback (which we call
[here](https://github.com/denoland/deno/blob/fb31eaa9ca59f6daaee0210d5cd206185c7041b9/cli/napi/js_native_api.rs#L132)).
We then were [reading the `ownership`
field](https://github.com/denoland/deno/blob/fb31eaa9ca59f6daaee0210d5cd206185c7041b9/cli/napi/js_native_api.rs#L136)
of that freed reference.

For some reason on arm macs the freed memory gets zeroed, so the value
of `ownership` was `0` when we read it (i.e. it was
`ReferenceOwnership::Runtime`). We then freed it again (since we thought
we owned it), causing the segfault.

cli/napi/js_native_api.rs

@@ -127,13 +127,16 @@ impl Reference {
     let finalize_hint = reference.finalize_hint;
     reference.reset();
 
+    // copy this value before the finalize callback, since
+    // it might free the reference (which would be a UAF)
+    let ownership = reference.ownership;
     if let Some(finalize_cb) = finalize_cb {
       unsafe {
         finalize_cb(reference.env as _, finalize_data, finalize_hint);
       }
     }
 
-    if reference.ownership == ReferenceOwnership::Runtime {
+    if ownership == ReferenceOwnership::Runtime {
       unsafe { drop(Reference::from_raw(reference)) }
     }
   }
@@ -3440,7 +3443,6 @@ fn napi_add_finalizer(
   } else {
     ReferenceOwnership::Userland
   };
-
   let reference = Reference::new(
     env,
     value.into(),

tests/napi/object_wrap_test.js

@@ -40,3 +40,11 @@ Deno.test("napi external arraybuffer", function () {
   assertEquals(new Uint8Array(buf), new Uint8Array([1, 2, 3]));
   buf = null;
 });
+
+Deno.test("napi object wrap userland owned", function () {
+  let obj = new objectWrap.NapiObjectOwned(1);
+  assertEquals(obj.get_value(), 1);
+  obj = null;
+  // force finalize callback to get called
+  globalThis.gc();
+});

tests/napi/src/object_wrap.rs

@@ -5,6 +5,8 @@ use crate::napi_get_callback_info;
 use crate::napi_new_property;
 use napi_sys::ValueType::napi_number;
 use napi_sys::*;
+use std::cell::RefCell;
+use std::collections::HashMap;
 use std::os::raw::c_char;
 use std::os::raw::c_void;
 use std::ptr;
@@ -13,9 +15,36 @@ pub struct NapiObject {
   counter: i32,
 }
 
+thread_local! {
+  // map from native object ptr to napi reference (this is similar to what napi-rs does)
+  static REFS: RefCell<HashMap<*mut c_void, napi_ref>> = RefCell::new(HashMap::new());
+}
+
+pub extern "C" fn finalize_napi_object(
+  env: napi_env,
+  finalize_data: *mut c_void,
+  _finalize_hint: *mut c_void,
+) {
+  let obj = unsafe { Box::from_raw(finalize_data as *mut NapiObject) };
+  drop(obj);
+  if let Some(reference) =
+    REFS.with_borrow_mut(|map| map.remove(&finalize_data))
+  {
+    unsafe { napi_delete_reference(env, reference) };
+  }
+}
+
 impl NapiObject {
-  #[allow(clippy::new_ret_no_self)]
-  pub extern "C" fn new(env: napi_env, info: napi_callback_info) -> napi_value {
+  fn new_inner(
+    env: napi_env,
+    info: napi_callback_info,
+    finalizer: napi_finalize,
+    out_ptr: Option<*mut napi_ref>,
+  ) -> napi_value {
+    assert!(matches!(
+      (finalizer, out_ptr),
+      (None, None) | (Some(_), Some(_))
+    ));
     let mut new_target: napi_value = ptr::null_mut();
     assert_napi_ok!(napi_get_new_target(env, info, &mut new_target));
     let is_constructor = !new_target.is_null();
@@ -33,21 +62,42 @@ impl NapiObject {
       assert_napi_ok!(napi_get_value_int32(env, args[0], &mut value));
 
       let obj = Box::new(Self { counter: value });
+      let obj_raw = Box::into_raw(obj) as *mut c_void;
       assert_napi_ok!(napi_wrap(
         env,
         this,
-        Box::into_raw(obj) as *mut c_void,
-        None,
-        ptr::null_mut(),
+        obj_raw,
+        finalizer,
         ptr::null_mut(),
+        out_ptr.unwrap_or(ptr::null_mut())
       ));
 
+      if let Some(p) = out_ptr {
+        if finalizer.is_some() {
+          REFS.with_borrow_mut(|map| map.insert(obj_raw, unsafe { p.read() }));
+        }
+      }
+
       return this;
     }
 
     unreachable!();
   }
 
+  #[allow(clippy::new_ret_no_self)]
+  pub extern "C" fn new(env: napi_env, info: napi_callback_info) -> napi_value {
+    Self::new_inner(env, info, None, None)
+  }
+
+  #[allow(clippy::new_ret_no_self)]
+  pub extern "C" fn new_with_finalizer(
+    env: napi_env,
+    info: napi_callback_info,
+  ) -> napi_value {
+    let mut out = ptr::null_mut();
+    Self::new_inner(env, info, Some(finalize_napi_object), Some(&mut out))
+  }
+
   pub extern "C" fn set_value(
     env: napi_env,
     info: napi_callback_info,
@@ -148,4 +198,23 @@ pub fn init(env: napi_env, exports: napi_value) {
     "NapiObject\0".as_ptr() as *const c_char,
     cons,
   ));
+
+  let mut cons: napi_value = ptr::null_mut();
+  assert_napi_ok!(napi_define_class(
+    env,
+    c"NapiObjectOwned".as_ptr(),
+    usize::MAX,
+    Some(NapiObject::new_with_finalizer),
+    ptr::null_mut(),
+    properties.len(),
+    properties.as_ptr(),
+    &mut cons,
+  ));
+
+  assert_napi_ok!(napi_set_named_property(
+    env,
+    exports,
+    "NapiObjectOwned\0".as_ptr() as *const c_char,
+    cons,
+  ));
 }

tests/napi/tests/napi_tests.rs

@@ -69,6 +69,7 @@ fn napi_tests() {
     .arg("--allow-env")
     .arg("--allow-ffi")
     .arg("--allow-run")
+    .arg("--v8-flags=--expose-gc")
     .arg("--config")
     .arg(deno_config_path())
     .arg("--no-lock")