From: James Jones <jejones3141@gmail.com>
Date: Wed, 14 Aug 2024 21:33:52 +0000 (-0500)
Subject: Add Coverity-only check for two-byte length case (CID #1604613)
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2a93744a7cb9c6571bb1a5c794d0fed07015e473;p=thirdparty%2Ffreeradius-server.git

Add Coverity-only check for two-byte length case (CID #1604613)

In fr_struct_to_network(), for structs prefixed by a length, the
length can be either one or two bytes. Space is set aside for it,
and when it comes time to encode it, you skip the appropriate number
of bytes and decrement length correspondingly. Coverity lets the one
byte length version pass without complaint, but in the two-byte
length case thinks length is 0 and hence underflows when 2 is subtracted
from it.

We add a Coverity-only check that returns an error if len < 2; it
never will be, but the check should persuade Coverity that at the
decrement, len will be at least 2.
---

diff --git a/src/lib/util/struct.c b/src/lib/util/struct.c
index 17a1296a132..5d5a20d0225 100644
--- a/src/lib/util/struct.c
+++ b/src/lib/util/struct.c
@@ -855,6 +855,17 @@ done:
 
 			(void) fr_dbuff_in(&hdr, (uint8_t) length);
 		} else {
+#ifdef __COVERITY__
+			/*
+			 * 	If you look at the code where do_length is set, work_dbuff
+			 * 	is extended by one byte for FLAG_LENGTH_UINT8, two bytes
+			 * 	otherwise, so the decrease in length will never make it
+			 * 	underflow. Coverity appears to recognize the former case
+			 * 	but not the latter, so we add a Coverity-only check that
+			 * 	will reassure it.
+			 */
+			if (length < 2) return PAIR_ENCODE_FATAL_ERROR;
+#endif
 			length -= 2;
 
 			length += da_length_offset(parent);