Possible Denial of Service: panic if guest sends packet with len < virtio net header length

[nirs]

The NetWorker thread in the host can panic if a guest sends a virtio-net packet with a total descriptor length less than the virtio-net header size (VNET_HDR_LEN, which is 12 bytes). This happens because the write_frame method (e.g., in the Unixgram backend) attempts to slice the buffer using VNET_HDR_LEN as the start index (&buf[hdr_len..]). If the buffer's length (derived from the guest's descriptor chain) is smaller than hdr_len, Rust will panic. A malicious guest can exploit this to crash the networking worker thread of the host, leading to a Denial of Service.

            self.tx_frame_len = read_count;
            if read_count < VNET_HDR_LEN {
                tx_queue
                    .add_used(&self.mem, head_index, 0)
                    .map_err(TxError::QueueError)?;
                raise_irq = true;
                continue;
            }
            match self
                .backend
                .write_frame(VNET_HDR_LEN, &mut self.tx_frame_buf[..read_count])

Originally posted by @gemini-code-assist[bot] in #574 (comment)

[mtjhrc]

This cannot fail from userspace (and if it can it's a bug in the Linux kernel), only way this crashes is if the guest kernel driver doesn't follow the virtio-spec. A bad kernel driver crashing the hardware it's running on isn't really a denial of service - if you are an attacker and have access to the guest kernel, you can denial of service everything in the guest anyway already (e.g you can literally just halt 😃).

Still might be nice to log a descriptive error and skip the malformed packet.

[nirs]

If we validate packet length, we can drop invalid ethernet frames len < 60.

[slp]

I agree with @mtjhrc here, as long as the guest can't escape (beyond the considerations already described in https://github.com/containers/libkrun?tab=readme-ov-file#security-model) or take control of the VMM, I wouldn't consider it a security issue. The guest's kernel can always trigger an exit of the VMM by issuing a reboot.

That said, it'd be nice to validate the read and fail orderly. We already have plans to check for this kind of issues over the next Qs, so this can be part of it.

[mtjhrc]

here, as long as the guest can't escape (beyond the considerations already described in https://github.com/containers/libkrun?tab=readme-ov-file#security-model) or take control of the VMM, I wouldn't consider it a security issue

I think denial-of-service is a valid security issue, but it would have to be something done from user-space (or allowed within virtio spec) to crash the device. I would also consider something like guest issuing a lot of commands to some device very quickly and libkrun consuming a growing amount of memory without a cap (or fully crashing when we hit a cap) to be a denial-of-service too (I mean we must have some backpressure).

[slp]

here, as long as the guest can't escape (beyond the considerations already described in https://github.com/containers/libkrun?tab=readme-ov-file#security-model) or take control of the VMM, I wouldn't consider it a security issue

I think denial-of-service is a valid security issue, but it would have to be something done from user-space (or allowed within virtio spec) to crash the device. I would also consider something like guest issuing a lot of commands to some device very quickly and libkrun consuming a growing amount of memory without a cap (or fully crashing when we hit a cap) to be a denial-of-service too (I mean we must have some backpressure).

If it can be triggered from user-space, I agree. This is also something we might need to review in 2.x, which will officially become a generic VMM, in the sense of being even stricter and protecting against the guest's kernel.