From: Alan T. DeKok <aland@freeradius.org>
Date: Mon, 18 Oct 2021 18:27:52 +0000 (-0400)
Subject: more checks on packet sanity
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fc8bb2239852a761f61f7f80931bcbebeba42469;p=thirdparty%2Ffreeradius-server.git

more checks on packet sanity
---

diff --git a/src/protocols/dns/base.c b/src/protocols/dns/base.c
index 3cb9e762337..f9fd9ce4eef 100644
--- 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 128ab5d8037..93a72637fc6 100644
--- 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 19c16f941db..ea3f4909148 100644
--- 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;