Metatables can trigger segfaults in safe code

[gretchenfrage]

I was reading through this code out of interest, and I saw that the metatable can be used to trigger undefined behavior without using any unsafe code. Basically, if an implementation of CastFrom borrows out an object reference to sub-data within the itself, instead of its entire self, it will incorrectly cast raw pointers.

This program trigger a seg fault by dereferencing null:

extern crate shred;

use shred::{CastFrom, MetaTable};

pub trait PointsToU64 {
    fn get_u64(&self) -> u64;
}

impl PointsToU64 for Box<u64> {
    fn get_u64(&self) -> u64 {
        *(&**self)
    }
}

struct MultipleData {
    _number: u64,
    pointer: Box<u64>,
}

impl CastFrom<MultipleData> for PointsToU64 {
    fn cast(t: &MultipleData) -> &Self {
        &t.pointer
    }

    fn cast_mut(t: &mut MultipleData) -> &mut Self {
        &mut t.pointer
    }
}

fn main() {
    let mut table: MetaTable<PointsToU64> = MetaTable::new();
    let md = MultipleData {
        _number: 0x0, // this will be casted to a pointer, then dereferenced
        pointer: Box::new(42),
    };
    table.register(&md);
    if let Some(t) = table.get(&md) {
        println!("{}", t.get_u64());
    }
}

I'm not sure if you're aware of this, but it seems noteworthy to me. One solution I could see would be to make CastFrom an unsafe trait, and create a macro for correctly implementing it on a type. Another approach that might work would be to use the Unsize<T> API, but i'm not sure how stable that is. Another approach would be to modify the metatable to actually invoke the implementation of CastFrom, which seems like the less disruptive solution, but it would add a slight additional runtime cost.

[torkleyy]

Oh, thanks for the report! I'm thinking we should be conservative and make two changes:

1) make it an unsafe trait 2) assert the trait object pointer and the reference share the same address

The macro is probably not necessary, since the trait is meant to be implemented using a blanket impl.

I don't remember the details of Unsized, but if it's stable and works it should probably replace CastFrom entirely.