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=DBq4gEn1; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 880CF5A026E for ; Thu, 16 Jul 2026 07:45:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1784180743; bh=vXSA7zTzWFH0fSb2GH7FluhbrZhU2Uunl5128XWPorg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=DBq4gEn1uqmvy0synkuB6/99rlWfpScmBYqbTWcVITzQyXnl/CPizvbua4QXs6Tnb RnoZengXvIZzt39bOk/IXpIKJMLGJTzJqFE+a0/jdRqkGpE0+0/IRfJL8THc0hKgvE W/B3d3X4Sbij2LeDnIq7CaKdYXwUJs4Gz9dehFZedTTjYIBcy/1EDgYv792rgVQnR+ VRQXWbVCwq9GndGTqpKgx/pr7YrwB2Nys+5+nO5K0ikLfGL0E7kGbVbtR+LarP9lQ3 rmkuUIsm3qgw1/9K4pqIEd4h1C5XsfmhMtV6UmbcgAjDjGc2+nD+Zub9idMaaooUk1 mmxXD8smzCA3A== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4h12870Mnvz4wCX; Thu, 16 Jul 2026 15:45:43 +1000 (AEST) Date: Thu, 16 Jul 2026 15:37:13 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 2/4] dhcp: Make option parsing more robust, explicitly handle options 0 and 255 Message-ID: References: <20260715232523.3372714-1-sbrivio@redhat.com> <20260715232523.3372714-3-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="fHjh+4TIcJjCaPSK" Content-Disposition: inline In-Reply-To: <20260715232523.3372714-3-sbrivio@redhat.com> Message-ID-Hash: I4YHKK7FQUT4OIMXNSETYT64TJEWJGKL X-Message-ID-Hash: I4YHKK7FQUT4OIMXNSETYT64TJEWJGKL 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: --fHjh+4TIcJjCaPSK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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). >=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 > 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 > --- > dhcp.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) >=20 > 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]; > }; > =20 > -static struct opt opts[255]; > +static struct opt opts[256]; > =20 > #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; > =20 > + if (*type =3D=3D 255) > + break; > + > + if (*type =3D=3D 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 >=3D 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 =3D iov_tail_size(data))) > opt_len =3D iov_tail_size(data); > if (opt_len < *olen) > return -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 --fHjh+4TIcJjCaPSK Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmpYbgAACgkQzQJF27ox 2Gc2aQ//ecWeLzYQgllbnIEyRm3sdOuJe/Xt790yXHN+3yblOHmGwB1/0CFhW0a5 qnxktNnuKWJry0jvTRN8tHxfFrmHh0LjBjmPHOwoFHcfhnnWxkxFGGWNM32H0RzY P7kb2/vh4flY0n1PgJBYua44Td8nzZBRkE4TRWCKSc4ne73FCmFWFiTUL1p7EORl ExIKWp1EqqN9bU00EgQnc6VQ5krQFCMV3VEEcy/zGk9IooPYSwVQS8K0FkED6aG5 cG+2prhQOirR61GpU1wlFK3+W/7v7bnC/QUPfjRwLdpqkhf6R7+v1xBM3mylqbG4 n7eoI7bTxm3FfYsCGYJ9N0gt1ynaFrrfXYlvYp2harHC71Kj+R3bzkgDSgAEJH9N OcEku60g4JORCab7rLOhluGbU508i+7x8QvX838O0ya9HYQVZVQi0zAkps+xCY9E FcXR7R2hcaROt1dqmn0MbbmYZ96ecN96lbd8LW1H6SD1WPkcV/dNcK2oHPcYf0lC pGvmA9h58pA7u2lxmSZfc0fmRMAapsTwvtOPHIZirdL53eO+sdFle2Cj4fPOE2IU XX5PaVlQPovi4uZAjKZ/D/tNTc0qJh6Cde1DIyqugf2XUI91pE3ISgP8MZzxDBy5 UXek5mu5TCI6XxyrxTVAxeo60f9D5p65LuzP7cN16haoZSorXAY= =OA8l -----END PGP SIGNATURE----- --fHjh+4TIcJjCaPSK--