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 a66a931..5fd922d 100644 (file)
--- a/src/modules/proto_dhcp/dhcp.c
+++ b/src/modules/proto_dhcp/dhcp.c
@@ -629,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
                 *
@@ -774,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;