Skip to content

Commit

Permalink
Soundness fix: guard against silly Pin-misusage (shadowing and inte…
Browse files Browse the repository at this point in the history
…rmixing)
  • Loading branch information
danielhenrymantilla committed Mar 16, 2023
1 parent 661496f commit c0b2292
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 26 deletions.
1 change: 1 addition & 0 deletions src/_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@ mod __ {
::core::{
self,
marker::PhantomData,
pin::Pin,
primitive::{
u8, u16, u32, usize, u64, u128,
i8, i16, i32, isize, i64, i128,
Expand Down
47 changes: 36 additions & 11 deletions src/proc_macro/derives/dyn_trait/_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,20 +205,45 @@ fn try_handle_trait (
if mut_ == mutability
))
};
let has_pin =
each_vtable_entry
let (mut has_pin, mut has_non_pin_non_ref) = (None, None);
each_vtable_entry
.iter()
.any(|it| matches!(
*it,
VTableEntry::VirtualMethod {
receiver: ReceiverType {
pinned: true,
..
},
.filter_map(|it| match *it {
| VTableEntry::VirtualMethod {
ref receiver,
ref src,
..
}
))
} => {
Some((receiver, || src.sig.ident.clone()))
},
})
.for_each(|(receiver, fname)| match receiver {
| ReceiverType {
pinned: true,
..
} => {
has_pin.get_or_insert_with(fname);
},
// Tolerate `&self` refs mixed with `Pin`s:
| ReceiverType {
kind: ReceiverKind::Reference { mut_: false },
..
} => {
/* OK */
},
| _otherwise => {
has_non_pin_non_ref.get_or_insert_with(fname);
},
})
;
match (&has_pin, has_non_pin_non_ref) {
| (Some(pinned_span), Some(non_pin_non_ref_span)) => bail! {
"`Pin<>` receivers can only be mixed with `&self` receivers"
=> quote!(#pinned_span #non_pin_non_ref_span)
},
| _ => {},
}
let has_pin = has_pin.is_some();
let (has_ref, has_mut) = (has_mutability(false), has_mutability(true));

let send_trait = &squote!( #ඞ::marker::Send );
Expand Down
30 changes: 20 additions & 10 deletions src/proc_macro/derives/dyn_trait/receiver_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ struct ReceiverType {
impl ReceiverType {
pub(crate)
fn from_fn_arg(
fn_arg: &'_ FnArg,
fn_arg: &'_ mut FnArg,
) -> Result<ReceiverType>
{
let pinned = false;
let mut storage = None;
Self::from_type_of_self(
match fn_arg {
| &FnArg::Receiver(Receiver {
| &mut FnArg::Receiver(Receiver {
attrs: _,
reference: ref ref_,
mutability: ref mut_,
Expand Down Expand Up @@ -64,7 +64,7 @@ impl ReceiverType {
}

fn from_type_of_self<'i>(
type_of_self: &'i Type,
type_of_self: &'i mut Type,
pinned: bool,
) -> Result<ReceiverType>
{
Expand Down Expand Up @@ -110,26 +110,26 @@ impl ReceiverType {
// `: path::to::SomeWrapper<…>`
| Type::Path(TypePath {
qself: None,
path: ref ty_path,
path: ref mut ty_path,
}) => {
use AngleBracketedGenericArguments as Generic;
let extract_generic_ty = |args: &'i syn::PathArguments| -> Option<&Type> {
fn extract_generic_ty(args: &'_ mut syn::PathArguments) -> Option<&mut Type> {
match args {
| PathArguments::AngleBracketed(AngleBracketedGenericArguments {
args, ..
})
if args.len() == 1
=> match args[0] {
| GenericArgument::Type(ref inner) => Some(inner),
| GenericArgument::Type(ref mut inner) => Some(inner),
| _ => None,
},
| _ => None,
}
};
}

// `SomeWrapper<inner>`
let last = ty_path.segments.last().unwrap();
match (&last.ident.to_string()[..], extract_generic_ty(&last.arguments)) {
let last = ty_path.segments.last_mut().unwrap();
let ret = match (&last.ident.to_string()[..], extract_generic_ty(&mut last.arguments)) {
// `Box<Self>`
| ("Box", Some(inner)) if is_Self(inner) => Self {
pinned,
Expand Down Expand Up @@ -157,7 +157,17 @@ impl ReceiverType {
(more complex `Self` types are not supported)\
" => last,
},
}
};
// Replace any encountered `Box`,`Arc`,`Pin`, with *our* fully qualified to the
// expected item, to guard against silly shadowings.
ty_path.leading_colon = Some(token::Colon2 { spans: [last.span(), last.span() ]});
ty_path.segments =
Punctuated::parse_separated_nonempty.parse2(quote_spanned!(last.span()=>
safer_ffi::::#last
))
.unwrap()
;
ret
},

// `([<Something as Complex>::Assoc; 3], bool)`
Expand Down
11 changes: 6 additions & 5 deletions src/proc_macro/derives/dyn_trait/vtable_entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use super::*;
pub(in super)
enum VTableEntry<'trait_> {
VirtualMethod {
src: &'trait_ TraitItemMethod,
src: TraitItemMethod,
name: &'trait_ Ident,
each_for_lifetime: Vec<&'trait_ Lifetime>,
receiver: ReceiverType,
Expand Down Expand Up @@ -42,7 +42,7 @@ impl<'trait_> VTableEntry<'trait_> {
ErasedSelf: _,
EachArgTy: _,
OutputTy: _,
src: &TraitItemMethod {
src: TraitItemMethod {
sig: ref full_signature,
ref attrs,
..
Expand Down Expand Up @@ -86,11 +86,11 @@ impl<'trait_> VTableEntry<'trait_> {
pub(in super)
fn attrs<'r> (
self: &'r VTableEntry<'trait_>
) -> &'trait_ Vec<Attribute>
) -> &'r Vec<Attribute>
{
match self {
| Self::VirtualMethod {
src: &TraitItemMethod {
src: TraitItemMethod {
ref attrs,
..
},
Expand Down Expand Up @@ -254,6 +254,7 @@ fn vtable_entries<'trait_> (
default: _,
semi_token: _,
}) => {
let mut trait_item_method = trait_item_method.clone();
// // Is there a `Self : Sized` opt-out-of-`dyn` clause?
// if matches!(
// generics.where_clause, Some(ref where_clause)
Expand Down Expand Up @@ -300,7 +301,7 @@ fn vtable_entries<'trait_> (
// )
// })
// };
let receiver = if let Some(fn_arg) = sig.receiver() {
let receiver = if let Some(fn_arg) = trait_item_method.sig.inputs.iter_mut().next() {
match ReceiverType::from_fn_arg(fn_arg) {
| Ok(it) => it,
| Err(err) => return Some(Err(err)),
Expand Down

0 comments on commit c0b2292

Please sign in to comment.