In net_tx_pkt_build_vheader(), we are insufficiently cautious about headers from the packet from the guest. Add more checking: * that the segment offload payload gso_size isn't 0 * that the packet is not too small * that the TCP th_off is inside the payload * that we aren't going to overflow in our header length calculation
Of these, the most significant extra check is in calculation of the virt_hdr length. We calculate this with pkt->virt_hdr.hdr_len = pkt->hdr_len + l4hdr.th_off * sizeof(uint32_t); The struct field is 16 bits, and so this can overflow if the guest hands us a packet of close to 64K in size and an appropriate l4hdr.th_off. The effect is that we record a bogus virt_hdr.hdr_len, and later on in net_tx_pkt_tcp_fragment_init() this will cause us to make a very large allocation that causes g_malloc() to abort. This is a hardening issue rather than a security issue, because the amount of guest control over the allocation size is such that it can only generate allocations very close to SIZE_MAX which always make g_malloc() abort, and so it's a guest self-denial-of-service only. Cc: [email protected] Fixes: e263cd49c73c595 ("Packet abstraction for VMWARE network devices") Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3648 Signed-off-by: Peter Maydell <[email protected]> --- hw/net/net_tx_pkt.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c index b134348fe80..b2935900fe7 100644 --- a/hw/net/net_tx_pkt.c +++ b/hw/net/net_tx_pkt.c @@ -335,6 +335,11 @@ bool net_tx_pkt_build_vheader(struct NetTxPkt *pkt, bool tso_enable, break; case VIRTIO_NET_HDR_GSO_UDP: + if (IP_FRAG_ALIGN_SIZE(gso_size) == 0 || + pkt->payload_len < sizeof(struct udp_header) || + pkt->hdr_len + sizeof(struct udp_header) > UINT16_MAX) { + return false; + } pkt->virt_hdr.gso_size = gso_size; pkt->virt_hdr.hdr_len = pkt->hdr_len + sizeof(struct udp_header); break; @@ -344,7 +349,10 @@ bool net_tx_pkt_build_vheader(struct NetTxPkt *pkt, bool tso_enable, bytes_read = iov_to_buf(&pkt->vec[NET_TX_PKT_PL_START_FRAG], pkt->payload_frags, 0, &l4hdr, sizeof(l4hdr)); if (bytes_read < sizeof(l4hdr) || - l4hdr.th_off * sizeof(uint32_t) < sizeof(l4hdr)) { + gso_size == 0 || + l4hdr.th_off * sizeof(uint32_t) < sizeof(l4hdr) || + l4hdr.th_off * sizeof(uint32_t) > pkt->payload_len || + pkt->hdr_len + l4hdr.th_off * sizeof(uint32_t) > UINT16_MAX) { return false; } -- 2.43.0
