diff --git a/packages/yew/src/functional/hooks/use_reducer.rs b/packages/yew/src/functional/hooks/use_reducer.rs index db81c38179d..eada5a7100e 100644 --- a/packages/yew/src/functional/hooks/use_reducer.rs +++ b/packages/yew/src/functional/hooks/use_reducer.rs @@ -35,7 +35,12 @@ pub struct UseReducerHandle where T: Reducible, { - value: Rc, + /// Shared source of truth, updated synchronously by dispatch. + current_state: Rc>>, + /// Accumulates `Rc` clones returned by [`Deref::deref`] so that references + /// remain valid for the lifetime of this handle. Reset on each re-render when + /// a new handle is created. + deref_history: RefCell>>, dispatch: DispatchFn, } @@ -63,7 +68,34 @@ where type Target = T; fn deref(&self) -> &Self::Target { - &self.value + let rc = match self.current_state.try_borrow() { + Ok(shared) => Rc::clone(&*shared), + Err(_) => { + // RefCell is mutably borrowed (during dispatch). Use the last + // value we successfully read. + let history = self.deref_history.borrow(); + Rc::clone(history.last().expect("deref_history is never empty")) + } + }; + + let ptr: *const T = Rc::as_ptr(&rc); + + // Only store a new entry when the Rc allocation differs from the most + // recent one, avoiding unbounded growth from repeated reads of the same + // state. + { + let mut history = self.deref_history.borrow_mut(); + if !Rc::ptr_eq(history.last().expect("deref_history is never empty"), &rc) { + history.push(rc); + } + } + + // SAFETY: `ptr` points into the heap allocation of an `Rc`. That Rc + // is kept alive in `self.deref_history` (either the entry we just pushed, + // or a previous entry with the same allocation). `deref_history` lives as + // long as `self`, and `Rc` guarantees its heap allocation stays live while + // any clone exists. Therefore `ptr` is valid for the lifetime of `&self`. + unsafe { &*ptr } } } @@ -72,8 +104,17 @@ where T: Reducible, { fn clone(&self) -> Self { + // Take a fresh snapshot so the clone's deref_history is never empty. + let snapshot = match self.current_state.try_borrow() { + Ok(shared) => Rc::clone(&*shared), + Err(_) => { + let history = self.deref_history.borrow(); + Rc::clone(history.last().expect("deref_history is never empty")) + } + }; Self { - value: Rc::clone(&self.value), + current_state: Rc::clone(&self.current_state), + deref_history: RefCell::new(vec![snapshot]), dispatch: Rc::clone(&self.dispatch), } } @@ -84,8 +125,17 @@ where T: Reducible + fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let value = if let Ok(rc_ref) = self.current_state.try_borrow() { + format!("{:?}", *rc_ref) + } else { + let history = self.deref_history.borrow(); + format!( + "{:?}", + **history.last().expect("deref_history is never empty") + ) + }; f.debug_struct("UseReducerHandle") - .field("value", &format!("{:?}", self.value)) + .field("value", &value) .finish() } } @@ -95,7 +145,7 @@ where T: Reducible + PartialEq, { fn eq(&self, rhs: &Self) -> bool { - self.value == rhs.value + **self == **rhs } } @@ -239,10 +289,15 @@ where } }); - let value = state.current_state.borrow().clone(); + let current_state = state.current_state.clone(); + let snapshot = state.current_state.borrow().clone(); let dispatch = state.dispatch.clone(); - UseReducerHandle { value, dispatch } + UseReducerHandle { + current_state, + deref_history: RefCell::new(vec![snapshot]), + dispatch, + } } } diff --git a/packages/yew/src/functional/hooks/use_state.rs b/packages/yew/src/functional/hooks/use_state.rs index 61b4c57d71a..66638296a08 100644 --- a/packages/yew/src/functional/hooks/use_state.rs +++ b/packages/yew/src/functional/hooks/use_state.rs @@ -109,7 +109,7 @@ pub struct UseStateHandle { impl fmt::Debug for UseStateHandle { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("UseStateHandle") - .field("value", &format!("{:?}", self.inner.value)) + .field("value", &format!("{:?}", **self)) .finish() } } @@ -132,7 +132,7 @@ impl Deref for UseStateHandle { type Target = T; fn deref(&self) -> &Self::Target { - &(*self.inner).value + &self.inner.value } } diff --git a/packages/yew/tests/use_state.rs b/packages/yew/tests/use_state.rs index 1a04c0ce27d..9f3cdcbf126 100644 --- a/packages/yew/tests/use_state.rs +++ b/packages/yew/tests/use_state.rs @@ -106,3 +106,229 @@ async fn use_state_eq_works() { assert_eq!(result.as_str(), "1"); assert_eq!(RENDER_COUNT.load(Ordering::Relaxed), 2); } + +/// Exercises the exact pattern that causes use-after-free in the original PR #3963 +/// fix, where `UseReducerHandle::deref()` drops the `Ref` guard but returns a +/// pointer derived from it. +/// +/// The dangerous sequence within a single callback: +/// 1. `handle.set(v1)` — dispatch puts a *new* `Rc` (refcount=1) in the shared `RefCell`, +/// replacing the one from render time. +/// 2. `let r: &T = &*handle` — `deref()` borrows the RefCell, grabs a raw pointer into the Rc +/// (refcount still 1), and **drops the `Ref` guard**. +/// 3. `handle.set(v2)` — dispatch replaces that Rc. Because its refcount was 1, it is freed. `r` +/// is now dangling. +/// 4. Allocate objects of similar size to encourage the allocator to reuse the freed memory, +/// overwriting the old `T`. +/// 5. Read through `r` — **use-after-free**. +/// +/// With the `deref_history` fix, step 2 clones the Rc into a `Vec` kept alive by +/// the handle, bumping the refcount to 2. Step 3 only drops it to 1, so the +/// allocation survives and `r` remains valid. +#[wasm_bindgen_test] +async fn deref_remains_valid_across_multiple_dispatches_in_callback() { + use std::cell::RefCell; + + use gloo::utils::document; + use wasm_bindgen::JsCast; + use web_sys::HtmlElement; + + thread_local! { + static DEREF_RESULT: RefCell> = const { RefCell::new(None) }; + } + + #[component(UBTestComponent)] + fn ub_test_comp() -> Html { + let state = use_state(|| "initial".to_string()); + + let trigger = { + let state = state.clone(); + Callback::from(move |_| { + // Step 1: dispatch. The RefCell now contains a *new* Rc whose only + // owner is the RefCell itself (refcount = 1). + state.set("first_dispatch".to_string()); + + // Step 2: deref. In the original fix the Ref guard is dropped + // immediately, leaving us with a bare pointer into the refcount-1 + // Rc. With deref_history, the Rc is cloned into the Vec so the + // refcount is bumped to 2. + let borrowed: &String = &*state; + + // Step 3: dispatch again. The RefCell's old Rc is replaced. + // Original fix: refcount was 1 → drops to 0 → freed → `borrowed` + // dangles. + // deref_history fix: refcount was 2 → drops to 1 (still in Vec) + // → allocation survives → `borrowed` is valid. + state.set("second_dispatch".to_string()); + + // Step 4: churn the allocator. Create and drop many heap objects + // of ~32 bytes (the size of the freed Rc+UseStateReducer+String + // struct on wasm32) to maximize the chance that the allocator + // hands out the freed address to one of these, overwriting the + // memory `borrowed` points into. + for _ in 0..256 { + // Each Box<[u8; 32]> is roughly the same size as the freed Rc + // allocation containing UseStateReducer. + let overwrite = Box::new([0xFFu8; 32]); + std::hint::black_box(&*overwrite); + drop(overwrite); + } + + // Also allocate Strings whose *buffers* might reuse the freed + // String buffer from step 1. + let _noise: Vec = (0..64).map(|i| format!("noise_{:032}", i)).collect(); + + // Step 5: read through the potentially-dangling reference. + // With the original fix this is UB: the memory behind `borrowed` + // may have been reused by the allocations above, so `.clone()` + // could read a garbage ptr/len/cap triple and trap, or silently + // return corrupted data. + // With deref_history, this always reads "first_dispatch". + let value = borrowed.clone(); + + DEREF_RESULT.with(|r| { + *r.borrow_mut() = Some(value); + }); + }) + }; + + html! { +
+ +
{(*state).clone()}
+
+ } + } + + yew::Renderer::::with_root(document().get_element_by_id("output").unwrap()) + .render(); + sleep(Duration::ZERO).await; + + // Fire the callback + document() + .get_element_by_id("ub-trigger") + .unwrap() + .unchecked_into::() + .click(); + + sleep(Duration::ZERO).await; + + // The reference obtained between the two dispatches must still read the + // value from the first dispatch, not garbage or "second_dispatch". + let captured = DEREF_RESULT.with(|r| r.borrow().clone()); + assert_eq!( + captured, + Some("first_dispatch".to_string()), + "deref() reference must remain valid across subsequent dispatches" + ); +} + +/// Regression test for issue #3796 +/// Tests that state handles always read the latest value even when accessed +/// from callbacks before a rerender occurs. +/// +/// The bug occurred when: +/// 1. State A is updated via set() +/// 2. State B is updated via set() +/// 3. A callback reads both states before rerender +/// 4. The callback would see stale value for B because the handle was caching a snapshot instead of +/// reading from the shared RefCell +#[wasm_bindgen_test] +async fn use_state_handles_read_latest_value_issue_3796() { + use std::cell::RefCell; + + use gloo::utils::document; + use wasm_bindgen::JsCast; + use web_sys::HtmlElement; + + // Shared storage for the values read by the submit handler + thread_local! { + static CAPTURED_VALUES: RefCell> = const { RefCell::new(None) }; + } + + #[component(FormComponent)] + fn form_comp() -> Html { + let field_a = use_state(String::new); + let field_b = use_state(String::new); + + let update_a = { + let field_a = field_a.clone(); + Callback::from(move |_| { + field_a.set("value_a".to_string()); + }) + }; + + let update_b = { + let field_b = field_b.clone(); + Callback::from(move |_| { + field_b.set("value_b".to_string()); + }) + }; + + // This callback reads both states - the bug caused field_b to be stale + let submit = { + let field_a = field_a.clone(); + let field_b = field_b.clone(); + Callback::from(move |_| { + let a = (*field_a).clone(); + let b = (*field_b).clone(); + CAPTURED_VALUES.with(|v| { + *v.borrow_mut() = Some((a.clone(), b.clone())); + }); + }) + }; + + html! { +
+ + + +
{format!("a={}, b={}", *field_a, *field_b)}
+
+ } + } + + yew::Renderer::::with_root(document().get_element_by_id("output").unwrap()) + .render(); + sleep(Duration::ZERO).await; + + // Initial state + let result = obtain_result(); + assert_eq!(result.as_str(), "a=, b="); + + // Click update-a, then update-b, then submit WITHOUT waiting for rerender. + // This simulates rapid user interaction (like the Firefox bug in issue #3796). + document() + .get_element_by_id("update-a") + .unwrap() + .unchecked_into::() + .click(); + + document() + .get_element_by_id("update-b") + .unwrap() + .unchecked_into::() + .click(); + + document() + .get_element_by_id("submit") + .unwrap() + .unchecked_into::() + .click(); + + // Now wait for rerenders to complete + sleep(Duration::ZERO).await; + + // Check the values captured by the submit handler. + // Before the fix, field_b would be empty because the callback captured a stale handle. + let captured = CAPTURED_VALUES.with(|v| v.borrow().clone()); + assert_eq!( + captured, + Some(("value_a".to_string(), "value_b".to_string())), + "Submit handler should see latest values for both fields" + ); + + // Also verify the DOM shows correct values after rerender + let result = obtain_result(); + assert_eq!(result.as_str(), "a=value_a, b=value_b"); +}