]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
Add Coverity-only check for two-byte length case (CID #1604613)
authorJames Jones <jejones3141@gmail.com>
Wed, 14 Aug 2024 21:33:52 +0000 (16:33 -0500)
committerAlan DeKok <aland@freeradius.org>
Thu, 15 Aug 2024 15:02:40 +0000 (11:02 -0400)
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.

src/lib/util/struct.c

index 17a1296a132d2f0703e1b514932b7c62906fe8e4..5d5a20d02259388c0a7f2770c4aefd6a18ba3598 100644 (file)
@@ -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);