add verify_tlvs() function

so we don't go crazy creating structures, only to throw them
away when we discover that something bad is going on

diff --git a/src/lib/util/decode.c b/src/lib/util/decode.c
index dc9d505f13644f8cabdef1372660aeefede0659a..14b175b69cb028df2f6c78b9c41e957dcd5a2a8c 100644 (file)
--- a/src/lib/util/decode.c
+++ b/src/lib/util/decode.c
@@ -123,6 +123,7 @@ ssize_t fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_a
  * @param[in] data_len         of data to parse.
  * @param[in] decode_ctx       passed to decode_tlv
  * @param[in] decode_tlv       function to decode one attribute / option / tlv
+ * @param[in] verify_tlvs      simple function to see if the TLVs are even vaguely well-formed
  * @param[in] nested           whether or not we create nested VPs.
  *     <0 on error - decode error, or OOM
  *     data_len on success
@@ -138,7 +139,9 @@ ssize_t fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_a
 ssize_t fr_pair_tlvs_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
                                  fr_dict_attr_t const *parent,
                                  uint8_t const *data, size_t const data_len,
-                                 void *decode_ctx, fr_pair_decode_value_t decode_tlv, bool nested)
+                                 void *decode_ctx, fr_pair_decode_value_t decode_tlv,
+                                 fr_pair_tlvs_verify_t verify_tlvs,
+                                 bool nested)
 {
        uint8_t const *p, *end;
        fr_pair_list_t tlvs, *list;
@@ -150,6 +153,12 @@ ssize_t fr_pair_tlvs_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
        if (!fr_cond_assert_msg((parent->type == FR_TYPE_TLV || (parent->type == FR_TYPE_VENDOR)),
                                "%s: Internal sanity check failed, attribute \"%s\" is not of type 'tlv'",
                                __FUNCTION__, parent->name)) return PAIR_DECODE_FATAL_ERROR;
+
+       /*
+        *      Do a quick sanity check to see if the TLVs are at all OK.
+        */
+       if (verify_tlvs && !verify_tlvs(data, data_len)) return fr_pair_raw_from_network(ctx, out, parent, data, data_len);
+
        p = data;
        end = data + data_len;
 
@@ -168,7 +177,8 @@ ssize_t fr_pair_tlvs_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
                ssize_t slen;
 
                slen = decode_tlv(child_ctx, list, parent, p, (end - p), decode_ctx);
-               if (slen <= 0) {
+               if (slen < 0) {
+                       FR_PROTO_TRACE("    tlv decode failed at offset %zu - converting to raw", (size_t) (p - data));
                        fr_pair_list_free(list);
                        talloc_free(vp);
                        return fr_pair_raw_from_network(ctx, out, parent, data, data_len);

diff --git a/src/lib/util/decode.h b/src/lib/util/decode.h
index 166963545f517629592e7b06f724c8fa2c5d9c3d..f296fc2c2501d26b835a175785cd9ef95505f780 100644 (file)
--- a/src/lib/util/decode.h
+++ b/src/lib/util/decode.h
@@ -45,6 +45,8 @@ typedef ssize_t (*fr_pair_decode_value_t)(TALLOC_CTX *ctx, fr_pair_list_t *out,
                                           fr_dict_attr_t const *parent,
                                           uint8_t const *data, size_t const data_len, void *decode_ctx);
 
+typedef bool (*fr_pair_tlvs_verify_t)(uint8_t const *data, size_t const data_len);
+
 #define PROTO_DECODE_FUNC(_name) static ssize_t decode_ ## _name(TALLOC_CTX *ctx, fr_pair_list_t *out, \
                                           fr_dict_attr_t const *parent, \
                                           uint8_t const *data, size_t const data_len, void *decode_ctx)
@@ -58,7 +60,8 @@ ssize_t fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_a
 ssize_t fr_pair_tlvs_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
                                  fr_dict_attr_t const *parent,
                                  uint8_t const *data, size_t const data_len,
-                                 void *decode_ctx, fr_pair_decode_value_t decode_tlv, bool nested) CC_HINT(nonnull(1,2,3,4,7));
+                                 void *decode_ctx, fr_pair_decode_value_t decode_tlv, fr_pair_tlvs_verify_t verify_tlvs,
+                                 bool nested) CC_HINT(nonnull(1,2,3,4,7));
 
 ssize_t fr_pair_dns_labels_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
                                        fr_dict_attr_t const *parent, uint8_t const *start,

diff --git a/src/protocols/dhcpv4/decode.c b/src/protocols/dhcpv4/decode.c
index 242bf68ebba0c8c4b2b26030bcf21400533fd00a..23c384f132fc2953297f8da34b2d7a52ccc62d98 100644 (file)
--- a/src/protocols/dhcpv4/decode.c
+++ b/src/protocols/dhcpv4/decode.c
@@ -36,11 +36,27 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out,
                              fr_dict_attr_t const *parent,
                              uint8_t const *data, size_t const data_len, void *decode_ctx);
 
+static bool verify_tlvs(uint8_t const *data, size_t data_len)
+{
+       uint8_t const *p = data;
+       uint8_t const *end = data + data_len;
+
+       while (p < end) {
+               if ((end - p) < 2) return false;
+
+               if ((p + p[1]) > end) return false;
+
+               p += 2 + p[1];
+       }
+
+       return true;
+}
+
 static ssize_t decode_tlv_trampoline(TALLOC_CTX *ctx, fr_pair_list_t *out,
                                     fr_dict_attr_t const *parent,
                                     uint8_t const *data, size_t const data_len, void *decode_ctx)
 {
-       return fr_pair_tlvs_from_network(ctx, out, parent, data, data_len, decode_ctx, decode_option, false);
+       return fr_pair_tlvs_from_network(ctx, out, parent, data, data_len, decode_ctx, decode_option, verify_tlvs, false);
 }
 
 static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent,
@@ -388,7 +404,7 @@ next:
        }
        p++;
 
-       len = fr_pair_tlvs_from_network(ctx, out, vendor, p, option_len, decode_ctx, decode_option, false);
+       len = fr_pair_tlvs_from_network(ctx, out, vendor, p, option_len, decode_ctx, decode_option, verify_tlvs, false);
        if (len <= 0) return len;
 
        p += len;
@@ -453,10 +469,10 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out,
        if (!da) {
                da = fr_dict_unknown_attr_afrom_num(packet_ctx->tmp_ctx, parent, option);
                if (!da) return PAIR_DECODE_OOM;
-       }
-       FR_PROTO_TRACE("decode context changed %s -> %s",da->parent->name, da->name);
 
-       if ((da->type == FR_TYPE_STRING) && da_is_dns_label(da)) {
+               slen = fr_pair_raw_from_network(ctx, out, da, data + 2, len);
+
+       } else if ((da->type == FR_TYPE_STRING) && da_is_dns_label(da)) {
                slen = fr_pair_dns_labels_from_network(ctx, out, da, data + 2, data + 2, len, NULL, true);
 
        } else if (da->flags.array) {
@@ -466,7 +482,7 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out,
                slen = decode_vsa(ctx, out, da, data + 2, len, decode_ctx);
 
        } else if (da->type == FR_TYPE_TLV) {
-               slen = fr_pair_tlvs_from_network(ctx, out, da, data + 2, len, decode_ctx, decode_option, false);
+               slen = fr_pair_tlvs_from_network(ctx, out, da, data + 2, len, decode_ctx, decode_option, verify_tlvs, false);
 
        } else {
                slen = decode_value(ctx, out, da, data + 2, len, decode_ctx);
@@ -572,7 +588,8 @@ ssize_t fr_dhcpv4_decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out,
                        slen = decode_vsa(ctx, out, da, packet_ctx->buffer, q - packet_ctx->buffer, packet_ctx);
 
                } else if (da->type == FR_TYPE_TLV) {
-                       slen = fr_pair_tlvs_from_network(ctx, out, da, packet_ctx->buffer, q - packet_ctx->buffer, packet_ctx, decode_option, false);
+                       slen = fr_pair_tlvs_from_network(ctx, out, da, packet_ctx->buffer, q - packet_ctx->buffer,
+                                                        packet_ctx, decode_option, verify_tlvs, false);
 
                } else if (da->flags.array) {
                        slen = fr_pair_array_from_network(ctx, out, da, packet_ctx->buffer, q - packet_ctx->buffer, packet_ctx, decode_value);

diff --git a/src/protocols/dhcpv6/decode.c b/src/protocols/dhcpv6/decode.c
index c30b7b1ec55309c73b87e783c5715b8bca25b1cc..1adfa10bead6df8def88bd8d51eb15a80b25f83b 100644 (file)
--- a/src/protocols/dhcpv6/decode.c
+++ b/src/protocols/dhcpv6/decode.c
@@ -44,7 +44,7 @@ static ssize_t decode_tlv_trampoline(TALLOC_CTX *ctx, fr_pair_list_t *out,
                                     fr_dict_attr_t const *parent,
                                     uint8_t const *data, size_t const data_len, void *decode_ctx)
 {
-       return fr_pair_tlvs_from_network(ctx, out, parent, data, data_len, decode_ctx, decode_option, true);
+       return fr_pair_tlvs_from_network(ctx, out, parent, data, data_len, decode_ctx, decode_option, NULL, true);
 }
 
 static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
@@ -194,7 +194,7 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
                 *      header, as we're just decoding the values
                 *      here.
                 */
-               slen = fr_pair_tlvs_from_network(vp, &vp->vp_group, fr_dict_root(dict_dhcpv6), data, data_len, decode_ctx, decode_option, false);
+               slen = fr_pair_tlvs_from_network(vp, &vp->vp_group, fr_dict_root(dict_dhcpv6), data, data_len, decode_ctx, decode_option, NULL, false);
                if (slen < 0) {
                        talloc_free(vp);
                        return slen;
@@ -291,7 +291,7 @@ static ssize_t decode_vsa(TALLOC_CTX *ctx, fr_pair_list_t *out,
 
        FR_PROTO_TRACE("decode context %s -> %s", parent->name, da->name);
 
-       return fr_pair_tlvs_from_network(ctx, out, da, data + 4, data_len - 4, decode_ctx, decode_option, false);
+       return fr_pair_tlvs_from_network(ctx, out, da, data + 4, data_len - 4, decode_ctx, decode_option, NULL, false);
 }
 
 static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out,
@@ -364,7 +364,7 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out,
                slen = decode_vsa(ctx, out, da, data + 4, len, decode_ctx);
 
        } else if (da->type == FR_TYPE_TLV) {
-               slen = fr_pair_tlvs_from_network(ctx, out, da, data + 4, len, decode_ctx, decode_option, false);
+               slen = fr_pair_tlvs_from_network(ctx, out, da, data + 4, len, decode_ctx, decode_option, NULL, false);
 
        } else {
                slen = decode_value(ctx, out, da, data + 4, len, decode_ctx);

diff --git a/src/protocols/dns/decode.c b/src/protocols/dns/decode.c
index 4c12f2cae0ecdb2a414ed2095b8cdf62833c5d5e..f4ed6f3a0ee7bcad7c0efcbc907a8f726b9c6470 100644 (file)
--- a/src/protocols/dns/decode.c
+++ b/src/protocols/dns/decode.c
@@ -45,7 +45,7 @@ static ssize_t decode_tlv_trampoline(TALLOC_CTX *ctx, fr_pair_list_t *out,
                                     fr_dict_attr_t const *parent,
                                     uint8_t const *data, size_t const data_len, void *decode_ctx)
 {
-       return fr_pair_tlvs_from_network(ctx, out, parent, data, data_len, decode_ctx, decode_option, true);
+       return fr_pair_tlvs_from_network(ctx, out, parent, data, data_len, decode_ctx, decode_option, NULL, true);
 }
 
 

diff --git a/src/protocols/radius/decode.c b/src/protocols/radius/decode.c
index 95b13de0cbd81a8b00ad47f4ab407acb513e0eb7..50797a3f0f23edda6e256633026dc3d8fd91af54 100644 (file)
--- a/src/protocols/radius/decode.c
+++ b/src/protocols/radius/decode.c
@@ -478,7 +478,7 @@ static ssize_t decode_rfc(TALLOC_CTX *ctx, fr_pair_list_t *out,
                slen = fr_pair_array_from_network(ctx, out, da, data + 2, len - 2, decode_ctx, decode_value);
 
        } else if (da->type == FR_TYPE_TLV) {
-               slen = fr_pair_tlvs_from_network(ctx, out, da, data + 2, len - 2, decode_ctx, decode_rfc, false);
+               slen = fr_pair_tlvs_from_network(ctx, out, da, data + 2, len - 2, decode_ctx, decode_rfc, NULL, false);
 
        } else {
                slen = decode_value(ctx, out, da, data + 2, len - 2, decode_ctx);
@@ -623,7 +623,7 @@ static ssize_t decode_digest_attributes(TALLOC_CTX *ctx, fr_pair_list_t *out,
        if (!vp) return PAIR_DECODE_OOM;
 
 redo:
-       slen = fr_pair_tlvs_from_network(vp, &vp->vp_group, parent, p + 2, p[1] - 2, packet_ctx, decode_rfc, false);
+       slen = fr_pair_tlvs_from_network(vp, &vp->vp_group, parent, p + 2, p[1] - 2, packet_ctx, decode_rfc, NULL, false);
        if (slen <= 0) {
                talloc_free(vp);
                return slen;