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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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 |