safety(neon): Improve safety of JsBox on Node-API >= 8

Generate a unique type id for the crate and tag externals

crates/neon/Cargo.toml

@@ -25,6 +25,7 @@ semver = "0.9.0"
 smallvec = "1.4.2"
 once_cell = "1.10.0"
 neon-macros = { version = "=0.10.1", path = "../neon-macros" }
+rand = { version = "0.8.5", optional = true }
 
 [features]
 default = ["napi-1"]
@@ -45,7 +46,7 @@ napi-4 = ["napi-3"]
 napi-5 = ["napi-4"]
 napi-6 = ["napi-5"]
 napi-7 = ["napi-6"]
-napi-8 = ["napi-7"]
+napi-8 = ["napi-7", "rand"]
 napi-latest = ["napi-8"]
 napi-experimental = ["napi-8"]
 

crates/neon/src/lib.rs

@@ -98,3 +98,10 @@ pub use neon_macros::*;
 
 #[cfg(feature = "napi-6")]
 mod lifecycle;
+
+#[cfg(feature = "napi-8")]
+static MODULE_TAG: once_cell::sync::Lazy<crate::sys::TypeTag> = once_cell::sync::Lazy::new(|| {
+    let (lower, upper) = rand::random::<(u64, u64)>();
+
+    crate::sys::TypeTag { lower, upper }
+});

crates/neon/src/object/mod.rs

@@ -36,7 +36,7 @@ use crate::{
     context::{internal::Env, Context},
     handle::{Handle, Managed, Root},
     result::{NeonResult, Throw},
-    sys::{self, raw, Status},
+    sys::{self, raw},
     types::{build, function::CallOptions, utf8::Utf8, JsFunction, JsUndefined, JsValue, Value},
 };
 
@@ -194,8 +194,8 @@ pub trait Object: Value {
         let obj = self.to_raw();
         unsafe {
             match sys::object::freeze(env, obj) {
-                Status::Ok => Ok(self),
-                Status::PendingException => Err(Throw::new()),
+                sys::Status::Ok => Ok(self),
+                sys::Status::PendingException => Err(Throw::new()),
                 _ => cx.throw_type_error("object cannot be frozen"),
             }
         }
@@ -207,8 +207,8 @@ pub trait Object: Value {
         let obj = self.to_raw();
         unsafe {
             match sys::object::seal(env, obj) {
-                Status::Ok => Ok(self),
-                Status::PendingException => Err(Throw::new()),
+                sys::Status::Ok => Ok(self),
+                sys::Status::PendingException => Err(Throw::new()),
                 _ => cx.throw_type_error("object cannot be sealed"),
             }
         }

crates/neon/src/sys/bindings/functions.rs

@@ -335,6 +335,13 @@ mod napi8 {
         extern "C" {
             fn object_freeze(env: Env, object: Value) -> Status;
             fn object_seal(env: Env, object: Value) -> Status;
+            fn type_tag_object(env: Env, object: Value, tag: *const TypeTag) -> Status;
+            fn check_object_type_tag(
+                env: Env,
+                object: Value,
+                tag: *const TypeTag,
+                result: *mut bool,
+            ) -> Status;
         }
     );
 }

crates/neon/src/sys/bindings/types.rs

@@ -226,3 +226,11 @@ pub struct Deferred__ {
 }
 
 pub type Deferred = *mut Deferred__;
+
+#[cfg(feature = "napi-8")]
+#[repr(C)]
+#[derive(Debug, Copy, Clone)]
+pub struct TypeTag {
+    pub(crate) lower: u64,
+    pub(crate) upper: u64,
+}

crates/neon/src/sys/external.rs

@@ -35,14 +35,17 @@ pub unsafe fn deref<T: Send + 'static>(env: Env, local: Local) -> Option<*const
 
     let result = result.assume_init();
 
-    // Note: This only validates it is an external, not that it was created by
-    // this module. In this future, this can be improved with type tagging:
-    // https://nodejs.org/api/n-api.html#n_api_napi_type_tag
-    // https://github.com/neon-bindings/neon/issues/591
+    // Ensure we have an external
     if result != napi::ValueType::External {
         return None;
     }
 
+    #[cfg(feature = "napi-8")]
+    // Check the external came from this module
+    if !super::tag::check_object_type_tag(env, local, &*crate::MODULE_TAG) {
+        return None;
+    }
+
     let mut result = MaybeUninit::uninit();
     let status = napi::get_value_external(env, local, result.as_mut_ptr());
 
@@ -70,5 +73,11 @@ pub unsafe fn create<T: Send + 'static>(env: Env, v: T, finalizer: fn(Env, T)) -
     // or shutting down.
     assert_eq!(status, napi::Status::Ok);
 
-    result.assume_init()
+    let external = result.assume_init();
+
+    #[cfg(feature = "napi-8")]
+    // Tag the object as coming from this module
+    super::tag::type_tag_object(env, external, &*crate::MODULE_TAG);
+
+    external
 }

crates/neon/src/sys/no_panic.rs

@@ -240,6 +240,7 @@ unsafe fn panic_msg(panic: &Panic) -> Option<&str> {
 }
 
 unsafe fn external_from_panic(env: Env, panic: Panic) -> Local {
+    let fail = || fatal_error("Failed to create a neon::types::JsBox from a panic");
     let mut result = MaybeUninit::uninit();
     let status = napi::create_external(
         env,
@@ -250,10 +251,17 @@ unsafe fn external_from_panic(env: Env, panic: Panic) -> Local {
     );
 
     if status != napi::Status::Ok {
-        fatal_error("Failed to create a neon::types::JsBox from a panic");
+        fail();
     }
 
-    result.assume_init()
+    let external = result.assume_init();
+
+    #[cfg(feature = "napi-8")]
+    if napi::type_tag_object(env, external, &*crate::MODULE_TAG) != napi::Status::Ok {
+        fail();
+    }
+
+    external
 }
 
 extern "C" fn finalize_panic(_env: Env, data: *mut c_void, _hint: *mut c_void) {

crates/neon/src/sys/tag.rs

@@ -114,3 +114,21 @@ pub unsafe fn is_promise(env: Env, val: Local) -> bool {
     );
     result
 }
+
+#[cfg(feature = "napi-8")]
+pub unsafe fn type_tag_object(env: Env, object: Local, tag: &super::TypeTag) {
+    assert_eq!(
+        napi::type_tag_object(env, object, tag as *const _),
+        napi::Status::Ok
+    );
+}
+
+#[cfg(feature = "napi-8")]
+pub unsafe fn check_object_type_tag(env: Env, object: Local, tag: &super::TypeTag) -> bool {
+    let mut result = false;
+    assert_eq!(
+        napi::check_object_type_tag(env, object, tag as *const _, &mut result as *mut _),
+        napi::Status::Ok
+    );
+    result
+}