Re: [PATCH v1] net: bootp: bound DHCP option parsing by the received packet length

Hi Pranav,

On 16/08/2026 00:08, Pranav Rajendran wrote:
> dhcp_packet_process_options() derives the end of the option area from
> BOOTP_HDR_SIZE, a compile-time constant, rather than from the length of
> the packet that was actually received:
> 
> 	uchar *popt = (uchar *)&bp->bp_vend[4];
> 	uchar *end = popt + BOOTP_HDR_SIZE;
> 
> Since popt already starts near the end of the header, 'end' lands
> sizeof(struct bootp_hdr) bytes beyond it, so a short reply leaves
> dhcp_process_options() walking off the end of the received data and
> into whatever the receive buffer held before - typically the remains of
> earlier packets.
> 
> That is not only a disclosure: the options found there are acted on
> like any others, so stale bytes that happen to parse as an option can
> influence the boot file name, the DNS server or the root path.
> 
> The BOOTP path already gets this right and passes the real length to
> bootp_process_vendor(), and both callers here have the length in scope
> - they hand it to dhcp_message_type() on the lines above. Pass it in
> and use it as the limit. The overloaded 'file' and 'sname' areas are
> clamped the same way, as a truncated packet need not contain them
> either.
> 
> Fixes: 774c3e05ec0a ("net: parse DHCP options from overloaded file/sname fields")
> Signed-off-by: Pranav Rajendran <[email protected]>
> ---
>  net/bootp.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/net/bootp.c b/net/bootp.c
> index f0dc329d6e4..fdedecb3f50 100644
> --- a/net/bootp.c
> +++ b/net/bootp.c
> @@ -968,32 +968,43 @@ static void dhcp_process_options(uchar *popt, uchar *end)
>  	}
>  }
>  
> -static void dhcp_packet_process_options(struct bootp_hdr *bp)
> +static void dhcp_packet_process_options(struct bootp_hdr *bp, unsigned int len)
>  {
> -	uchar *popt = (uchar *)&bp->bp_vend[4];
> -	uchar *end = popt + BOOTP_HDR_SIZE;
> +	uchar *pkt_end = (uchar *)bp + len;
> +	uchar *popt, *end;
> +
> +	if (len < offsetof(struct bootp_hdr, bp_vend) + 4)
> +		return;
>  
>  	if (net_read_u32((u32 *)&bp->bp_vend[0]) != htonl(BOOTP_VENDOR_MAGIC))
>  		return;
>  
> +	popt = (uchar *)&bp->bp_vend[4];
> +
>  	dhcp_option_overload = 0;
>  
>  	/*
>  	 * The 'options' field MUST be interpreted first, 'file' next,
>  	 * 'sname' last.
>  	 */
> -	dhcp_process_options(popt, end);
> +	dhcp_process_options(popt, pkt_end);

This fixes the outer bound, but I don't think it is sufficient to make the parser
safe for truncated packets.

dhcp_process_options() currently starts with:

	while (popt < end && *popt != 0xff) {
		oplen = *(popt + 1);

If the received packet ends with a single option code byte, popt < end is true but
popt + 1 is already out of bounds.

There is also no validation that the complete option payload is present before the
switch processes it.

So I think a prerequisite patch is needed to fix dhcp_process_options() first.

Thanks,
-- 
Jerome

>  
>  	if (dhcp_option_overload & OVERLOAD_FILE) {
>  		popt = (uchar *)bp->bp_file;
>  		end = popt + sizeof(bp->bp_file);
> -		dhcp_process_options(popt, end);
> +		if (end > pkt_end)
> +			end = pkt_end;
> +		if (popt < end)
> +			dhcp_process_options(popt, end);
>  	}
>  
>  	if (dhcp_option_overload & OVERLOAD_SNAME) {
>  		popt = (uchar *)bp->bp_sname;
>  		end = popt + sizeof(bp->bp_sname);
> -		dhcp_process_options(popt, end);
> +		if (end > pkt_end)
> +			end = pkt_end;
> +		if (popt < end)
> +			dhcp_process_options(popt, end);
>  	}
>  }
>  
> @@ -1124,7 +1135,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>  				debug("got BOOTP response; transitioning to BOUND\n");
>  				goto dhcp_got_bootp;
>  			}
> -			dhcp_packet_process_options(bp);
> +			dhcp_packet_process_options(bp, len);
>  			if (CONFIG_IS_ENABLED(EFI_LOADER) &&
>  			    IS_ENABLED(CONFIG_NETDEVICES))
>  				efi_net_set_dhcp_ack(pkt, len);
> @@ -1151,7 +1162,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
>  
>  		if (dhcp_message_type((u8 *)bp->bp_vend, (u8 *)pkt + len) == DHCP_ACK) {
>  dhcp_got_bootp:
> -			dhcp_packet_process_options(bp);
> +			dhcp_packet_process_options(bp, len);
>  			/* Store net params from reply */
>  			store_net_params(bp);
>  			dhcp_state = BOUND;