[bug] StateManager is unsound

[WSH032]

Describe the bug

fn foo(app: tauri::App) {
    use tauri::Manager;

    #[derive(Debug)]
    struct Foo(i32);

    app.manage(Foo(0));
    let state = app.state::<Foo>().inner();
    let mut returned_state = app.unmanage::<Foo>().unwrap();
    returned_state.0 = 42;

    println!("{}", state.0); // 👈 encountered a dangling reference (use-after-free)
}

While retaining a reference to the state, removing the managed state from the manager will result in a dangling pointer.

Reproduction

Here is an equivalent implementation, run it with miri (miri cannot directly run tauri):

use std::{
    any::{Any, TypeId},
    cell::UnsafeCell,
    collections::HashMap,
    hash::BuildHasherDefault,
    sync::{Arc, Mutex},
};

/// A guard for a state value.
///
/// See [`Manager::manage`](`crate::Manager::manage`) for usage examples.
pub struct State<'r, T: Send + Sync + 'static>(&'r T);

impl<'r, T: Send + Sync + 'static> State<'r, T> {
    /// Retrieve a borrow to the underlying value with a lifetime of `'r`.
    /// Using this method is typically unnecessary as `State` implements
    /// [`std::ops::Deref`] with a [`std::ops::Deref::Target`] of `T`.
    #[inline(always)]
    pub fn inner(&self) -> &'r T {
        self.0
    }
}

impl<T: Send + Sync + 'static> std::ops::Deref for State<'_, T> {
    type Target = T;

    #[inline(always)]
    fn deref(&self) -> &T {
        self.0
    }
}

impl<T: Send + Sync + 'static> Clone for State<'_, T> {
    fn clone(&self) -> Self {
        State(self.0)
    }
}

impl<T: Send + Sync + 'static + PartialEq> PartialEq for State<'_, T> {
    fn eq(&self, other: &Self) -> bool {
        self.0 == other.0
    }
}

impl<T: Send + Sync + std::fmt::Debug> std::fmt::Debug for State<'_, T> {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        f.debug_tuple("State").field(&self.0).finish()
    }
}

#[derive(Default)]
struct IdentHash(u64);

impl std::hash::Hasher for IdentHash {
    fn finish(&self) -> u64 {
        self.0
    }

    fn write(&mut self, bytes: &[u8]) {
        for byte in bytes {
            self.write_u8(*byte);
        }
    }

    fn write_u8(&mut self, i: u8) {
        self.0 = (self.0 << 8) | (i as u64);
    }

    fn write_u64(&mut self, i: u64) {
        self.0 = i;
    }
}

type TypeIdMap = HashMap<TypeId, Box<dyn Any>, BuildHasherDefault<IdentHash>>;

/// The Tauri state manager.
#[derive(Debug)]
pub struct StateManager {
    map: Mutex<UnsafeCell<TypeIdMap>>,
}

// SAFETY: data is accessed behind a lock
unsafe impl Sync for StateManager {}
unsafe impl Send for StateManager {}

impl StateManager {
    pub(crate) fn new() -> Self {
        Self {
            map: Default::default(),
        }
    }

    fn with_map_ref<'a, F: FnOnce(&'a TypeIdMap) -> R, R>(&'a self, f: F) -> R {
        let map = self.map.lock().unwrap();
        let map = map.get();
        // SAFETY: safe to access since we are holding a lock
        f(unsafe { &*map })
    }

    fn with_map_mut<F: FnOnce(&mut TypeIdMap) -> R, R>(&self, f: F) -> R {
        let mut map = self.map.lock().unwrap();
        let map = map.get_mut();
        f(map)
    }

    pub(crate) fn set<T: Send + Sync + 'static>(&self, state: T) -> bool {
        self.with_map_mut(|map| {
            let type_id = TypeId::of::<T>();
            let already_set = map.contains_key(&type_id);
            if !already_set {
                map.insert(type_id, Box::new(state) as Box<dyn Any>);
            }
            !already_set
        })
    }

    pub(crate) fn unmanage<T: Send + Sync + 'static>(&self) -> Option<T> {
        self.with_map_mut(|map| {
            let type_id = TypeId::of::<T>();
            map.remove(&type_id)
                .and_then(|ptr| ptr.downcast().ok().map(|b| *b))
        })
    }

    /// Gets the state associated with the specified type.
    pub fn get<T: Send + Sync + 'static>(&self) -> State<'_, T> {
        self.try_get()
            .expect("state: get() when given type is not managed")
    }

    /// Gets the state associated with the specified type.
    pub fn try_get<T: Send + Sync + 'static>(&self) -> Option<State<'_, T>> {
        self.with_map_ref(|map| {
            map.get(&TypeId::of::<T>())
                .and_then(|ptr| ptr.downcast_ref::<T>())
                .map(State)
        })
    }
}

fn main() {
    #[derive(Debug)]
    struct Foo(i32);

    let state_manager = Arc::new(StateManager::new());

    state_manager.set(Foo(0)); // manage the state
    let state = state_manager.try_get::<Foo>().unwrap().inner(); // then get the state
    let mut returned_state = state_manager.unmanage::<Foo>().unwrap(); // unmanage the state
    returned_state.0 = 42;

    println!("{}", state.0); // 👈 encountered a dangling reference (use-after-free)
}

Expected behavior

The unsoundness comes from this line:

tauri/crates/tauri/src/state.rs

Line 123 in 3f68058

f(unsafe { &*map })

.

It allows retaining a reference to the Hashmap value after the MutexGuard is dropped.

Full tauri info output

rust 1.82 stable
tauri 2.2.5

Stack trace

cargo +nightly miri run

error: Undefined Behavior: out-of-bounds pointer use: alloc1105 has been freed, so this pointer is dangling
   --> src/main.rs:152:20
    |
152 |     println!("{}", state.0); // 👈 encountered a dangling reference (use-after-free)
    |                    ^^^^^^^ out-of-bounds pointer use: alloc1105 has been freed, so this pointer is dangling
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
help: alloc1105 was allocated here:
   --> src/main.rs:111:37
    |
    |                                     ^^^^^^^^^^^^^^^
help: alloc1105 was deallocated here:
   --> src/main.rs:121:62
    |
121 |                 .and_then(|ptr| ptr.downcast().ok().map(|b| *b))
    |                                                              ^
    = note: BACKTRACE (of the first span):
    = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

Additional context

No response

[WSH032]

Due to semantic compatibility, it seems that it can't be fixed 😰, especially since users can still hold &Foo after drop(state) (via State::inner).

It seems we can only deprecate Manager::unmanage to let users know about its unsoundness.