From: Alan T. DeKok <aland@freeradius.org>
Date: Sat, 16 Oct 2021 20:49:26 +0000 (-0400)
Subject: more errors and more pointer validation
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=86448daf65fa4ee8a98710cd30d0c719342eba02;p=thirdparty%2Ffreeradius-server.git

more errors and more pointer validation
---

diff --git a/src/protocols/dns/base.c b/src/protocols/dns/base.c
index 282616e00ee..3cb9e762337 100644
--- 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 5cfdcbd9517..128ab5d8037 100644
--- 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 a0ed04d822a..19c16f941db 100644
--- 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,