From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202606 header.b=ho7T3gK/; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 30BC15A0269 for ; Thu, 16 Jul 2026 09:34:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1784187272; bh=pIqBQXpNhgKZ9SRFVMUoyobVEJLh5j5SsDPSt1L70/A=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ho7T3gK/guS9XLYXzxdiDt1wP6iic1MH5sPFoRxuSHkq3HTbo/mFaKrTmP5rPwejw p76iYF6f2fmciHQ164/31vUdIc+bcD65suLjo1rcuJd8F2FBPnntXUOmES9IVRSv9f +671fuJb6r/NEepU1DvjrxtjVJx4oe2iN5ZCp776EnTP+5ja6IYSHFd76BjIe9AHz8 kfU2UjLeXTZ04pnw7ZC69C00Mo2HMUb40pSRvSr3QhXZ5i7qZu2dCdXRZG6saQMCdv t2PQGDKJBnOEKSu41gDIh0GXFEZAX1vPALZxsebELkxm1tSA+pqGZn4IOBxqBw+++s Wm8BMMhQWMAEg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4h14Yh5kQ8z4wCC; Thu, 16 Jul 2026 17:34:32 +1000 (AEST) Date: Thu, 16 Jul 2026 17:30:21 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2 2/4] dhcp: Make option parsing more robust, explicitly handle options 0 and 255 Message-ID: References: <20260716072222.1819811-1-sbrivio@redhat.com> <20260716072222.1819811-3-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="66U265VjEp8YTfuE" Content-Disposition: inline In-Reply-To: <20260716072222.1819811-3-sbrivio@redhat.com> Message-ID-Hash: O5PHIRPX5NQWVWFFNQ6PEVN55NV5C3JC X-Message-ID-Hash: O5PHIRPX5NQWVWFFNQ6PEVN55NV5C3JC X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --66U265VjEp8YTfuE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 16, 2026 at 09:22:20AM +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). >=20 > As a result: >=20 > - 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 >=20 > - 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 >=20 > 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.). >=20 > 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. >=20 > Make the option parsing loop more robust by: >=20 > - 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 >=20 > - explicitly handle options 0 (skip one byte, continue) and 255 (stop > processing options) in the option-scanning loop >=20 > - scanning the last two bytes of options as well and using > iov_tail_size(data) directly as loop condition, instead of a rather > inconsistent usage of opt_len >=20 > This bug was found and an initial version of the patch was written by > the AISLE AI security scanning tool (https://aisle.com/platform). >=20 > Reported-by: AISLE > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > v2: > - Handle one-byte options before IOV_REMOVE_HEADER() for the length byte > - Use iov_tail_size(data) as loop condition instead of mixing things up > with opt_len >=20 > dhcp.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) >=20 > diff --git a/dhcp.c b/dhcp.c > index 1ff8cba..c3c7422 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -49,7 +49,7 @@ struct opt { > uint8_t c[255]; > }; > =20 > -static struct opt opts[255]; > +static struct opt opts[256]; > =20 > #define DHCPDISCOVER 1 > #define DHCPOFFER 2 > @@ -363,25 +363,29 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > for (i =3D 0; i < ARRAY_SIZE(opts); i++) > opts[i].clen =3D -1; > =20 > - opt_len =3D iov_tail_size(data); > - while (opt_len >=3D 2) { > + while ((opt_len =3D iov_tail_size(data))) { > uint8_t olen_storage, type_storage; > const uint8_t *olen; > uint8_t *type; > =20 > - type =3D IOV_REMOVE_HEADER(data, type_storage); > - olen =3D IOV_REMOVE_HEADER(data, olen_storage); > - if (!type || !olen) > + if (!(type =3D IOV_REMOVE_HEADER(data, type_storage))) > return -1; > =20 > - opt_len =3D iov_tail_size(data); > - if (opt_len < *olen) > + if (*type =3D=3D 255) > + break; > + > + if (*type =3D=3D 0) /* Pad Option (RFC 2132, 3.1): one byte */ > + continue; > + > + if (!(olen =3D IOV_REMOVE_HEADER(data, olen_storage))) > + return -1; > + > + if (opt_len - 2 < *olen) > return -1; > =20 > iov_to_buf(&data->iov[0], data->cnt, data->off, &opts[*type].c, *olen); > opts[*type].clen =3D *olen; > iov_drop_header(data, *olen); > - opt_len -=3D *olen; > } > =20 > opts[80].slen =3D -1; > --=20 > 2.43.0 >=20 --=20 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 --66U265VjEp8YTfuE Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmpYiIcACgkQzQJF27ox 2GdLDg/+J47zrhXNNLg1Yj1rn3Fjbquu4H8Og1og6X7Ks4o+pCFDGg9tDpEeokeB DALe2WQlmVQByyCWhjzc3W/t2B6NBzekZpQ1yLBLpZllU4Hs+HkWqItNhh01023s tdYNcR3z4vveSbCKlJFcwaQmsadM+AN9U33vfna8GISQ43Aj0TAYNyIXDnfomChQ V6nqAyKae73mMtwliapNFafp2TDmidwhwJGtrQkDrPKlX4GDSxB0/VtNBLVl0zbs 2IShBg/WB5wWGYv2NHKFadXI6royQTYhgaztmdQbx4ajGqvj84uTV7e02Je6PhA9 ZSdEMNZMvkhxrox5i9eXnfmEcSKCgpQbvSPV+N1TaItgxWoO7ITxCghYFHi5ePdc ag9DH93ZoRL3YECvDn/UHfzueiX9R12tXTiz/qbC/E+QMqxgVtVziVVY0KUHMbNZ oEX3eeS4tlvVMO+aN7/QhLBb9HxLTUilOeM20NKTQ/+W6OKpQ3VHAGb2Nv+BgOBi FEVNQYecd2nOQqQ/CLlOABHRnmjCqrOA3Yd9OUgoS2hb8+8WZacHUrhhQA/uGRTq +a2P2uaZ0Yx57PHvQ4p4VsHgdBhVbbxvFZLu+of3dal+2iVIoLJWuSo+1xc6CfDF eT7rFt6tvUjyGOrrt15w8DRC4VcFd+6KrzjU3NvbcR05NOAitBo= =w98B -----END PGP SIGNATURE----- --66U265VjEp8YTfuE--