FR-GV-304 - check for option overflowing the packet

[freeradius.git] / src / modules / proto_dhcp / dhcp.c

diff --git a/src/modules/proto_dhcp/dhcp.c b/src/modules/proto_dhcp/dhcp.c
index ea8369b..5fd922d 100644 (file)
--- a/src/modules/proto_dhcp/dhcp.c
+++ b/src/modules/proto_dhcp/dhcp.c
@@ -1,3 +1,4 @@
+
 /*
  * dhcp.c      Functions to send/receive dhcp packets.
  *
@@ -628,6 +629,24 @@ static int fr_dhcp_decode_suboption(TALLOC_CTX *ctx, VALUE_PAIR **tlv, uint8_t c
                uint32_t        attr;
 
                /*
+                *      Not enough room for the option header, it's a
+                *      bad packet.
+                */
+               if ((p + 2) > (data + len)) {
+                       fr_pair_list_free(&head);
+                       return -1;
+               }
+
+               /*
+                *      Not enough room for the option header + data,
+                *      it's a bad packet.
+                */
+               if ((p + 2 + p[1]) > (data + len)) {
+                       fr_pair_list_free(&head);
+                       return -1;
+               }
+
+               /*
                 *      The initial OID string looks like:
                 *      <iana>.0
                 *
@@ -773,25 +792,23 @@ static int fr_dhcp_attr2vp(TALLOC_CTX *ctx, VALUE_PAIR **vp_p, uint8_t const *da
                 *      multiple additional VPs
                 */
                fr_cursor_init(&cursor, vp_p);
-               for (;;) {
-                       q = memchr(p, '\0', q - p);
+               while (p < end) {
+                       q = memchr(p, '\0', end - p);
                        /* Malformed but recoverable */
                        if (!q) q = end;
 
                        fr_pair_value_bstrncpy(vp, (char const *)p, q - p);
                        p = q + 1;
 
+                       if (p >= end) break;
+
                        /* Need another VP for the next round */
-                       if (p < end) {
-                               vp = fr_pair_afrom_da(ctx, vp->da);
-                               if (!vp) {
-                                       fr_pair_list_free(vp_p);
-                                       return -1;
-                               }
-                               fr_cursor_insert(&cursor, vp);
-                               continue;
+                       vp = fr_pair_afrom_da(ctx, vp->da);
+                       if (!vp) {
+                               fr_pair_list_free(vp_p);
+                               return -1;
                        }
-                       break;
+                       fr_cursor_insert(&cursor, vp);
                }
        }
                break;
@@ -1096,8 +1113,8 @@ int fr_dhcp_decode(RADIUS_PACKET *packet)
                        /*
                         *      Vendor is "MSFT 98"
                         */
-                       vp = fr_pair_find_by_num(head, 63, DHCP_MAGIC_VENDOR, TAG_ANY);
-                       if (vp && (strcmp(vp->vp_strvalue, "MSFT 98") == 0)) {
+                       vp = fr_pair_find_by_num(head, 60, DHCP_MAGIC_VENDOR, TAG_ANY);
+                       if (vp && (vp->vp_length >= 7) && (memcmp(vp->vp_octets, "MSFT 98", 7) == 0)) {
                                vp = fr_pair_find_by_num(head, 262, DHCP_MAGIC_VENDOR, TAG_ANY);
 
                                /*
@@ -1155,11 +1172,13 @@ int8_t fr_dhcp_attr_cmp(void const *a, void const *b)
         *      DHCP-Message-Type is first, for simplicity.
         */
        if ((my_a->da->attr == PW_DHCP_MESSAGE_TYPE) && (my_b->da->attr != PW_DHCP_MESSAGE_TYPE)) return -1;
+       if ((my_a->da->attr != PW_DHCP_MESSAGE_TYPE) && (my_b->da->attr == PW_DHCP_MESSAGE_TYPE)) return +1;
 
        /*
         *      Relay-Agent is last
         */
-       if ((my_a->da->attr == PW_DHCP_OPTION_82) && (my_b->da->attr != PW_DHCP_OPTION_82)) return 1;
+       if ((my_a->da->attr == PW_DHCP_OPTION_82) && (my_b->da->attr != PW_DHCP_OPTION_82)) return +1;
+       if ((my_a->da->attr != PW_DHCP_OPTION_82) && (my_b->da->attr == PW_DHCP_OPTION_82)) return -1;
 
        if (my_a->da->attr < my_b->da->attr) return -1;
        if (my_a->da->attr > my_b->da->attr) return 1;