From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Sat, 8 Nov 2025 16:47:06 +0000 (-0800)
Subject: Fix infinite encoder loop encoding Message-Type as a foreign attribute
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;p=thirdparty%2Ffreeradius-server.git

Fix infinite encoder loop encoding Message-Type as a foreign attribute
---

diff --git a/src/protocols/dhcpv4/base.c b/src/protocols/dhcpv4/base.c
index b0c2350abb..fc481dcacd 100644
--- a/src/protocols/dhcpv4/base.c
+++ b/src/protocols/dhcpv4/base.c
@@ -43,7 +43,7 @@ fr_dict_t const *dict_dhcpv4;
 extern fr_dict_autoload_t dhcpv4_dict[];
 fr_dict_autoload_t dhcpv4_dict[] = {
 	{ .out = &dict_dhcpv4, .proto = "dhcpv4" },
-	
+
 	DICT_AUTOLOAD_TERMINATOR
 };
 
@@ -525,19 +525,24 @@ ssize_t fr_dhcpv4_encode_dbuff(fr_dbuff_t *dbuff, dhcp_packet_t *original, int c
 	/* DHCP magic number */
 	FR_DBUFF_IN_RETURN(&work_dbuff, (uint32_t) DHCP_OPTION_MAGIC_NUMBER);
 
-	if ((vp = fr_pair_find_by_da(vps, NULL, attr_dhcp_message_type))) {
-		FR_DBUFF_IN_BYTES_RETURN(&work_dbuff, FR_MESSAGE_TYPE, 0x01, vp->vp_uint8);
-	} else {
-		FR_DBUFF_IN_BYTES_RETURN(&work_dbuff, FR_MESSAGE_TYPE, 0x01, (uint8_t)code);
-	}
-
 	/*
 	 *  Pre-sort attributes into contiguous blocks so that fr_dhcpv4_encode_option
 	 *  operates correctly. This changes the order of the list, but never mind...
+	 *
+	 *  If attr_dhcp_message_type is present it will have been sorted as the first
+	 *  option, so we don't need to search for it.
 	 */
 	fr_pair_list_sort(vps, fr_dhcpv4_attr_cmp);
 	fr_pair_dcursor_iter_init(&cursor, vps, fr_dhcpv4_next_encodable, dict_dhcpv4);
 
+	vp = fr_dcursor_head(&cursor);
+	if (vp && vp->da == attr_dhcp_message_type) {
+		FR_DBUFF_IN_BYTES_RETURN(&work_dbuff, FR_MESSAGE_TYPE, 0x01, vp->vp_uint8);
+		fr_dcursor_next(&cursor);	/* Skip message type so it doesn't get double encoded */
+	} else {
+		FR_DBUFF_IN_BYTES_RETURN(&work_dbuff, FR_MESSAGE_TYPE, 0x01, (uint8_t)code);
+	}
+
 	/*
 	 *  Each call to fr_dhcpv4_encode_option will encode one complete DHCP option,
 	 *  and sub options.
diff --git a/src/protocols/dhcpv4/encode.c b/src/protocols/dhcpv4/encode.c
index e690a37972..c02273f6c0 100644
--- a/src/protocols/dhcpv4/encode.c
+++ b/src/protocols/dhcpv4/encode.c
@@ -739,13 +739,7 @@ ssize_t fr_dhcpv4_encode_option(fr_dbuff_t *dbuff, fr_dcursor_t *cursor, void *e
 	vp = fr_dcursor_current(cursor);
 	if (!vp) return -1;
 
-	if (vp->da == attr_dhcp_message_type) goto next; /* already done */
-	if (vp->da->attr > 255) {
-		fr_strerror_printf("Attribute \"%s\" is not a DHCP option", vp->da->name);
-	next:
-		(void)fr_dcursor_next(cursor);
-		return 0;
-	}
+	fr_assert_msg(vp->da->attr <= 255, "Cursor provided unencodable attribute to enecoder");
 
 	fr_proto_da_stack_build(&da_stack, vp->da);
 
diff --git a/src/tests/unit/protocols/radius/foreign.txt b/src/tests/unit/protocols/radius/foreign.txt
index 1833f242a1..82cf72cab6 100644
--- a/src/tests/unit/protocols/radius/foreign.txt
+++ b/src/tests/unit/protocols/radius/foreign.txt
@@ -15,7 +15,7 @@ decode-pair -
 match Extended-Attribute-5 = { DHCPv4-Options = { Time-Offset = 2112 } }
 
 #
-#  So long 
+#  So long
 #
 encode-pair Vendor-Specific = { Nokia-SR = { ToServer-Dhcp-Options = { Time-Offset = 2112 } } }
 match 1a 0e 00 00 19 7f 66 08 02 04 00 00 08 40
@@ -68,5 +68,12 @@ match Packet-Type = ::Accounting-Request, Packet-Authentication-Vector = 0xd6040
 decode-proto 1f000260b50307ffededdef5ff04f504da0000026004ffedf53cfffffdff13daf504ffed000000000c0000180000000000000076e504ffdaf504ffecf504ffddf500ffed8104ffdaf504ff82f504ffda0bfaffdaf504ffdaf504ffecf504ff73f504ffddf504ffedf504ffdaf5ff04f5ed249e0038fffe0002ff2b3100bd001f000000810f02010004000f1b00549e00e402ef046b02cf04c05400046b02cf047d41cf04e7cf02040002fe147c02cf040205cf7d02cf00047d02cf04e802cf067d02cf7a007c02027dcfcf020404e8cf067d02cf04cf02040002fe147c02cf040205cf7d02cf00047d02cf04e802cf067d02cf7a047c02027dcfcf020404e8cf067d02cf047c02cf040302cf04e8023d02cf0024151c2a160000000000000000018303d67b0303023002cf03025902cf0306bd000014fb02cf03000000000076e504ffdaf504ffecf504ffddf500ff82f504ffda0bfaffdaf504ffdaf504ffecf504ff73f504ffddf504ffedf504ffdaf5ff04f5ed249e0038fffe0002ff2b3100bd0000000000810ffeff0000000f1b00549e00e402ef046b02cf04c05400046b02cf047d41cf040000000000000076e504ffdaf504ffecf504ffddf500ffed8104ffdaf504ff82f504ffda0bfaffdaf504ffdaf504ffecf504ff73f504ffddf504ffedf504ffdaf5ff04f5ed249e0038fffe0002ff2b3100bd0000000000810ffeff0000000f1b00549e00e402ef046b02cf04c05400046b02cf047d41cf04e7cf02040002fe147c02cf040205cf7d02cf06bd02cf0302cc03030302cf03435d03594302cf02cf03025902cf03063d02cf2b063d0302cf03435902cf030302029e9e9e9e9e9e9e9e9e9e9e9e9e9e9e46160000000000000000c2c2c2c2c2c2c2e6f604ffedf104045a
 match Packet-Type = ::Terminate-Session, Packet-Authentication-Vector = 0xb50307ffededdef5ff04f504da000002, raw.Framed-Interface-Id = 0xffed, Extended-Attribute-5 = { raw.255 = 0xfdff13daf504ffed000000000c0000180000000000000076e504ffdaf504ffecf504ffddf500ffed8104ffdaf504ff82f504ffda0bfaffda, raw.DHCPv4-Options = 0xed249e0038fffe0002ff2b3100bd001f000000810f02010004000f1b00549e00e402ef046b02cf04c05400046b02cf047d41cf04e7cf02040002fe147c02cf040205cf7d02cf00047d02cf04e802cf067d02cf7a007c02027dcfcf020404e8cf067d02cf04cf02040002fe147c02cf040205cf7d02cf00047d02cf04e802cf067d02cf7a047c02027dcfcf020404e8cf067d02cf047c02cf040302cf04e8023d02cf0024151c2a160000000000000000018303d67b0303023002cf03025902cf0306bd000014fb02cf03000000000076e504ffdaf504ffecf504ffddf500ff82f504ffda0bfaffdaf504ffdaf504ffecf504ff73f504ffddf504ff }, raw.Extended-Attribute-5 = 0xffdd, raw.Extended-Attribute-5 = 0xffed, raw.Extended-Attribute-5 = 0xffda, raw.237 = 0x04ffdaf5ff04f5ed249e0038fffe0002ff2b3100bd0000000000810ffeff0000000f1b00549e00e402ef046b02cf04c05400046b02cf047d41cf040000000000000076e504ffdaf504ffecf504ffddf500ffed8104ffdaf504ff82f504ffda0bfaffdaf504ffdaf504ffecf504ff73f504ffddf504ffedf504ffdaf5ff04f5ed249e0038fffe0002ff2b3100bd0000000000810ffeff0000000f1b00549e00e402ef046b02cf04c05400046b02cf047d41cf04e7cf02040002fe147c02cf040205cf7d02cf06bd02cf0302cc03030302cf03435d03594302cf02cf03025902cf03063d02cf2b063d0302cf03435902cf030302
 
+# Regression test for infinite loop when encoding Message-Type DHCP option
+encode-pair Vendor-Specific.Nokia-SR.ToServer-Dhcp-Options.Message-Type = ::Discover
+match 1a 0b 00 00 19 7f 66 05 35 01 01
+
+decode-pair -
+match Vendor-Specific = { Nokia-SR = { ToServer-Dhcp-Options = { Message-Type = ::Discover } } }
+
 count
-match 25
+match 29