From: Alan T. DeKok <aland@freeradius.org>
Date: Tue, 8 Mar 2022 20:51:44 +0000 (-0500)
Subject: minor cleanups
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a4d1b276abdf332ff7e946764ddf5a2f59daa856;p=thirdparty%2Ffreeradius-server.git

minor cleanups

use correct da_is_length_field() macro
"too long" data isn't an assert.  It's a "can't encode" error
---

diff --git a/src/protocols/dhcpv6/decode.c b/src/protocols/dhcpv6/decode.c
index ac604a3f1c9..7771bd73901 100644
--- a/src/protocols/dhcpv6/decode.c
+++ b/src/protocols/dhcpv6/decode.c
@@ -296,6 +296,7 @@ static ssize_t decode_array(TALLOC_CTX *ctx, fr_pair_list_t *out,
 			 *	decode the last bit as raw data.
 			 */
 			if ((size_t) (end - p) < element_len) {
+			raw:
 				slen = decode_raw(ctx, out, parent, p, end - p , decode_ctx);
 				if (slen < 0) return slen;
 				break;
@@ -319,22 +320,20 @@ static ssize_t decode_array(TALLOC_CTX *ctx, fr_pair_list_t *out,
 	 *	If the data is variable length i.e. strings or octets
 	 *	there is a length field before each element.
 	 *
+	 *	Note that we don't bother checking if the data type is
+	 *	string or octets.  There will only be issues if
+	 *	someone edited the dictionaries and broke them.
+	 *
 	 *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+
 	 *   |       text-len                |        String                 |
 	 *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+
 	 */
 	while (p < end) {
-		if ((end - p) < 2) {
-		raw:
-			slen = decode_raw(ctx, out, parent, p, end - p , decode_ctx);
-			if (slen < 0) return slen;
-			break;
-		}
+		if ((end - p) < 2) goto raw;
 
 		element_len = fr_net_to_uint16(p);
-		if ((p + 2 + element_len) > end) {
-			goto raw;
-		}
+
+		if ((size_t) (end - p) < (((size_t) element_len) + 2)) goto raw;
 
 		p += 2;
 		slen = decode_value(ctx, out, parent, p, element_len, decode_ctx);
diff --git a/src/protocols/dhcpv6/encode.c b/src/protocols/dhcpv6/encode.c
index a202d810108..eb552ec79f7 100644
--- a/src/protocols/dhcpv6/encode.c
+++ b/src/protocols/dhcpv6/encode.c
@@ -441,14 +441,14 @@ static inline ssize_t encode_array(fr_dbuff_t *dbuff,
 		 *   |       text-len                |        String                 |
 		 *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+
 		 */
-		if (!da->flags.length) {
+		if (da_is_length_field(da)) {
 			len_field = true;
 			FR_DBUFF_ADVANCE_RETURN(&element_dbuff, sizeof(uint16_t));	/* Make room for the length field */
 		}
 
 		slen = encode_value(&element_dbuff, da_stack, depth, cursor, encode_ctx);
 		if (slen < 0) return slen;
-		if (!fr_cond_assert(slen < UINT16_MAX)) return PAIR_ENCODE_FATAL_ERROR;
+		if (slen > UINT16_MAX) return PAIR_ENCODE_FATAL_ERROR;
 
 		/*
 		 *	Ensure we always create elements of the correct length.
@@ -467,10 +467,9 @@ static inline ssize_t encode_array(fr_dbuff_t *dbuff,
 		/*
 		 *	Populate the length field
 		 */
-		if (len_field) fr_dbuff_in(&work_dbuff, (uint16_t) slen);
+		else if (len_field) fr_dbuff_in(&work_dbuff, (uint16_t) slen);
 		fr_dbuff_set(&work_dbuff, &element_dbuff);
 
-
 		vp = fr_dcursor_current(cursor);
 		if (!vp || (vp->da != da)) break;		/* Stop if we have an attribute of a different type */
 	}