Skip to content

Commit

Permalink
Merge pull request #797 from neon-bindings/unforgeable-throw
Browse files Browse the repository at this point in the history
Make it harder to forge `Throw` tokens
  • Loading branch information
dherman authored Mar 3, 2022
2 parents fd2bc3f + 074d0e2 commit 79c983c
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ pub trait Context<'a>: ContextInternal<'a> {
unsafe {
neon_runtime::error::throw(self.env().to_raw(), v.to_raw());
}
Err(Throw)
Err(Throw::new())
}

/// Creates a direct instance of the [`Error`](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Error) class.
Expand Down
2 changes: 1 addition & 1 deletion src/object/class/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl ConstructorCallCallback {
);
}
}
Err(Throw)
Err(Throw::new())
}

ConstructorCallCallback(callback::<T>)
Expand Down
6 changes: 3 additions & 3 deletions src/object/class/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ pub(crate) trait ClassInternal: Class {
);

if metadata_pointer.is_null() {
return Err(Throw);
return Err(Throw::new());
}

// NOTE: None of the error cases below need to delete the ClassMetadata object, since the
Expand All @@ -186,7 +186,7 @@ pub(crate) trait ClassInternal: Class {
class_name.as_ptr(),
class_name.len() as u32,
) {
return Err(Throw);
return Err(Throw::new());
}

for (name, method) in descriptor.methods {
Expand All @@ -201,7 +201,7 @@ pub(crate) trait ClassInternal: Class {
name.len() as u32,
method.to_raw(),
) {
return Err(Throw);
return Err(Throw::new());
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/object/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ mod traits {
if unsafe { key.set_from(&mut result, self.to_raw(), val.to_raw()) } {
Ok(result)
} else {
Err(Throw)
Err(Throw::new())
}
}
}
Expand Down Expand Up @@ -268,7 +268,7 @@ mod traits {
if unsafe { key.set_from(cx, &mut result, self.to_raw(), val.to_raw()) } {
Ok(result)
} else {
Err(Throw)
Err(Throw::new())
}
}

Expand Down
9 changes: 8 additions & 1 deletion src/result/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use crate::context::Context;
use crate::handle::Handle;
use crate::types::Value;
use std::fmt::{Display, Formatter, Result as FmtResult};
use std::marker::PhantomData;

/// A [unit type][unit] indicating that the JavaScript thread is throwing an exception.
///
Expand All @@ -47,7 +48,13 @@ use std::fmt::{Display, Formatter, Result as FmtResult};
///
/// [unit]: https://doc.rust-lang.org/book/ch05-01-defining-structs.html#unit-like-structs-without-any-fields
#[derive(Debug)]
pub struct Throw;
pub struct Throw(PhantomData<*mut ()>); // *mut is !Send + !Sync, making it harder to accidentally store

impl Throw {
pub(crate) fn new() -> Self {
Self(PhantomData)
}
}

impl Display for Throw {
fn fmt(&self, fmt: &mut Formatter) -> FmtResult {
Expand Down
2 changes: 1 addition & 1 deletion src/types/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ pub(crate) fn convert_panics<T, F: UnwindSafe + FnOnce() -> NeonResult<T>>(
#[cfg(feature = "napi-1")]
neon_runtime::error::clear_exception(env.to_raw());
neon_runtime::error::throw_error_from_utf8(env.to_raw(), data, len);
Err(Throw)
Err(Throw::new())
}
}
}
Expand Down
9 changes: 8 additions & 1 deletion src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,14 @@
//! cx: &mut impl Context<'a>,
//! object: Handle<'a, JsObject>
//! ) -> JsResult<'a, JsArray> {
//! # #[cfg(feature = "legacy-runtime")]
//! # return
//! # object.downcast().or_throw(cx)
//! # ;
//! # #[cfg(feature = "napi-1")]
//! # return
//! object.downcast(cx).or_throw(cx)
//! # ;
//! }
//! # }
//! ```
Expand Down Expand Up @@ -127,7 +134,7 @@ pub(crate) fn build<'a, T: Managed, F: FnOnce(&mut raw::Local) -> bool>(
if init(&mut local) {
Ok(Handle::new_internal(T::from_raw(env, local)))
} else {
Err(Throw)
Err(Throw::new())
}
}
}
Expand Down
34 changes: 33 additions & 1 deletion test/dynamic/native/src/js/functions.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::cell::RefCell;

use neon::object::This;
use neon::prelude::*;
use neon::result::Throw;
Expand Down Expand Up @@ -140,8 +142,38 @@ pub fn panic_and_catch(mut cx: FunctionContext) -> JsResult<JsValue> {
Ok(cx.try_catch(|_| panic!("oh no")).unwrap_or_else(|err| err))
}

thread_local! {
static FORGED_THROW: RefCell<Option<Throw>> = RefCell::new(None);
}

fn forge_throw(mut cx: FunctionContext) -> JsResult<JsValue> {
// Force an arbitrary JS error to temporarily enter the throwing state.
let throw = cx.throw_error::<_, ()>("forced error").unwrap_err();
// Save the throw token in thread-local storage.
FORGED_THROW.with(|forged_throw| {
*forged_throw.borrow_mut() = Some(throw);
});
panic!("get us out of here");
}

pub fn unexpected_throw_and_catch(mut cx: FunctionContext) -> JsResult<JsValue> {
Ok(cx.try_catch(|_| Err(Throw)).unwrap_or_else(|err| err))
// It's hard to forge a bogus throw token, but you can still do it by
// forcing a real throw, saving the token for later, and catching the
// throw to return the VM back out of its throwing state.
let _ = cx.try_catch(|cx| {
let forge: Handle<JsFunction> = JsFunction::new(cx, forge_throw)?;
let null: Handle<JsValue> = cx.null().upcast();
let args: Vec<Handle<JsValue>> = vec![];
forge.call(cx, null, args)?;
Ok(())
});

let retrieved_throw = FORGED_THROW.with(|forged_throw| forged_throw.borrow_mut().take());

match retrieved_throw {
Some(throw) => Ok(cx.try_catch(|_| Err(throw)).unwrap_or_else(|err| err)),
None => Ok(cx.string("failed to retrieve forged throw").upcast()),
}
}

pub fn downcast_error(mut cx: FunctionContext) -> JsResult<JsString> {
Expand Down

0 comments on commit 79c983c

Please sign in to comment.