untrusted comment: verify with openbsd-68-base.pub
RWQZj25CSG5R2l61cdgQ56038WHHKquXI+WJ7kBwDmm0mlSaqwSxaS7LkpOJJg2PAuLaVOFtLIMOGfVgZVKzjZkfrLIMawrmCAc=
OpenBSD 6.8 errata 025, June 9, 2021:
vmd guests could cause stack overflows by crafting malicious dhcp
requests when using local interfaces.
Apply by doing:
signify -Vep /etc/signify/openbsd-68-base.pub -x 025_vmd.patch.sig \
-m - | (cd /usr/src && patch -p0)
And then rebuild and install vmd(8):
cd /usr/src/usr.sbin/vmd
make obj
make
make install
Index: usr.sbin/vmd/dhcp.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/dhcp.c,v
diff -u -p -r1.8 dhcp.c
--- usr.sbin/vmd/dhcp.c 27 Dec 2018 19:51:30 -0000 1.8
+++ usr.sbin/vmd/dhcp.c 6 Jun 2021 17:04:22 -0000
@@ -21,6 +21,8 @@
#include <net/if.h>
#include <netinet/in.h>
+#include <netinet/ip.h>
+#include <netinet/udp.h>
#include <netinet/if_ether.h>
#include <arpa/inet.h>
@@ -34,6 +36,10 @@
#include "dhcp.h"
#include "virtio.h"
+#define OPTIONS_OFFSET offsetof(struct dhcp_packet, options)
+#define OPTIONS_MAX_LEN \
+ (1500 - sizeof(struct ip) - sizeof(struct udphdr) - OPTIONS_OFFSET)
+
static const uint8_t broadcast[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
extern struct vmd *env;
@@ -41,7 +47,8 @@ ssize_t
dhcp_request(struct vionet_dev *dev, char *buf, size_t buflen, char **obuf)
{
unsigned char *respbuf = NULL, *op, *oe, dhcptype = 0;
- ssize_t offset, respbuflen = 0;
+ unsigned char *opts = NULL;
+ ssize_t offset, optslen, respbuflen = 0;
struct packet_ctx pc;
struct dhcp_packet req, resp;
struct in_addr server_addr, mask, client_addr, requested_addr;
@@ -50,7 +57,8 @@ dhcp_request(struct vionet_dev *dev, cha
struct vmd_vm *vm;
const char *hostname = NULL;
- if (buflen < (ssize_t)(BOOTP_MIN_LEN + sizeof(struct ether_header)))
+ if (buflen < BOOTP_MIN_LEN + ETHER_HDR_LEN ||
+ buflen > 1500 + ETHER_HDR_LEN)
return (-1);
memset(&pc, 0, sizeof(pc));
@@ -68,8 +76,11 @@ dhcp_request(struct vionet_dev *dev, cha
ntohs(ss2sin(&pc.pc_dst)->sin_port) != SERVER_PORT)
return (-1);
+ /* Only populate the base DHCP fields. Options are parsed separately. */
+ if ((size_t)offset + OPTIONS_OFFSET > buflen)
+ return (-1);
memset(&req, 0, sizeof(req));
- memcpy(&req, buf + offset, buflen - offset);
+ memcpy(&req, buf + offset, OPTIONS_OFFSET);
if (req.op != BOOTREQUEST ||
req.htype != pc.pc_htype ||
@@ -81,27 +92,37 @@ dhcp_request(struct vionet_dev *dev, cha
if (req.ciaddr.s_addr != 0 || req.file[0] != '\0' || req.hops != 0)
return (-1);
- /* Get a few DHCP options (best effort as we fall back to BOOTP) */
- if (memcmp(&req.options,
- DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN) == 0) {
- memset(&requested_addr, 0, sizeof(requested_addr));
- op = req.options + DHCP_OPTIONS_COOKIE_LEN;
- oe = req.options + sizeof(req.options);
- while (*op != DHO_END && op < oe) {
- if (op[0] == DHO_PAD) {
- op++;
- continue;
+ /*
+ * If packet has data that could be DHCP options, check for the cookie
+ * and then see if the region is still long enough to contain at least
+ * one variable length option (3 bytes). If not, fallback to BOOTP.
+ */
+ optslen = buflen - offset - OPTIONS_OFFSET;
+ if (optslen > DHCP_OPTIONS_COOKIE_LEN + 3 &&
+ optslen < (ssize_t)OPTIONS_MAX_LEN) {
+ opts = buf + offset + OPTIONS_OFFSET;
+
+ if (memcmp(opts, DHCP_OPTIONS_COOKIE,
+ DHCP_OPTIONS_COOKIE_LEN) == 0) {
+ memset(&requested_addr, 0, sizeof(requested_addr));
+ op = opts + DHCP_OPTIONS_COOKIE_LEN;
+ oe = opts + optslen;
+ while (*op != DHO_END && op + 1 < oe) {
+ if (op[0] == DHO_PAD) {
+ op++;
+ continue;
+ }
+ if (op + 2 + op[1] > oe)
+ break;
+ if (op[0] == DHO_DHCP_MESSAGE_TYPE &&
+ op[1] == 1)
+ dhcptype = op[2];
+ else if (op[0] == DHO_DHCP_REQUESTED_ADDRESS &&
+ op[1] == sizeof(requested_addr))
+ memcpy(&requested_addr, &op[2],
+ sizeof(requested_addr));
+ op += 2 + op[1];
}
- if (op + 1 + op[1] >= oe)
- break;
- if (op[0] == DHO_DHCP_MESSAGE_TYPE &&
- op[1] == 1)
- dhcptype = op[2];
- else if (op[0] == DHO_DHCP_REQUESTED_ADDRESS &&
- op[1] == sizeof(requested_addr))
- memcpy(&requested_addr, &op[2],
- sizeof(requested_addr));
- op += 2 + op[1];
}
}
@@ -140,8 +161,7 @@ dhcp_request(struct vionet_dev *dev, cha
if (*obuf != NULL)
goto fail;
- buflen = 0;
- respbuflen = DHCP_MTU_MAX;
+ respbuflen = sizeof(resp);
if ((respbuf = calloc(1, respbuflen)) == NULL)
goto fail;
@@ -155,14 +175,9 @@ dhcp_request(struct vionet_dev *dev, cha
goto fail;
}
- /* BOOTP uses a 64byte vendor field instead of the DHCP options */
- resplen = BOOTP_MIN_LEN;
-
/* Add BOOTP Vendor Extensions (DHCP options) */
- o = 0;
- memcpy(&resp.options,
- DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN);
- o+= DHCP_OPTIONS_COOKIE_LEN;
+ memcpy(&resp.options, DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN);
+ o = DHCP_OPTIONS_COOKIE_LEN;
/* Did we receive a DHCP request or was it just BOOTP? */
if (dhcptype) {
@@ -213,8 +228,13 @@ dhcp_request(struct vionet_dev *dev, cha
memcpy(&resp.options[o], &server_addr, sizeof(server_addr));
o += sizeof(server_addr);
- if (hostname != NULL) {
- len = strlen(hostname);
+ if (hostname != NULL && (len = strlen(hostname)) > 1) {
+ /* Check if there's still room for the option type and len (2),
+ * hostname, and a final to-be-added DHO_END (1). */
+ if (o + 2 + len + 1 > sizeof(resp.options)) {
+ log_debug("%s: hostname too long", __func__);
+ goto fail;
+ }
resp.options[o++] = DHO_HOST_NAME;
resp.options[o++] = len;
memcpy(&resp.options[o], hostname, len);
@@ -223,7 +243,7 @@ dhcp_request(struct vionet_dev *dev, cha
resp.options[o++] = DHO_END;
- resplen = offsetof(struct dhcp_packet, options) + o;
+ resplen = OPTIONS_OFFSET + o;
/* Minimum packet size */
if (resplen < BOOTP_MIN_LEN)
@@ -242,5 +262,5 @@ dhcp_request(struct vionet_dev *dev, cha
return (respbuflen);
fail:
free(respbuf);
- return (0);
+ return (-1);
}
