Skip to content

Commit 045d7cb

Browse files
Disable unwinding for catch_unwind error payloads.
This does something similar to what was suggested over #86027 (comment) that is, to cheat a little bit and tweak/amend the `Box<dyn Any…>` obtained from `catch_unwind`: - keep the `.type_id()` the same to avoid breakage with downcasting; - but make it so the virtual `.drop_in_place()` is somehow overridden with a shim around the real drop glue that prevents unwinding (_e.g._, by aborting when that happens). This is achieved through the `DropNoUnwindSameAnyTypeId<T>`, wrapper: - with the very same layout as the `T` it wraps; - with an overridden/fake `.type_id()` so as to impersonate its inner `T`; - with a manual `impl Drop` which uses an abort bomb to ensure no unwinding can happen. That way, the `Box<DropNoUnwindSameAnyTypeId<T>>`, when box-erased to a `Box<dyn Any…>`, becomes, both layout-wise, and `type_id`-wise, undistinguishable from a `Box<T>`, thence avoiding any breakage. And yet, when that `Box<dyn Any…>` is implicitly dropped with `catch_unwind`s, no further unwinding can happen. Handle `resume_unwind` payloads too Clean up logic: centralize drop-override in catch_unwind & virtual method
1 parent e1d7dec commit 045d7cb

File tree

4 files changed

+127
-0
lines changed

4 files changed

+127
-0
lines changed

library/core/src/any.rs

+79
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@
154154

155155
use crate::fmt;
156156
use crate::intrinsics;
157+
use crate::mem::{self, ManuallyDrop};
157158

158159
///////////////////////////////////////////////////////////////////////////////
159160
// Any trait
@@ -194,15 +195,93 @@ pub trait Any: 'static {
194195
/// ```
195196
#[stable(feature = "get_type_id", since = "1.34.0")]
196197
fn type_id(&self) -> TypeId;
198+
199+
/// Project a `Box<T>` to a `Box<DropNoUnwindSameAnyTypeId<T>>`.
200+
#[unstable(feature = "dyn_into_drop_no_unwind", issue = "none")]
201+
#[doc(hidden)]
202+
fn __dyn_into_drop_no_unwind(self: *mut Self) -> *mut dyn Any;
197203
}
198204

199205
#[stable(feature = "rust1", since = "1.0.0")]
200206
impl<T: 'static + ?Sized> Any for T {
201207
fn type_id(&self) -> TypeId {
208+
unsafe impl<T: ?Sized + Any> OverrideAnyTypeId for T {
209+
default fn overridden_type_id() -> TypeId {
210+
TypeId::of::<Self>()
211+
}
212+
}
213+
214+
<T as OverrideAnyTypeId>::overridden_type_id()
215+
}
216+
217+
#[doc(hidden)]
218+
fn __dyn_into_drop_no_unwind(self: *mut Self) -> *mut dyn Any {
219+
<T as OverrideDynIntoDropNoUnwind>::dyn_into_drop_no_unwind(self)
220+
}
221+
}
222+
223+
// Safety: overriding the dynamic `type_id` of a type must not be done
224+
// in way where arbitrary code may witness the static and dynamic type mismatch.
225+
unsafe trait OverrideAnyTypeId: Any {
226+
fn overridden_type_id() -> TypeId;
227+
}
228+
229+
#[allow(missing_debug_implementations)]
230+
#[doc(hidden)]
231+
#[repr(transparent)] // repr ≥ C to ensure same layout as `T`
232+
struct DropNoUnwindSameAnyTypeId<T>(ManuallyDrop<T>);
233+
234+
// SAFETY: `DropNoUnwindSameAnyTypeId` is private and only constructed here,
235+
// in a manner which indeed prevents witnessing the static vs. dynamic type mismatch.
236+
unsafe impl<T: 'static> OverrideAnyTypeId for DropNoUnwindSameAnyTypeId<T> {
237+
fn overridden_type_id() -> TypeId {
202238
TypeId::of::<T>()
203239
}
204240
}
205241

242+
impl<T> Drop for DropNoUnwindSameAnyTypeId<T> {
243+
fn drop(&mut self) {
244+
struct AbortOnDrop {}
245+
246+
impl Drop for AbortOnDrop {
247+
fn drop(&mut self) {
248+
crate::intrinsics::abort();
249+
}
250+
}
251+
252+
let abort_on_drop_bomb = AbortOnDrop {};
253+
// SAFETY: textbook ManuallyDrop.
254+
unsafe {
255+
ManuallyDrop::drop(&mut self.0);
256+
}
257+
mem::forget(abort_on_drop_bomb);
258+
}
259+
}
260+
261+
trait OverrideDynIntoDropNoUnwind: Any {
262+
fn dyn_into_drop_no_unwind(ptr: *mut Self) -> *mut dyn Any;
263+
}
264+
/// `: !Sized`.
265+
impl<Dst: ?Sized + 'static> OverrideDynIntoDropNoUnwind for Dst {
266+
default fn dyn_into_drop_no_unwind(_wide_ptr: *mut Dst) -> *mut dyn Any {
267+
unimplemented!("not to be called");
268+
}
269+
}
270+
/// `: Sized & ≠ DropNoUnwindSameAnyTypeId<_>`
271+
impl<T: 'static> OverrideDynIntoDropNoUnwind for T {
272+
default fn dyn_into_drop_no_unwind(thin_ptr: *mut T) -> *mut dyn Any {
273+
// Note: this can only be reached by deliberate misusage of
274+
// `resume_unwind` (not resuming a payload, but a custom dyn any).
275+
<*mut T>::cast::<DropNoUnwindSameAnyTypeId<T>>(thin_ptr) as _
276+
}
277+
}
278+
/// `DropNoUnwindSameAnyTypeId<_>`
279+
impl<T: 'static> OverrideDynIntoDropNoUnwind for DropNoUnwindSameAnyTypeId<T> {
280+
fn dyn_into_drop_no_unwind(thin_ptr: *mut DropNoUnwindSameAnyTypeId<T>) -> *mut dyn Any {
281+
thin_ptr as _
282+
}
283+
}
284+
206285
///////////////////////////////////////////////////////////////////////////////
207286
// Extension methods for Any trait objects.
208287
///////////////////////////////////////////////////////////////////////////////

library/core/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@
170170
#![feature(adt_const_params)]
171171
#![feature(allow_internal_unsafe)]
172172
#![feature(allow_internal_unstable)]
173+
#![feature(arbitrary_self_types)]
173174
#![feature(associated_type_bounds)]
174175
#![feature(auto_traits)]
175176
#![feature(cfg_sanitize)]

library/std/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@
280280
#![feature(cstr_internals)]
281281
#![feature(duration_checked_float)]
282282
#![feature(duration_constants)]
283+
#![feature(dyn_into_drop_no_unwind)]
283284
#![feature(error_generic_member_access)]
284285
#![feature(error_in_core)]
285286
#![feature(error_iter)]

library/std/src/panic.rs

+46
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,52 @@ where
135135
#[stable(feature = "catch_unwind", since = "1.9.0")]
136136
pub fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> {
137137
unsafe { panicking::r#try(f) }
138+
// So, there is a footgun with `catch_unwind()`'s API:
139+
//
140+
// 1. when running "untrusted" / arbitrary code, it may panic and unwind.
141+
// 2. this, in turn, could lead to an impromptu interruption of your own code.
142+
// 3. thence `panic::catch_unwind()`, to act as an unwinding boundary and
143+
// ensure the unwinding can be stopped.
144+
// 4. but the so obtained `Result::<R, Box<dyn Any…>>::Err` variant contains,
145+
// in that case, an arbitrary payload (_e.g._, a boxed `42` in case of
146+
// `panic!(42)`), with arbitrary drop glue, such as a `PanicOnDrop` bomb.
147+
// 5. this means that if the `Err` variant is just dismissed or discarded, when
148+
// it gets dropped, it can still cause an unwind which goes against the
149+
// caller's intent to block them.
150+
//
151+
// See #86027 for more context.
152+
//
153+
// In order to tackle this, the idea behind this `.map_err()`, and the
154+
// `DropNoUnwindSameAnyTypeId` type from `::core`, is to do something similar
155+
// to what was suggested over
156+
// https://github.com/rust-lang/rust/issues/86027#issuecomment-858083087:
157+
// To cheat a little bit and tweak/amend the obtained `Box<dyn Any…>`:
158+
// - keep the `.type_id()` the same to avoid breakage with downcasting;
159+
// - but make it so the virtual `.drop_in_place()` is somehow overridden with
160+
// a shim around the real drop glue that prevents unwinding (_e.g._, by
161+
// aborting when that happens).
162+
//
163+
// This is achieved through the `DropNoUnwindSameAnyTypeId<T>`, wrapper:
164+
// - with the very same layout as the `T` it wraps;
165+
// - with an overridden/fake `.type_id()` so as to impersonate its inner `T`;
166+
// - with a manual `impl Drop` which uses an abort bomb to ensure no
167+
// unwinding can happen.
168+
//
169+
// That way, the `Box<DropNoUnwindSameAnyTypeId<T>>`, when box-erased to a
170+
// `Box<dyn Any…>`, becomes, both layout-wise, and `type_id`-wise,
171+
// undistinguishable from a `Box<T>`, thence avoiding any breakage.
172+
//
173+
// And yet, when that `Box<dyn Any…>` is implicitly dropped with
174+
// `catch_unwind`s, no further unwinding can happen.
175+
.map_err(|any| {
176+
// *Project* the `Box<M>` to a `Box<DropNoUnwindSameAnyTypeId<M>>`.
177+
// Safety: if `M : Send`, then `DropNoUnwindSameAnyTypeId<M> : Send`.
178+
unsafe {
179+
let drop_nounwind: *mut dyn Any = Box::into_raw(any).__dyn_into_drop_no_unwind();
180+
let drop_nounwind_send: *mut (dyn Any + Send) = drop_nounwind as _;
181+
Box::from_raw(drop_nounwind_send)
182+
}
183+
})
138184
}
139185

140186
/// Triggers a panic without invoking the panic hook.

0 commit comments

Comments
 (0)