Re: [PATCH 2/4] dhcp: Make option parsing more robust, explicitly handle options 0 and 255

On Thu, Jul 16, 2026 at 01:25:21AM +0200, Stefano Brivio wrote:

The initial option-scanning loop in dhcp(), so far, ignored options 0 (Pad Option, RFC 2132, Section 3.1) and 255 (End Option, RFC 2132, Section 3.2).

As a result:

- if we ever encountered option 0 in the middle of option fields (never seen in practice), we would potentially terminate the loop too early, before scanning remaining options

- a malformed message with an option 255 followed by a length byte would (reliably) cause us to terminate as we would exceed the allocated size for the 'opts' array, which is detected as buffer overflow by the FORTIFY_SOURCE mechanism

The latter was reported as potential vulnerability by AISLE, but it's not actually a vulnerability as we always terminate without carrying on further handling, and in our security model the guest is able to sabotage its own connectivity in any case (for example, a malformed frame from the hypervisor would cause us to reset the connection, or entirely flooding the flow table would cause inbound connectivity to stop working, etc.).

The reported behaviour, however, is indeed a defect, as it affects the functional robustness to a hypothetical issue in a DHCP client, and that's something we definitely want to fix.

Make the option parsing loop more robust by:

- resizing 'opts' from 255 to 256 elements: there's no particular reason to try to save a tiny bit of memory (which shouldn't even be allocated in practice) instead of being defensive about it

- explicitly handle options 0 (skip one byte, continue) and 255 (stop processing options) in the option-scanning loop

This bug was found and an initial version of the patch was written by the AISLE AI security scanning tool (https://aisle.com/platform).

Reported-by: AISLE Signed-off-by: Stefano Brivio --- dhcp.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/dhcp.c b/dhcp.c index 1ff8cba..632019a 100644 --- a/dhcp.c +++ b/dhcp.c @@ -49,7 +49,7 @@ struct opt { uint8_t c[255]; };

-static struct opt opts[255]; +static struct opt opts[256];

#define DHCPDISCOVER 1 #define DHCPOFFER 2 @@ -374,6 +374,14 @@ int dhcp(const struct ctx *c, struct iov_tail *data) if (!type || !olen) return -1;

+ if (*type == 255) + break; + + if (*type == 0) { /* Pad Option (RFC 2132, 3.1): one byte */ + opt_len--; + continue; + } +

I don't think this is quite right: at this point we've already stripped off the non-existent length-byte with IOV_REMOVE_HEADER, meaning opt_len will get out of sync with iov_tail_size(data). I think we instead need to check for the 1-byte option cases between the two IOV_REMOVE_HEADER() calls. And.. since presumably the options could theoretically end with some padding options, we probably want the loop to be while (opt_len >= 1) instead of 2. In fact the way we mix recalculating opt_len from iov_tail_size() with sometimes directly updating it is pretty nasty. We might be better off with while ((opt_len = iov_tail_size(data)))

opt_len = iov_tail_size(data); if (opt_len < *olen) return -1; -- 2.43.0

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson