From: James Jones <jejones3141@gmail.com>
Date: Sun, 12 Jan 2025 20:48:01 +0000 (-0600)
Subject: Switch Coverity-only code to assert (CID #1619299) (#5441)
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=71bba04df2beed3b47da2c3c08b17ea57df3926c;p=thirdparty%2Ffreeradius-server.git

Switch Coverity-only code to assert (CID #1619299) (#5441)

fr_nbo_from_uint64v() does not have an error return--it doesn't
need one. The buffers are big enough, the "| 0x80" means it will
always use as least one byte, so fr_high_bit_pos() won't return 0
even if num == 0. So adding a bogus error return check for Coverity
actually misleads Coverity about any call to fr_nbo_from_uint64v(),
making it the probable cause of the CID.

Co-authored-by: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
---

diff --git a/src/lib/util/nbo.h b/src/lib/util/nbo.h
index 25eb27f9877..627eef671c7 100644
--- a/src/lib/util/nbo.h
+++ b/src/lib/util/nbo.h
@@ -123,10 +123,12 @@ static inline size_t fr_nbo_from_uint64v(uint8_t out[static sizeof(uint64_t)], u
 	ret = ROUND_UP_DIV((size_t)fr_high_bit_pos(num | 0x80), 8);
 #ifdef __COVERITY__
 	/*
-	 * 	Coverity doesn't realize that ret is necessarily <= 8,
-	 * 	so we give it a hint.
+	 * Coverity doesn't realize that the fr_high_bit_pos() call will always
+	 * return a value between 1 and 8 inclusive, the former thanks to the
+	 * "| 0x80". and this function doesn't specify an error return value,
+	 * so we use a Coverity-only assert.
 	 */
-	if (ret > 8) return 1;
+	if (!fr_cond_assert((ret >= 1) && (ret <= 8))) return 1;
 #endif
 
 	fr_nbo_from_uint64(swapped, num);