From 8198ae7c21a4d37f7e365058f973867c41d44d21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 29 Jul 2021 04:56:25 -0400 Subject: [PATCH 06/14] bootp: limit vendor-specific area to input packet memory buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RH-Author: Marc-André Lureau Message-id: <20210708082537.1550263-3-marcandre.lureau@redhat.com> Patchwork-id: 101821 O-Subject: [RHEL-8.5.0 qemu-kvm PATCH 2/8] bootp: limit vendor-specific area to input packet memory buffer Bugzilla: 1970819 1970835 1970843 1970853 RH-Acked-by: Stefano Garzarella RH-Acked-by: Eric Blake RH-Acked-by: Stefan Hajnoczi From: Marc-André Lureau sizeof(bootp_t) currently holds DHCP_OPT_LEN. Remove this optional field from the structure, to help with the following patch checking for minimal header size. Modify the bootp_reply() function to take the buffer boundaries and avoiding potential buffer overflow. Related to CVE-2021-3592. https://gitlab.freedesktop.org/slirp/libslirp/-/issues/44 Signed-off-by: Marc-André Lureau (cherry picked from commit f13cad45b25d92760bb0ad67bec0300a4d7d5275) Signed-off-by: Marc-André Lureau Signed-off-by: Miroslav Rezanina --- slirp/src/bootp.c | 26 +++++++++++++++----------- slirp/src/bootp.h | 2 +- slirp/src/mbuf.c | 5 +++++ slirp/src/mbuf.h | 1 + 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/slirp/src/bootp.c b/slirp/src/bootp.c index 3f9ce2553e..5754327138 100644 --- a/slirp/src/bootp.c +++ b/slirp/src/bootp.c @@ -92,21 +92,22 @@ found: return bc; } -static void dhcp_decode(const struct bootp_t *bp, int *pmsg_type, +static void dhcp_decode(const struct bootp_t *bp, + const uint8_t *bp_end, + int *pmsg_type, struct in_addr *preq_addr) { - const uint8_t *p, *p_end; + const uint8_t *p; int len, tag; *pmsg_type = 0; preq_addr->s_addr = htonl(0L); p = bp->bp_vend; - p_end = p + DHCP_OPT_LEN; if (memcmp(p, rfc1533_cookie, 4) != 0) return; p += 4; - while (p < p_end) { + while (p < bp_end) { tag = p[0]; if (tag == RFC1533_PAD) { p++; @@ -114,10 +115,10 @@ static void dhcp_decode(const struct bootp_t *bp, int *pmsg_type, break; } else { p++; - if (p >= p_end) + if (p >= bp_end) break; len = *p++; - if (p + len > p_end) { + if (p + len > bp_end) { break; } DPRINTF("dhcp: tag=%d len=%d\n", tag, len); @@ -144,7 +145,9 @@ static void dhcp_decode(const struct bootp_t *bp, int *pmsg_type, } } -static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) +static void bootp_reply(Slirp *slirp, + const struct bootp_t *bp, + const uint8_t *bp_end) { BOOTPClient *bc = NULL; struct mbuf *m; @@ -157,7 +160,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) uint8_t client_ethaddr[ETH_ALEN]; /* extract exact DHCP msg type */ - dhcp_decode(bp, &dhcp_msg_type, &preq_addr); + dhcp_decode(bp, bp_end, &dhcp_msg_type, &preq_addr); DPRINTF("bootp packet op=%d msgtype=%d", bp->bp_op, dhcp_msg_type); if (preq_addr.s_addr != htonl(0L)) DPRINTF(" req_addr=%08" PRIx32 "\n", ntohl(preq_addr.s_addr)); @@ -179,9 +182,10 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) return; } m->m_data += IF_MAXLINKHDR; + m_inc(m, sizeof(struct bootp_t) + DHCP_OPT_LEN); rbp = (struct bootp_t *)m->m_data; m->m_data += sizeof(struct udpiphdr); - memset(rbp, 0, sizeof(struct bootp_t)); + memset(rbp, 0, sizeof(struct bootp_t) + DHCP_OPT_LEN); if (dhcp_msg_type == DHCPDISCOVER) { if (preq_addr.s_addr != htonl(0L)) { @@ -235,7 +239,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) rbp->bp_siaddr = saddr.sin_addr; /* Server IP address */ q = rbp->bp_vend; - end = (uint8_t *)&rbp[1]; + end = rbp->bp_vend + DHCP_OPT_LEN; memcpy(q, rfc1533_cookie, 4); q += 4; @@ -365,6 +369,6 @@ void bootp_input(struct mbuf *m) struct bootp_t *bp = mtod(m, struct bootp_t *); if (bp->bp_op == BOOTP_REQUEST) { - bootp_reply(m->slirp, bp); + bootp_reply(m->slirp, bp, m_end(m)); } } diff --git a/slirp/src/bootp.h b/slirp/src/bootp.h index 03ece9bf28..0d20a944a8 100644 --- a/slirp/src/bootp.h +++ b/slirp/src/bootp.h @@ -114,7 +114,7 @@ struct bootp_t { uint8_t bp_hwaddr[16]; uint8_t bp_sname[64]; uint8_t bp_file[128]; - uint8_t bp_vend[DHCP_OPT_LEN]; + uint8_t bp_vend[]; }; typedef struct { diff --git a/slirp/src/mbuf.c b/slirp/src/mbuf.c index 6d0653ed3d..7db07c088e 100644 --- a/slirp/src/mbuf.c +++ b/slirp/src/mbuf.c @@ -233,3 +233,8 @@ void *mtod_check(struct mbuf *m, size_t len) return NULL; } + +void *m_end(struct mbuf *m) +{ + return m->m_data + m->m_len; +} diff --git a/slirp/src/mbuf.h b/slirp/src/mbuf.h index 2015e3232f..a9752a36e0 100644 --- a/slirp/src/mbuf.h +++ b/slirp/src/mbuf.h @@ -119,6 +119,7 @@ void m_adj(struct mbuf *, int); int m_copy(struct mbuf *, struct mbuf *, int, int); struct mbuf *dtom(Slirp *, void *); void *mtod_check(struct mbuf *, size_t len); +void *m_end(struct mbuf *); static inline void ifs_init(struct mbuf *ifm) { -- 2.27.0