more errors and more pointer validation

diff --git a/src/protocols/dns/base.c b/src/protocols/dns/base.c
index 282616e00ee7b88069c8375876552ff36c27b1f9..3cb9e7623377c91dfba8b2301262fd4dffe910c9 100644 (file)
--- a/src/protocols/dns/base.c
+++ b/src/protocols/dns/base.c
@@ -87,6 +87,11 @@ bool fr_dns_packet_ok(uint8_t const *packet, size_t packet_len, bool query, fr_d
                return false;
        }
 
+       if (packet_len > 65535) {
+               DECODE_FAIL(MAX_LENGTH_PACKET);
+               return false;
+       }
+
        /*
         *      query=0, response=1
         */
@@ -128,6 +133,18 @@ bool fr_dns_packet_ok(uint8_t const *packet, size_t packet_len, bool query, fr_d
        p = packet + DNS_HDR_LEN;
        end = packet + packet_len;
 
+       /*
+        *      We track valid label targets in a simple array (up to
+        *      2^14 bits of compressed pointer).
+        *
+        *      Note that some labels might appear in the RRDATA
+        *      field, and we don't verify those here.  However, this
+        *      function will verify the most common packets.  As a
+        *      result, any issues with overflow, etc. are more
+        *      difficult to exploit.
+        */
+       memset(fr_dns_marker, 0, packet_len < (1 << 14) ? packet_len : (1 << 14));
+
        /*
         *      Check for wildly fake packets, by making rough
         *      estimations.  This way we don't actually have to walk
@@ -159,6 +176,15 @@ bool fr_dns_packet_ok(uint8_t const *packet, size_t packet_len, bool query, fr_d
 
                /*
                 *      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
+                *      label?  fr_dns_label_uncompressed_length()
+                *      does similar but slightly different things.
                 */
                while (p < end) {
                        /*
@@ -200,6 +226,11 @@ bool fr_dns_packet_ok(uint8_t const *packet, size_t packet_len, bool query, fr_d
                                        return false;
                                }
 
+                               if (!fr_dns_marker[offset]) {
+                                       DECODE_FAIL(POINTER_TO_NON_LABEL);
+                                       return false;
+                               }
+
                                /*
                                 *      A compressed pointer is the end of the current label.
                                 */
@@ -232,6 +263,12 @@ bool fr_dns_packet_ok(uint8_t const *packet, size_t packet_len, bool query, fr_d
                                return false;
                        }
 
+                       /*
+                        *      Remember that this is where we have a
+                        *      label.
+                        */
+                       fr_dns_marker[p - packet] = 1;
+
                        /*
                         *      Go to the next label.
                         */

diff --git a/src/protocols/dns/decode.c b/src/protocols/dns/decode.c
index 5cfdcbd9517a68e3b22db4d9e00865d4ba869e5d..128ab5d8037c5aefb623e7c7d04ccf1e482c66c1 100644 (file)
--- a/src/protocols/dns/decode.c
+++ b/src/protocols/dns/decode.c
@@ -664,6 +664,7 @@ static int decode_test_ctx(void **out, TALLOC_CTX *ctx)
 static fr_table_num_ordered_t reason_fail_table[] = {
        { L("none"),                                            DECODE_FAIL_NONE                },
        { L("packet is smaller than DNS header"),               DECODE_FAIL_MIN_LENGTH_PACKET   },
+       { L("packet is larger than 65535"),                     DECODE_FAIL_MAX_LENGTH_PACKET   },
        { L("expected query / answer, got answer / query"),     DECODE_FAIL_UNEXPECTED          },
        { L("no 'questions' in query packet"),                  DECODE_FAIL_NO_QUESTIONS        },
        { L("unexprected answers in query packet"),             DECODE_FAIL_ANSWERS_IN_QUESTION },
@@ -676,6 +677,7 @@ static fr_table_num_ordered_t reason_fail_table[] = {
        { L("fewer resource records than indicated in header"), DECODE_FAIL_TOO_FEW_RRS         },
        { L("pointer overflows packet"),                        DECODE_FAIL_POINTER_OVERFLOWS_PACKET    },
        { L("pointer points to packet header"),                 DECODE_FAIL_POINTER_TO_HEADER           },
+       { L("pointer does not point to a label"),               DECODE_FAIL_POINTER_TO_NON_LABEL        },
        { L("pointer creates a loop"),                          DECODE_FAIL_POINTER_LOOPS               },
        { L("invalid pointer"),                                 DECODE_FAIL_INVALID_POINTER             },
        { L("label overflows the packet"),                      DECODE_FAIL_LABEL_OVERFLOWS_PACKET      },

diff --git a/src/protocols/dns/dns.h b/src/protocols/dns/dns.h
index a0ed04d822a5742dc8daa97be66eba3392c6d5cf..19c16f941db4eb6afb3b4f235723cd9ae078556b 100644 (file)
--- a/src/protocols/dns/dns.h
+++ b/src/protocols/dns/dns.h
@@ -108,6 +108,7 @@ typedef enum {
 typedef enum {
        DECODE_FAIL_NONE = 0,
        DECODE_FAIL_MIN_LENGTH_PACKET,
+       DECODE_FAIL_MAX_LENGTH_PACKET,
        DECODE_FAIL_UNEXPECTED,
        DECODE_FAIL_NO_QUESTIONS,
        DECODE_FAIL_ANSWERS_IN_QUESTION,
@@ -118,6 +119,7 @@ typedef enum {
        DECODE_FAIL_RR_OVERFLOWS_PACKET,
        DECODE_FAIL_TOO_MANY_RRS,
        DECODE_FAIL_TOO_FEW_RRS,
+       DECODE_FAIL_POINTER_TO_NON_LABEL,
        DECODE_FAIL_POINTER_OVERFLOWS_PACKET,
        DECODE_FAIL_POINTER_TO_HEADER,
        DECODE_FAIL_POINTER_LOOPS,