[PATCH v2 0/3] net: dhcp: fix buffer overflows

[PATCH v2 0/3] net: dhcp: fix buffer overflows

[Sascha Hauer]

Fix buffer overflows on malicious incoming network packets or user data
in the dhcp code.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Changes in v2:
- use first character != 0 check instead of strnlen
- Link to v1: https://lore.barebox.org/20260402-net-dhcp-buffer-overflows-v1-0-cd60b651a629@pengutronix.de

---
Sascha Hauer (3):
      net: dhcp: add bounds checking to DHCP option parsing
      net: dhcp: use xstrndup for bp_file to prevent read past field
      net: dhcp: cap DHCP option string length to 255 bytes

 net/dhcp.c | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)
---
base-commit: 0933e8f2ebf0d91dfcf177a4e4292b02921a53f1
change-id: 20260402-net-dhcp-buffer-overflows-94a0e80363c1

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

[PATCH v2 1/3] net: dhcp: add bounds checking to DHCP option parsing

[Sascha Hauer]

dhcp_message_type() walks DHCP options with no end-of-buffer check. A
malicious DHCP server can craft a packet without a 0xff terminator or
with large option length fields, causing reads past the packet buffer
boundary.

Similarly, dhcp_options_process() computes its end bound from fixed
struct sizes rather than the actual received packet length, which could
parse past the real packet data.

Fix both functions by passing the actual packet length and computing
proper bounds. Validate that option type and length bytes are within
bounds before reading them, and that the option data region doesn't
extend past the packet.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 net/dhcp.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index e25b64842d..1cb3fef5d7 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -318,30 +318,39 @@ static void dhcp_options_handle(unsigned char option, void *popt,
 	}
 }
 
-static void dhcp_options_process(unsigned char *popt, struct bootp *bp)
+static void dhcp_options_process(unsigned char *popt, struct bootp *bp,
+				 unsigned int len)
 {
-	unsigned char *end = popt + sizeof(*bp) + OPT_SIZE;
+	unsigned char *end = (unsigned char *)bp + len;
 	int oplen;
 	unsigned char option;
 
-	while (popt < end && *popt != 0xff) {
+	while (popt + 1 < end && *popt != 0xff) {
 		oplen = *(popt + 1);
 		option = *popt;
 
+		if (popt + 2 + oplen > end)
+			break;
+
 		dhcp_options_handle(option, popt + 2, oplen, bp);
 
 		popt += oplen + 2;	/* Process next option */
 	}
 }
 
-static int dhcp_message_type(unsigned char *popt)
+static int dhcp_message_type(unsigned char *popt, unsigned int len)
 {
+	unsigned char *end = popt + len;
+
+	if (len < 4)
+		return -1;
+
 	if (net_read_uint32((uint32_t *)popt) != htonl(BOOTP_VENDOR_MAGIC))
 		return -1;
 
 	popt += 4;
-	while (*popt != 0xff) {
-		if (*popt == 53)	/* DHCP Message Type */
+	while (popt + 1 < end && *popt != 0xff) {
+		if (*popt == 53 && popt + 2 < end)
 			return *(popt + 2);
 		popt += *(popt + 1) + 2;	/* Scan through all options */
 	}
@@ -415,7 +424,7 @@ static void dhcp_handler(void *ctx, char *packet, unsigned int len)
 		dhcp_state = REQUESTING;
 
 		if (net_read_uint32(&bp->bp_vend[0]) == htonl(BOOTP_VENDOR_MAGIC))
-			dhcp_options_process((u8 *)&bp->bp_vend[4], bp);
+			dhcp_options_process((u8 *)&bp->bp_vend[4], bp, len);
 
 		bootp_copy_net_params(bp); /* Store net params from reply */
 
@@ -426,9 +435,10 @@ static void dhcp_handler(void *ctx, char *packet, unsigned int len)
 	case REQUESTING:
 		debug("%s: State REQUESTING\n", __func__);
 
-		if (dhcp_message_type((u8 *)bp->bp_vend) == DHCP_ACK ) {
+		if (dhcp_message_type((u8 *)bp->bp_vend,
+				      len - offsetof(struct bootp, bp_vend)) == DHCP_ACK) {
 			if (net_read_uint32(&bp->bp_vend[0]) == htonl(BOOTP_VENDOR_MAGIC))
-				dhcp_options_process(&bp->bp_vend[4], bp);
+				dhcp_options_process(&bp->bp_vend[4], bp, len);
 			bootp_copy_net_params(bp); /* Store net params from reply */
 			dhcp_state = BOUND;
 			dev_info(&dhcp_edev->dev, "DHCP client bound to address %pI4\n", &dhcp_result->ip);

-- 
2.47.3

[PATCH v2 2/3] net: dhcp: use xstrndup for bp_file to prevent read past field

[Sascha Hauer]

bootp_copy_net_params() uses strlen() and xstrdup() on bp->bp_file,
which is a fixed 128-byte field in the BOOTP packet. If a DHCP server
sends a packet where all 128 bytes are non-null, strlen() reads past
the field boundary into the options data looking for a null terminator,
and xstrdup() then copies the oversized result.

Use a simple bp->bp_file[0] check for the non-empty test and
xstrndup() bounded by sizeof(bp->bp_file) to ensure we never read
past the field.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 net/dhcp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index 1cb3fef5d7..f4e4033351 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -158,8 +158,8 @@ static void bootp_copy_net_params(struct bootp *bp)
 	dhcp_result->ip = net_read_ip(&bp->bp_yiaddr);
 	dhcp_result->serverip = net_read_ip(&bp->bp_siaddr);
 
-	if (strlen(bp->bp_file) > 0)
-		dhcp_result->bootfile = xstrdup(bp->bp_file);
+	if (bp->bp_file[0] != '\0')
+		dhcp_result->bootfile = xstrndup(bp->bp_file, sizeof(bp->bp_file));
 }
 
 static int dhcp_set_string_options(int option, const char *str, u8 *e)

-- 
2.47.3

[PATCH v2 3/3] net: dhcp: cap DHCP option string length to 255 bytes

[Sascha Hauer]

dhcp_set_string_options() stores the string length in a u8 field but
uses the full strlen() value for memcpy(). If a user sets a DHCP
option string (hostname, vendor_id, etc.) longer than 255 bytes, the
length field silently truncates while memcpy() copies the full string,
writing past the expected region in the packet buffer.

Cap str_len at 255 to match the maximum DHCP option length defined by
RFC2132, ensuring the length field and memcpy are consistent.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 net/dhcp.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/dhcp.c b/net/dhcp.c
index f4e4033351..21d48cfcf2 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -173,6 +173,10 @@ static int dhcp_set_string_options(int option, const char *str, u8 *e)
 	if (!str_len)
 		return 0;
 
+	/* DHCP option length field is a single byte per RFC2132 */
+	if (str_len > 255)
+		str_len = 255;
+
 	*e++ = option;
 	*e++ = str_len;
 	memcpy(e, str, str_len);

-- 
2.47.3