Soundness issues with cppgc

[0f-0b]

• try_unwrap_cppgc_object relies on the tag field in CppGcObject<T> being located at a constant offset. However, the order of tag and member may change depending on T. This makes it possible to implement transmute for 'static types without unsafe.

  pub fn transmute_static<Src: 'static, Dst: 'static>(src: Src) -> Dst {
    use std::any::{Any, TypeId};
    use std::cell::Cell;
  
    use deno_core::cppgc::make_cppgc_object;
    use deno_core::{op2, v8, JsRuntime, OpState, RuntimeOptions};
  
    #[repr(C, align(32))]
    struct SrcObject<Src> {
      tag: TypeId,
      value: Cell<Option<Box<Src>>>,
    }
  
    #[op2]
    fn op_take_src<'a, Src: Any>(
      scope: &mut v8::HandleScope<'a>,
      state: &mut OpState,
    ) -> v8::Local<'a, v8::Object> {
      make_cppgc_object(scope, state.take::<SrcObject<Src>>())
    }
  
    #[op2(fast)]
    fn op_put_dst<Dst: Any>(state: &mut OpState, #[cppgc] dst: &Cell<Option<Box<Dst>>>) {
      state.put(*dst.take().unwrap())
    }
  
    deno_core::extension!(
      transmute,
      parameters = [Src: Any, Dst: Any],
      ops = [op_take_src<Src>, op_put_dst<Dst>],
      esm_entry_point = "ext:transmute",
      esm = ["ext:transmute" = {
        source = r#"
          import { op_put_dst, op_take_src } from "ext:core/ops";
          op_put_dst(op_take_src());
        "#
      }],
      options = {
        src: Src,
      },
      state = |state, options| {
        state.put(SrcObject {
          tag: TypeId::of::<Cell<Option<Box<Dst>>>>(),
          value: Cell::new(Some(Box::new(options.src))),
        });
      },
    );
  
    let mut runtime = JsRuntime::new(RuntimeOptions {
      extensions: vec![transmute::init_ops_and_esm::<Src, Dst>(src)],
      ..Default::default()
    });
    let state_rc = runtime.op_state();
    let mut state = state_rc.borrow_mut();
    state.take()
  }

• try_unwrap_cppgc_object makes the assumption that no other code can create an object with internal fields containing pointer values, but neither set_internal_field_count nor set_aligned_pointer_in_internal_field is marked unsafe. This again leads to an "entirely safe" transmute.

  pub fn transmute_static_2<Src: 'static, Dst: 'static>(src: Src) -> Dst {
    use std::any::{Any, TypeId};
    use std::cell::Cell;
    use std::ptr;
  
    use deno_core::{op2, v8, JsRuntime, OpState, RuntimeOptions};
  
    #[repr(C)]
    struct SrcObject<Src> {
      tag: TypeId,
      value: Cell<Option<Box<Src>>>,
    }
  
    #[op2]
    fn op_take_src<'a, Src: Any>(
      scope: &mut v8::HandleScope<'a>,
      state: &mut OpState,
    ) -> v8::Local<'a, v8::Object> {
      #[repr(C)]
      struct InnerMember<T> {
        inner: [usize; 2],
        ptr: Box<T>,
      }
  
      let member = Box::new(InnerMember {
        inner: [0, 0],
        ptr: Box::new(state.take::<SrcObject<Src>>()),
      });
      let templ = v8::ObjectTemplate::new(scope);
      templ.set_internal_field_count(2);
      let obj = templ.new_instance(scope).unwrap();
      obj.set_aligned_pointer_in_internal_field(0, ptr::from_ref(&0xde90).cast());
      obj.set_aligned_pointer_in_internal_field(1, Box::into_raw(member).cast());
      obj
    }
  
    #[op2(fast)]
    fn op_put_dst<Dst: Any>(state: &mut OpState, #[cppgc] dst: &Cell<Option<Box<Dst>>>) {
      state.put(*dst.take().unwrap())
    }
  
    deno_core::extension!(
      transmute,
      parameters = [Src: Any, Dst: Any],
      ops = [op_take_src<Src>, op_put_dst<Dst>],
      esm_entry_point = "ext:transmute",
      esm = ["ext:transmute" = {
        source = r#"
          import { op_put_dst, op_take_src } from "ext:core/ops";
          op_put_dst(op_take_src());
        "#
      }],
      options = {
        src: Src,
      },
      state = |state, options| {
        state.put(SrcObject {
          tag: TypeId::of::<Cell<Option<Box<Dst>>>>(),
          value: Cell::new(Some(Box::new(options.src))),
        });
      },
    );
  
    let mut runtime = JsRuntime::new(RuntimeOptions {
      extensions: vec![transmute::init_ops_and_esm::<Src, Dst>(src)],
      ..Default::default()
    });
    let state_rc = runtime.op_state();
    let mut state = state_rc.borrow_mut();
    state.take()
  }

[mmastrac]

Looks like we should make make_garbage_collected and v8::cppgc::GarbageCollected unsafe in rusty_v8. CppGcObject should definitely be repr(C).

[littledivy]

try_unwrap_cppgc_object should be marked unsafe. I don't see why we need to make make_garbage_collected unsafe though.

[0f-0b]

I have edited the code above to not use make_garbage_collected. I also found that a segfault can be triggered by the following innocent-looking code. Is this maybe a V8 bug?

fn main() {
  let platform = v8::new_default_platform(0, false).make_shared();
  v8::V8::initialize_platform(platform.clone());
  v8::V8::initialize();
  v8::cppgc::initalize_process(platform.clone());

  let isolate = &mut v8::Isolate::new(v8::CreateParams::default());
  let heap = v8::cppgc::Heap::create(
    platform,
    v8::cppgc::HeapCreateParams::new(v8::cppgc::WrapperDescriptor::new(0, 1, 0xf7ba)),
  );
  isolate.attach_cpp_heap(&heap);

  let handle_scope = &mut v8::HandleScope::new(isolate);
  let context = v8::Context::new(handle_scope);
  let scope = &mut v8::ContextScope::new(handle_scope, context);

  let templ = v8::ObjectTemplate::new(scope);
  templ.set_internal_field_count(2);
  let obj = templ.new_instance(scope).unwrap();
  obj.set_internal_field(0, v8::Integer::new(scope, -1).into());
  obj.set_internal_field(1, v8::Integer::new(scope, -1).into());

  scope.low_memory_notification();
}

Update: WrapperDescriptor is deprecated now. There is no other safe API that can cause integers in internal fields to be reinterpreted as pointers, so this particular segfault can be considered fixed.