[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

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

[Ahmad Fatoum]

On 4/2/26 8:59 AM, Sascha Hauer wrote:
> 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>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  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);
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

[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

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

[Ahmad Fatoum]

On 4/2/26 8:59 AM, Sascha Hauer wrote:
> 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>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  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)
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

[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

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

[Ahmad Fatoum]

On 4/2/26 8:59 AM, Sascha Hauer wrote:
> 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>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  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);
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

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

[Sascha Hauer]

On Thu, 02 Apr 2026 08:59:56 +0200, Sascha Hauer wrote:
> Fix buffer overflows on malicious incoming network packets or user data
> in the dhcp code.
> 
> 

Applied, thanks!

[1/3] net: dhcp: add bounds checking to DHCP option parsing
      https://git.pengutronix.de/cgit/barebox/commit/?id=6199b241964b (link may not be stable)
[2/3] net: dhcp: use xstrndup for bp_file to prevent read past field
      https://git.pengutronix.de/cgit/barebox/commit/?id=77929b00dc11 (link may not be stable)
[3/3] net: dhcp: cap DHCP option string length to 255 bytes
      https://git.pengutronix.de/cgit/barebox/commit/?id=2626d503bdd6 (link may not be stable)

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