martinthomson / ohttp

Rust library for encapsulating HTTP messages in a cryptographic wrapper
Apache License 2.0
22 stars 16 forks source link

audit use of `Pin` #27

Closed mozkeeler closed 1 year ago

mozkeeler commented 1 year ago

Consider ParamItem:

pub(crate) struct ParamItem<T> {
    reference: Pin<Box<SECItem>>,
    _params: Vec<u8>,
    marker: PhantomData<T>,
}

impl<T: Sized> ParamItem<T> {
    pub fn new(v: &T) -> Self {
        let slc = unsafe {
            std::slice::from_raw_parts((v as *const T).cast::<u8>(), mem::size_of::<T>())
        };
        let mut params = Vec::from(slc);
        let reference = Box::pin(SECItem {
            type_: SECItemType::siBuffer,
            data: params.as_mut_ptr().cast(),
            len: c_uint::try_from(params.len()).unwrap(),
        });
        Self {
            reference,
            _params: params,
            marker: PhantomData::default(),
        }
    }

    pub fn ptr(&mut self) -> *mut SECItem {
        Pin::into_inner(self.reference.as_mut()) as *mut SECItem
    }
}

It's entirely possible I'm misunderstanding Pin, but I don't think the correct field is being pinned. Pinning the SECItem means it (the SECItem) can't be moved around in memory, but params can still be moved, because it isn't pinned. This means that the data field of the SECItem may point to invalid memory.

martinthomson commented 1 year ago

Oh yeah. I think that this is right. The SECItem can move without an issue, but the self-reference inside the SECItem depends on the memory referenced by the Vec not moving (this is not the Vec itself even).