more checks on packet sanity

diff --git a/src/protocols/dns/base.c b/src/protocols/dns/base.c
index 3cb9e7623377c91dfba8b2301262fd4dffe910c9..f9fd9ce4eeffe0049d1539a7717391bfb655d476 100644 (file)
--- a/src/protocols/dns/base.c
+++ b/src/protocols/dns/base.c
@@ -77,6 +77,28 @@ fr_dict_attr_autoload_t dns_dict_attr[] = {
 
 #define DECODE_FAIL(_reason) if (reason) *reason = DECODE_FAIL_ ## _reason
 
+static bool fr_dns_tlv_ok(uint8_t const *p, uint8_t const *end, fr_dns_decode_fail_t *reason)
+{
+       uint16_t len;
+
+       while (p < end) {
+               if ((p + 4) > end) {
+                       DECODE_FAIL(MISSING_TLV_HEADER);
+                       return false;
+               }
+
+               len = fr_net_to_uint16(p + 2);
+               if ((p + 4 + len) > end) {
+                       DECODE_FAIL(TLV_OVERFLOWS_RR);
+                       return false;
+               }
+
+               p += 4 + len;
+       }
+
+       return true;
+}
+
 bool fr_dns_packet_ok(uint8_t const *packet, size_t packet_len, bool query, fr_dns_decode_fail_t *reason)
 {
        uint8_t const *p, *end;
@@ -104,7 +126,12 @@ bool fr_dns_packet_ok(uint8_t const *packet, size_t packet_len, bool query, fr_d
 
        if (query) {
                /*
-                *      There should be at least one query, and no replies in the query!
+                *      There should be at least one query, and no
+                *      replies in the query.
+                *
+                *      @todo - unless it's an IQUERY, in which case
+                *      there should be no questions, and at least one
+                *      answer.
                 */
                if (!qdcount) {
                        DECODE_FAIL(NO_QUESTIONS);
@@ -172,14 +199,11 @@ bool fr_dns_packet_ok(uint8_t const *packet, size_t packet_len, bool query, fr_d
        while (p < end) {
                uint16_t len = 0;
                uint8_t const *start = p;
-
+               bool is_opt = false;
 
                /*
                 *      Simple DNS label decoder
                 *
-                *      @todo - use fr_dns_marker() to check for valid
-                *      pointers, too.
-                *
                 *      @todo - move this to src/lib/util/dns.c,
                 *      perhaps as fr_dns_label_verify(), and then
                 *      have it also return a pointer to the next
@@ -296,6 +320,7 @@ bool fr_dns_packet_ok(uint8_t const *packet, size_t packet_len, bool query, fr_d
                        DECODE_FAIL(MISSING_RR_HEADER);
                        return false;
                }
+               is_opt = (p[0] == 0) && (p[1] == 41);
                p += 8;
 
                /*
@@ -307,13 +332,25 @@ bool fr_dns_packet_ok(uint8_t const *packet, size_t packet_len, bool query, fr_d
                }
 
                len = fr_net_to_uint16(p);
+               if (len == 0) {
+                       DECODE_FAIL(ZERO_RR_LEN);
+                       return false;
+               }
+
                p += 2;
                if ((p + len) > end) {
                        DECODE_FAIL(RR_OVERFLOWS_PACKET);
                        return false;
                }
 
-               p+= len;
+               /*
+                *      Verify the TLVs, too.
+                */
+               if (is_opt && !fr_dns_tlv_ok(p, p + len, reason)) {
+                       return false;
+               }
+
+               p += len;
 
 next:
                count++;

diff --git a/src/protocols/dns/decode.c b/src/protocols/dns/decode.c
index 128ab5d8037c5aefb623e7c7d04ccf1e482c66c1..93a72637fc6c154c182d4028ae234742a68b55a9 100644 (file)
--- a/src/protocols/dns/decode.c
+++ b/src/protocols/dns/decode.c
@@ -672,6 +672,7 @@ static fr_table_num_ordered_t reason_fail_table[] = {
        { L("invalid label for resource record"),               DECODE_FAIL_INVALID_RR_LABEL    },
        { L("missing resource record header"),                  DECODE_FAIL_MISSING_RR_HEADER   },
        { L("missing resource record length field"),            DECODE_FAIL_MISSING_RR_LEN      },
+       { L("resource record length field is zero"),            DECODE_FAIL_ZERO_RR_LEN },
        { L("resource record length overflows the packet"),     DECODE_FAIL_RR_OVERFLOWS_PACKET },
        { L("more resource records than indicated in header"),  DECODE_FAIL_TOO_MANY_RRS        },
        { L("fewer resource records than indicated in header"), DECODE_FAIL_TOO_FEW_RRS         },
@@ -683,6 +684,8 @@ static fr_table_num_ordered_t reason_fail_table[] = {
        { L("label overflows the packet"),                      DECODE_FAIL_LABEL_OVERFLOWS_PACKET      },
        { L("too many characters in label"),                    DECODE_FAIL_LABEL_TOO_LONG              },
        { L("query record header is missing"),                  DECODE_FAIL_MISSING_QD_HEADER           },
+       { L("missing TLV header in OPT RR"),                    DECODE_FAIL_MISSING_TLV_HEADER          },
+       { L("TLV overflows enclosing RR"),                      DECODE_FAIL_TLV_OVERFLOWS_RR            },
 };
 static size_t reason_fail_table_len = NUM_ELEMENTS(reason_fail_table);
 

diff --git a/src/protocols/dns/dns.h b/src/protocols/dns/dns.h
index 19c16f941db4eb6afb3b4f235723cd9ae078556b..ea3f49091489ca66f356e586ec494e84fd80eed8 100644 (file)
--- a/src/protocols/dns/dns.h
+++ b/src/protocols/dns/dns.h
@@ -116,6 +116,7 @@ typedef enum {
        DECODE_FAIL_INVALID_RR_LABEL,
        DECODE_FAIL_MISSING_RR_HEADER,
        DECODE_FAIL_MISSING_RR_LEN,
+       DECODE_FAIL_ZERO_RR_LEN,
        DECODE_FAIL_RR_OVERFLOWS_PACKET,
        DECODE_FAIL_TOO_MANY_RRS,
        DECODE_FAIL_TOO_FEW_RRS,
@@ -127,6 +128,8 @@ typedef enum {
        DECODE_FAIL_LABEL_OVERFLOWS_PACKET,
        DECODE_FAIL_LABEL_TOO_LONG,
        DECODE_FAIL_MISSING_QD_HEADER,
+       DECODE_FAIL_MISSING_TLV_HEADER,
+       DECODE_FAIL_TLV_OVERFLOWS_RR,
        DECODE_FAIL_MAX
 } fr_dns_decode_fail_t;