From: Alan T. DeKok <aland@freeradius.org>
Date: Sun, 24 May 2026 13:17:21 +0000 (-0400)
Subject: tighten restrictions on decoding
X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=0ea0a8e2f596f3c2c162ee779281e16ebe11c95f;p=thirdparty%2Ffreeradius-server.git

tighten restrictions on decoding

* reject indefinite length forms for all tags
* reject lengths which are non minimal
---

diff --git a/src/protocols/der/decode.c b/src/protocols/der/decode.c
index ac8752c43e1..a53c8f71f71 100644
--- a/src/protocols/der/decode.c
+++ b/src/protocols/der/decode.c
@@ -2011,21 +2011,46 @@ static ssize_t fr_der_decode_hdr(fr_dict_attr_t const *parent, fr_dbuff_t *in, u
 		*len		= 0;
 
 		/*
-		 *	Length bits of zero is an indeterminate length field where
-		 *	the length is encoded in the data instead.
+		 *	Length-of-length of zero is the BER indefinite-length form.  DER (X.690 Section 10.1)
+		 *	forbids it for both primitive and constructed encodings.  If we accept it, then an
+		 *	attacker can hide a zero-length value with trailing bytes, which then gets reparsed as
+		 *	a sibling TLV.
 		 */
-		if (len_len > 0) {
-			if (unlikely(len_len > sizeof(*len))) {
-				fr_strerror_printf_push("Length field too large (%" PRIu32 ")", len_len);
-				return -1;
-			}
+		if (unlikely(len_len == 0)) {
+			fr_strerror_const_push("Indefinite-length form is forbidden in DER");
+			return -1;
+		}
 
-			while (len_len--) {
-				if (fr_dbuff_out(&len_byte, &our_in) < 0) goto error;
-				*len = (*len << 8) | len_byte;
-			}
-		} else if (!constructed) {
-			fr_strerror_const_push("Primitive data with indefinite form length field is invalid");
+		if (unlikely(len_len > sizeof(*len))) {
+			fr_strerror_printf_push("Length field too large (%" PRIu32 ")", len_len);
+			return -1;
+		}
+
+		/*
+		 *	DER (X.690 Section 10.1) mandates minimal length encoding.  In the long form, the
+		 *	leading length octet must not be zero, otherwise the length could be expressed in
+		 *	fewer octets.
+		 */
+		if (fr_dbuff_out(&len_byte, &our_in) < 0) goto error;
+		if (unlikely(len_byte == 0)) {
+			fr_strerror_const_push("Non-minimal DER length encoding (leading zero in long form)");
+			return -1;
+		}
+		*len = len_byte;
+		len_len--;
+
+		while (len_len--) {
+			if (fr_dbuff_out(&len_byte, &our_in) < 0) goto error;
+			*len = (*len << 8) | len_byte;
+		}
+
+		/*
+		 *	DER also mandates the short form whenever the
+		 *	length fits in 7 bits.  Reject the long form when
+		 *	the value is < 128.
+		 */
+		if (unlikely(*len < 128)) {
+			fr_strerror_printf_push("Non-minimal DER length encoding (long form used for length %zu)", *len);
 			return -1;
 		}
 
diff --git a/src/tests/unit/protocols/der/error.txt b/src/tests/unit/protocols/der/error.txt
index 5d7b9aaa1fe..c1180d34597 100644
--- a/src/tests/unit/protocols/der/error.txt
+++ b/src/tests/unit/protocols/der/error.txt
@@ -52,7 +52,7 @@ match Constructed flag mismatch for tag 1: Failed decoding Test-Boolean header
 proto-dictionary-root Test-Integer
 
 decode-pair 02 80 02 01 05 00 00
-match Primitive data with indefinite form length field is invalid: Failed decoding Test-Integer header
+match Indefinite-length form is forbidden in DER: Failed decoding Test-Integer header
 
 #
 #  ----  NULL errors  ----