]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
allow dns label decoding to stop at the 0x00 byte
authorAlan T. DeKok <aland@freeradius.org>
Fri, 13 Aug 2021 15:49:14 +0000 (11:49 -0400)
committerAlan T. DeKok <aland@freeradius.org>
Fri, 13 Aug 2021 17:53:28 +0000 (13:53 -0400)
so that when we're decoding DNS labels in a struct, or in an
array, we can stop when the labels end.  The next field is then
whatever happens to be after the DNS labels.

However, when they're encoded in an option, they must exactly
fill the option, otherwise it's an error

src/lib/util/dns.c
src/protocols/dhcpv6/decode.c

index 586fd0e1f059b6974c6dad0d956556ac5f9812aa..fb25da7e32573783626ff5fb674ef03d6c1944ce 100644 (file)
@@ -876,7 +876,7 @@ ssize_t fr_dns_label_uncompressed_length(uint8_t const *buf, size_t buf_len, uin
  * @param[in] buf_len  total length of the buffer
  * @return
  *     - <=0 on error, where in the buffer the invalid label is located.
- *     - > 0 total size of the labels.  SHOULD be buf_len
+ *     - > 0 total size of the encoded label(s).  Will be <= buf_len
  */
 ssize_t fr_dns_labels_network_verify(uint8_t const *buf, size_t buf_len)
 {
@@ -885,11 +885,16 @@ ssize_t fr_dns_labels_network_verify(uint8_t const *buf, size_t buf_len)
        uint8_t const *end = buf + buf_len;
 
        for (label = buf; label < end; /* nothing */) {
+               if (*label == 0x00) {
+                       label++;
+                       break;
+               }
+
                slen = fr_dns_label_uncompressed_length(buf, buf_len, &label);
                if (slen <= 0) return slen; /* already is offset from 'buf' and not 'label' */
        }
 
-       return buf_len;
+       return label - buf;
 }
 
 static ssize_t dns_label_decode(uint8_t const *buf, uint8_t const **start, uint8_t const **next)
index e2c353b8468e9893d0609e09ab2008d76b3f50b8..ee38718fd3ca8f047fc75481dcd814a5abae61a5 100644 (file)
@@ -360,7 +360,7 @@ static ssize_t decode_dns_labels(TALLOC_CTX *ctx, fr_dcursor_t *cursor, fr_dict_
                                 uint8_t const *data, size_t const data_len, void *decode_ctx)
 {
        ssize_t slen;
-       size_t total;
+       size_t total, labels_len;
        fr_pair_t *vp;
        uint8_t const *next = data;
 
@@ -381,23 +381,29 @@ static ssize_t decode_dns_labels(TALLOC_CTX *ctx, fr_dcursor_t *cursor, fr_dict_
                 */
                if (next != (data + data_len)) goto raw;
 
+               labels_len = next - data; /* decode only what we've found */
        } else {
                /*
-                *      If any one of the labels are invalid, then treat the
-                *      entire set as invalid.
+                *      Get the length of the entire set of labels, up
+                *      to (and including) the final 0x00.
+                *
+                *      If any of the labels point outside of this
+                *      area, OR they are otherwise invalid, then that's an error.
                 */
                slen = fr_dns_labels_network_verify(data, data_len);
                if (slen < 0) {
                raw:
                        return decode_raw(ctx, cursor, dict, parent, data, data_len, decode_ctx);
                }
+
+               labels_len = slen;
        }
 
        /*
         *      Loop over the input buffer, decoding the labels one by
         *      one.
         */
-       for (total = 0; total < data_len; total += slen) {
+       for (total = 0; total < labels_len; total += slen) {
                vp = fr_pair_afrom_da(ctx, parent);
                if (!vp) return PAIR_DECODE_OOM;
 
@@ -406,7 +412,7 @@ static ssize_t decode_dns_labels(TALLOC_CTX *ctx, fr_dcursor_t *cursor, fr_dict_
                 *      function should never fail unless there's a
                 *      bug in the code.
                 */
-               slen = fr_dns_label_to_value_box(vp, &vp->data, data, data_len, data + total, true);
+               slen = fr_dns_label_to_value_box(vp, &vp->data, data, labels_len, data + total, true);
                if (slen <= 0) {
                        talloc_free(vp);
                        goto raw;
@@ -416,7 +422,7 @@ static ssize_t decode_dns_labels(TALLOC_CTX *ctx, fr_dcursor_t *cursor, fr_dict_
                fr_dcursor_append(cursor, vp);
        }
 
-       return data_len;
+       return labels_len;
 }
 
 
@@ -559,6 +565,13 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_dcursor_t *cursor, fr_dict_t co
                fr_dcursor_insert(cursor, vp);
        } else if ((da->type == FR_TYPE_STRING) && !da->flags.extra && da->flags.subtype) {
                slen = decode_dns_labels(ctx, cursor, dict, da, data + 4, len, decode_ctx);
+               if (slen < 0) return slen;
+
+               /*
+                *      The DNS labels may only partially fill the
+                *      option.  If so, that's an error.
+                */
+               if ((size_t) slen != len) return -slen;
 
        } else if (da->flags.array) {
                slen = decode_array(ctx, cursor, dict, da, data + 4, len, decode_ctx);