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 b0c2350abb4eb3066f71b767b06eb74048419325..fc481dcacd4b69f25985c7b84db8ed9088805f4b 100644 (file)
--- 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 e690a37972d8f2801e968ccac0db513d2d0420ce..c02273f6c093ab6b3f6e60eacece6d4bc3448a27 100644 (file)
--- 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 1833f242a195241b8ea0e94964050e4c4c412936..82cf72cab68909e922934af6bbb764ceff074fc5 100644 (file)
--- 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