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

Ahmad Fatoum
2026-04-17T09:46:53Z

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 |