Unsoundness in AVFrame::plane

[lwz23]

hello, thank you for your contribution in this project, I am scanning the unsoundness problem in rust project.
I notice the following code:

pub struct AVFrame {
    /// Raw pointer on the AVFrame
    pub ptr: *mut ffmpeg::AVFrame,
}

impl AVFrame {
...........................
   pub fn plane(&mut self, indx: usize, len: usize) -> &mut [u8] {
        unsafe {
            let data_ptr = (*self.ptr).data[indx];
            std::slice::from_raw_parts_mut(data_ptr, len)
        }
    }
}

Considering that pub mod ffmpeg_helper, ptr is a pub field, and plane is also a pub function. I assume that users can directly manipulate this field. This potential situation could result in *self.ptr being dereference a null pointer, and directly dereferencing it might trigger undefined behavior (UB). For safety reasons, I felt it necessary to report this issue. If you have performed checks elsewhere that ensure this is safe, please don’t take offense at my raising this issue.

[lwz23]

ping

[serpilliere]

Hi @lwz23
First off all, thank you for the issue! (and sorry for the delay)
We have looked at the code here are the results:

• The avframe is instanciated as non mutable (

  sanzu/sanzu/src/video_encoder.rs

  Line 107 in 09a874f

  let frame = AVFrame::new()?;

  )
• But will be embedded as sub struct in EncoderFFmpeg
• the EncoderFFmpeg doesn't change the ptr value itself.

Anyway, as we do not need the pub attribute, we have made a patch to do in consequence. Only the api get_ptr returns the ptr to the mutable object.

https://github.com/cea-sec/sanzu/tree/updt_avframe_ptr

Do you agree with this patch or do you think we have missed something?

[lwz23]

Yes, thanks for your reply! I think that's enough.