From: James Jones <jejones3141@gmail.com>
Date: Tue, 3 Oct 2023 19:52:37 +0000 (-0500)
Subject: Add validation for attrlen (CID #504038)
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=baef03795d0fb71c7ee327dd97b0ac66b775cd6e;p=thirdparty%2Ffreeradius-server.git

Add validation for attrlen (CID #504038)

To satisfy coverity that attrlen is validated, insist that the
sum of the attrlen values doesn't exceed what's left of the
packet length after the header. It's done inside the loop so
each new attrlen value is checked to make coverity happy.
---

diff --git a/src/protocols/vmps/vmps.c b/src/protocols/vmps/vmps.c
index a802a2972a..39260b36e6 100644
--- a/src/protocols/vmps/vmps.c
+++ b/src/protocols/vmps/vmps.c
@@ -112,14 +112,27 @@ bool fr_vmps_ok(uint8_t const *packet, size_t *packet_len)
 
 		/*
 		 *	Length is 2 bytes
-		 *
+		 */
+		attrlen = fr_nbo_to_uint16(ptr + 4);
+
+		/*
+		 *	Total of attribute lengths shouldn't exceed *packet_len - header length,
+		 *	which happens iff at some point, attrlen exceeds data_lan.
+		 */
+		if (attrlen > data_len) {
+			fr_strerror_printf("Packet attributes cause total length "
+					   "plus header length to exceed packet length %lx",
+					   *packet_len);
+			return false;
+		}
+
+		/*
 		 *	We support short lengths, as there's no reason
 		 *	for bigger lengths to exist... admins won't be
 		 *	typing in a 32K vlan name.
 		 *
 		 *	It's OK for ethernet frames to be longer.
 		 */
-		attrlen = fr_nbo_to_uint16(ptr + 4);
 		if ((ptr[3] != 5) && (attrlen > 250)) {
 			fr_strerror_printf("Packet contains attribute with invalid length %02x %02x", ptr[4], ptr[5]);
 			return false;