nix icon indicating copy to clipboard operation
nix copied to clipboard

Possible Buffer-Overflow when sending control messages with `sendmmsg`

Open Jan561 opened this issue 2 years ago • 2 comments
trafficstars

pub fn sendmmsg<'a, C, S>(
    // elided ...
    data: &'a mut MultiHeaders<S>,
    cmsgs: C,
) 
where
    C: AsRef<[ControlMessage<'a>]>,
{

    for mmsghdr in data.items.iter_mut().enumerate() {
        let p = &mut mmsghdr.msg_hdr;

        let mut pmhdr: *mut cmsghdr = unsafe { CMSG_FIRSTHDR(p) };
        for cmsg in cmsgs.as_ref() {
            assert_ne!(pmhdr, ptr::null_mut());

            // This is unsound: it relies on the caller having initialized the control
            // message buffer inside `data` with enough space for holding all
            // control messages inside `cmsgs`, i.e. having used the `cmsg_space!`
            // macro properly in conjunction with `MultiHeaders::preallocate`. Since
            // the macro, the constructor of `MultiHeaders` and this function are
            // all safe to call, unsafe code must not rely on this invariant to be
            // upheld by the caller.
            //
            // Even the above assertion won't catch this bug, as there might
            // be enough space left to send *a* control message, different from `cmsg`.
            //
            // Original comment:
            //
            // > Safe because we know that pmhdr is valid, and we initialized it with
            // > sufficient space

            unsafe { cmsg.encode_into(pmhdr) };

            pmhdr = unsafe { CMSG_NXTHDR(p, pmhdr) };
        }
    }

    // ...
}

The security impact of this issue is probably negligible, as

  • the control message buffer inside data must have a non-zero length and the majority of the users of this function just want to reduce the amount of syscalls and don't need control messages,
  • an attacker would have to be able to influence which control messages are being sent, which is a very odd and unlikely scenario, or the program must be buggy in first place and initialize the control message buffer too small.

Jan561 avatar Nov 04 '23 17:11 Jan561

sendmmsg always makes me want to quit my programming job and pursue a second career as a swineherd. Before going further, could you please tell me if this bug was introduced by https://github.com/nix-rust/nix/pull/2120 ?

asomers avatar Nov 04 '23 23:11 asomers

No, this bug already existed before

Jan561 avatar Nov 04 '23 23:11 Jan561