From 242e4cf97fd846ece72b78dfcd571ba55d812f34 Mon Sep 17 00:00:00 2001
From: Timo <30553356+y21@users.noreply.github.com>
Date: Wed, 11 May 2022 02:22:10 +0200
Subject: [PATCH] Fix misuse of `MaybeUninit` and avoid refs to uninit memory
 (#954)

---
 src/inspector.rs          |  9 +++++++--
 src/scope.rs              | 14 ++++++--------
 src/value_deserializer.rs |  7 ++++---
 src/value_serializer.rs   |  7 ++++---
 4 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/src/inspector.rs b/src/inspector.rs
index 1ab4ec70..38084738 100644
--- a/src/inspector.rs
+++ b/src/inspector.rs
@@ -269,7 +269,8 @@ impl ChannelBase {
 
   fn get_cxx_base_offset() -> FieldOffset<Channel> {
     let buf = std::mem::MaybeUninit::<Self>::uninit();
-    FieldOffset::from_ptrs(buf.as_ptr(), unsafe { &(*buf.as_ptr()).cxx_base })
+    let base = unsafe { addr_of!((*buf.as_ptr()).cxx_base) };
+    FieldOffset::from_ptrs(buf.as_ptr(), base)
   }
 
   fn get_offset_within_embedder<T>() -> FieldOffset<Self>
@@ -278,6 +279,8 @@ impl ChannelBase {
   {
     let buf = std::mem::MaybeUninit::<T>::uninit();
     let embedder_ptr: *const T = buf.as_ptr();
+    // TODO(y21): the call to base() creates a reference to uninitialized memory (UB)
+    // fixing this requires changes in the ChannelImpl trait, namely ChannelImpl::base() can't take &self
     let self_ptr: *const Self = unsafe { (*embedder_ptr).base() };
     FieldOffset::from_ptrs(embedder_ptr, self_ptr)
   }
@@ -532,7 +535,8 @@ impl V8InspectorClientBase {
 
   fn get_cxx_base_offset() -> FieldOffset<V8InspectorClient> {
     let buf = std::mem::MaybeUninit::<Self>::uninit();
-    FieldOffset::from_ptrs(buf.as_ptr(), unsafe { &(*buf.as_ptr()).cxx_base })
+    let base = unsafe { addr_of!((*buf.as_ptr()).cxx_base) };
+    FieldOffset::from_ptrs(buf.as_ptr(), base)
   }
 
   fn get_offset_within_embedder<T>() -> FieldOffset<Self>
@@ -668,6 +672,7 @@ use std::iter::ExactSizeIterator;
 use std::iter::IntoIterator;
 use std::marker::PhantomData;
 use std::ops::Deref;
+use std::ptr::addr_of;
 use std::ptr::null;
 use std::ptr::NonNull;
 use std::slice;
diff --git a/src/scope.rs b/src/scope.rs
index 85bf5c9e..1d2e64de 100644
--- a/src/scope.rs
+++ b/src/scope.rs
@@ -1532,15 +1532,14 @@ mod raw {
 
   #[repr(C)]
   #[derive(Debug)]
-  pub(super) struct HandleScope([usize; 3]);
+  pub(super) struct HandleScope([MaybeUninit<usize>; 3]);
 
   impl HandleScope {
+    /// Creates an uninitialized `HandleScope`.
+    ///
     /// This function is marked unsafe because the caller must ensure that the
     /// returned value isn't dropped before `init()` has been called.
     pub unsafe fn uninit() -> Self {
-      // This is safe because there is no combination of bits that would produce
-      // an invalid `[usize; 3]`.
-      #[allow(clippy::uninit_assumed_init)]
       Self(MaybeUninit::uninit().assume_init())
     }
 
@@ -1591,15 +1590,14 @@ mod raw {
 
   #[repr(C)]
   #[derive(Debug)]
-  pub(super) struct TryCatch([usize; 6]);
+  pub(super) struct TryCatch([MaybeUninit<usize>; 6]);
 
   impl TryCatch {
+    /// Creates an uninitialized `TryCatch`.
+    ///
     /// This function is marked unsafe because the caller must ensure that the
     /// returned value isn't dropped before `init()` has been called.
     pub unsafe fn uninit() -> Self {
-      // This is safe because there is no combination of bits that would produce
-      // an invalid `[usize; 6]`.
-      #[allow(clippy::uninit_assumed_init)]
       Self(MaybeUninit::uninit().assume_init())
     }
 
diff --git a/src/value_deserializer.rs b/src/value_deserializer.rs
index aa9dc034..ee68d251 100644
--- a/src/value_deserializer.rs
+++ b/src/value_deserializer.rs
@@ -17,6 +17,7 @@ use crate::support::MaybeBool;
 use std::ffi::c_void;
 use std::mem::MaybeUninit;
 use std::pin::Pin;
+use std::ptr::addr_of;
 
 // Must be == sizeof(v8::ValueDeserializer::Delegate),
 // see v8__ValueDeserializer__Delegate__CONSTRUCT().
@@ -212,9 +213,9 @@ impl<'a, 's> ValueDeserializerHeap<'a, 's> {
   fn get_cxx_value_deserializer_delegate_offset(
   ) -> FieldOffset<CxxValueDeserializerDelegate> {
     let buf = std::mem::MaybeUninit::<Self>::uninit();
-    FieldOffset::from_ptrs(buf.as_ptr(), unsafe {
-      &(*buf.as_ptr()).cxx_value_deserializer_delegate
-    })
+    let delegate =
+      unsafe { addr_of!((*buf.as_ptr()).cxx_value_deserializer_delegate) };
+    FieldOffset::from_ptrs(buf.as_ptr(), delegate)
   }
 
   /// Starting from 'this' pointer a ValueDeserializerHeap ref can be created
diff --git a/src/value_serializer.rs b/src/value_serializer.rs
index 9d6b866d..fa703b9b 100644
--- a/src/value_serializer.rs
+++ b/src/value_serializer.rs
@@ -15,6 +15,7 @@ use std::alloc::dealloc;
 use std::alloc::realloc;
 use std::alloc::Layout;
 use std::mem::MaybeUninit;
+use std::ptr::addr_of;
 
 use crate::support::CxxVTable;
 use crate::support::FieldOffset;
@@ -274,9 +275,9 @@ impl<'a, 's> ValueSerializerHeap<'a, 's> {
   fn get_cxx_value_serializer_delegate_offset(
   ) -> FieldOffset<CxxValueSerializerDelegate> {
     let buf = std::mem::MaybeUninit::<Self>::uninit();
-    FieldOffset::from_ptrs(buf.as_ptr(), unsafe {
-      &(*buf.as_ptr()).cxx_value_serializer_delegate
-    })
+    let delegate =
+      unsafe { addr_of!((*buf.as_ptr()).cxx_value_serializer_delegate) };
+    FieldOffset::from_ptrs(buf.as_ptr(), delegate)
   }
 
   /// Starting from 'this' pointer a ValueSerializerHeap ref can be created