3364. [security] Named could die on specially crafted record.

[RT #30416]

CHANGES

@@ -1,3 +1,6 @@
+3364.	[security]	Named could die on specially crafted record.
+			[RT #30416]
+
 3363.	[bug]		Need to allow "forward" and "fowarders" options
 			in static-stub zones; this had been overlooked.
 			[RT #30482]

lib/dns/include/dns/rdata.h

@@ -146,6 +146,17 @@ struct dns_rdata {
 #define DNS_RDATA_VALIDFLAGS(rdata) \
 	(((rdata)->flags & ~(DNS_RDATA_UPDATE|DNS_RDATA_OFFLINE)) == 0)
 
+/*
+ * The maximum length of a RDATA that can be sent on the wire.
+ * Max packet size (65535) less header (12), less name (1), type (2),
+ * class (2), ttl(4), length (2).
+ *
+ * None of the defined types that support name compression can exceed
+ * this and all new types are to be sent uncompressed.
+ */
+
+#define DNS_RDATA_MAXLENGTH	65512U
+
 /*
  * Flags affecting rdata formatting style.  Flags 0xFFFF0000
  * are used by masterfile-level formatting and defined elsewhere.

lib/dns/master.c

@@ -75,7 +75,7 @@
 /*%
  * max message size - header - root - type - class - ttl - rdlen
  */
-#define MINTSIZ (65535 - 12 - 1 - 2 - 2 - 4 - 2)
+#define MINTSIZ DNS_RDATA_MAXLENGTH
 /*%
  * Size for tokens in the presentation format,
  * The largest tokens are the base64 blocks in KEY and CERT records,

lib/dns/rdata.c

@@ -438,6 +438,7 @@ dns_rdata_fromwire(dns_rdata_t *rdata, dns_rdataclass_t rdclass,
 	isc_buffer_t st;
 	isc_boolean_t use_default = ISC_FALSE;
 	isc_uint32_t activelength;
+	size_t length;
 
 	REQUIRE(dctx != NULL);
 	if (rdata != NULL) {
@@ -467,6 +468,14 @@ dns_rdata_fromwire(dns_rdata_t *rdata, dns_rdataclass_t rdclass,
 		}
 	}
 
+	/*
+	 * Reject any rdata that expands out to more than DNS_RDATA_MAXLENGTH
+	 * as we cannot transmit it.
+	 */
+	length = isc_buffer_usedlength(target) - isc_buffer_usedlength(&st);
+	if (result == ISC_R_SUCCESS && length > DNS_RDATA_MAXLENGTH)
+		result = DNS_R_FORMERR;
+
 	/*
 	 * We should have consumed all of our buffer.
 	 */
@@ -475,8 +484,7 @@ dns_rdata_fromwire(dns_rdata_t *rdata, dns_rdataclass_t rdclass,
 
 	if (rdata != NULL && result == ISC_R_SUCCESS) {
 		region.base = isc_buffer_used(&st);
-		region.length = isc_buffer_usedlength(target) -
+		region.length = length;
-				isc_buffer_usedlength(&st);
 		dns_rdata_fromregion(rdata, rdclass, type, &region);
 	}
 
@@ -611,6 +619,7 @@ dns_rdata_fromtext(dns_rdata_t *rdata, dns_rdataclass_t rdclass,
 	unsigned long line;
 	void (*callback)(dns_rdatacallbacks_t *, const char *, ...);
 	isc_result_t tresult;
+	size_t length;
 
 	REQUIRE(origin == NULL || dns_name_isabsolute(origin) == ISC_TRUE);
 	if (rdata != NULL) {
@@ -682,10 +691,13 @@ dns_rdata_fromtext(dns_rdata_t *rdata, dns_rdataclass_t rdclass,
 		}
 	} while (1);
 
+	length = isc_buffer_usedlength(target) - isc_buffer_usedlength(&st);
+	if (result == ISC_R_SUCCESS && length > DNS_RDATA_MAXLENGTH)
+		result = ISC_R_NOSPACE;
+
 	if (rdata != NULL && result == ISC_R_SUCCESS) {
 		region.base = isc_buffer_used(&st);
-		region.length = isc_buffer_usedlength(target) -
+		region.length = length;
-				isc_buffer_usedlength(&st);
 		dns_rdata_fromregion(rdata, rdclass, type, &region);
 	}
 	if (result != ISC_R_SUCCESS) {
@@ -819,6 +831,7 @@ dns_rdata_fromstruct(dns_rdata_t *rdata, dns_rdataclass_t rdclass,
 	isc_buffer_t st;
 	isc_region_t region;
 	isc_boolean_t use_default = ISC_FALSE;
+	size_t length;
 
 	REQUIRE(source != NULL);
 	if (rdata != NULL) {
@@ -833,10 +846,13 @@ dns_rdata_fromstruct(dns_rdata_t *rdata, dns_rdataclass_t rdclass,
 	if (use_default)
 		(void)NULL;
 
+	length = isc_buffer_usedlength(target) - isc_buffer_usedlength(&st);
+	if (result == ISC_R_SUCCESS && length > DNS_RDATA_MAXLENGTH)
+		result = ISC_R_NOSPACE;
+
 	if (rdata != NULL && result == ISC_R_SUCCESS) {
 		region.base = isc_buffer_used(&st);
-		region.length = isc_buffer_usedlength(target) -
+		region.length = length;
-				isc_buffer_usedlength(&st);
 		dns_rdata_fromregion(rdata, rdclass, type, &region);
 	}
 	if (result != ISC_R_SUCCESS)

lib/dns/rdataslab.c

@@ -305,6 +305,7 @@ dns_rdataslab_fromrdataset(dns_rdataset_t *rdataset, isc_mem_t *mctx,
 		length = x[i].rdata.length;
 		if (rdataset->type == dns_rdatatype_rrsig)
 			length++;
+		INSIST(length <= 0xffff);
 		*rawbuf++ = (length & 0xff00) >> 8;
 		*rawbuf++ = (length & 0x00ff);
 #if DNS_RDATASET_FIXED

lib/dns/tests/Makefile.in

@@ -39,15 +39,16 @@ LIBS =		@LIBS@ @ATFLIBS@
 OBJS =		dnstest.@O@
 SRCS =		dnstest.c master_test.c dbiterator_test.c time_test.c \
 		private_test.c update_test.c zonemgr_test.c zt_test.c \
-		dbdiff_test.c nsec3_test.c dispatch_test.c rdatasetstats_test.c \
+		dbdiff_test.c nsec3_test.c dispatch_test.c rbt_test.c \
-		rbt_test.c rdataset_test.c
+		rdata_test.c rdataset_test.c rdatasetstats_test.c
 
 SUBDIRS =
 TARGETS =	master_test@EXEEXT@ dbiterator_test@EXEEXT@ time_test@EXEEXT@ \
 		private_test@EXEEXT@ update_test@EXEEXT@ zonemgr_test@EXEEXT@ \
 		zt_test@EXEEXT@ dbversion_test@EXEEXT@ dbdiff_test@EXEEXT@ \
-		nsec3_test@EXEEXT@ dispatch_test@EXEEXT@ rdatasetstats_test@EXEEXT@ \
+		nsec3_test@EXEEXT@ dispatch_test@EXEEXT@ rbt_test@EXEEXT@ \
-		rbt_test@EXEEXT@ rdataset_test@EXEEXT@
+		rdata_test@EXEEXT@ rdataset_test@EXEEXT@ \
+		rdatasetstats_test@EXEEXT@ 
 
 @BIND9_MAKE_RULES@
 
@@ -127,6 +128,10 @@ rbt_test@EXEEXT@: rbt_test.@O@ dnstest.@O@ ${ISCDEPLIBS} ${DNSDEPLIBS}
 			rbt_test.@O@ dnstest.@O@ ${DNSLIBS} \
 				${ISCLIBS} ${LIBS}
 
+rdata_test@EXEEXT@: rdata_test.@O@ ${ISCDEPLIBS} ${DNSDEPLIBS}
+	${LIBTOOL_MODE_LINK} ${PURIFY} ${CC} ${CFLAGS} ${LDFLAGS} -o $@ \
+			rdata_test.@O@ ${DNSLIBS} ${ISCLIBS} ${LIBS}
+
 unit::
 	sh ${top_srcdir}/unit/unittest.sh
 

lib/dns/tests/master_test.c

@@ -44,7 +44,7 @@
  */
 
 #define	BUFLEN		255
-#define	BIGBUFLEN	(64 * 1024)
+#define	BIGBUFLEN	(70 * 1024)
 #define TEST_ORIGIN	"test"
 
 static dns_masterrawheader_t header;
@@ -251,6 +251,51 @@ ATF_TC_BODY(badclass, tc) {
 	dns_test_end();
 }
 
+/* Too big rdata test */
+ATF_TC(toobig);
+ATF_TC_HEAD(toobig, tc) {
+	atf_tc_set_md_var(tc, "descr", "dns_master_loadfile() returns "
+				       "ISC_R_NOSPACE when record is too big");
+}
+ATF_TC_BODY(toobig, tc) {
+	isc_result_t result;
+
+	UNUSED(tc);
+
+	result = dns_test_begin(NULL, ISC_FALSE);
+	ATF_REQUIRE_EQ(result, ISC_R_SUCCESS);
+
+	setup_master();
+	result = test_master("testdata/master/master15.data",
+			     dns_masterformat_text);
+	ATF_REQUIRE_EQ(result, ISC_R_NOSPACE);
+
+	dns_test_end();
+}
+
+/* Maximum rdata test */
+ATF_TC(maxrdata);
+ATF_TC_HEAD(maxrdata, tc) {
+	atf_tc_set_md_var(tc, "descr", "dns_master_loadfile() returns "
+				       "ISC_R_SUCCESS when record is maximum "
+				       "size");
+}
+ATF_TC_BODY(maxrdata, tc) {
+	isc_result_t result;
+
+	UNUSED(tc);
+
+	result = dns_test_begin(NULL, ISC_FALSE);
+	ATF_REQUIRE_EQ(result, ISC_R_SUCCESS);
+
+	setup_master();
+	result = test_master("testdata/master/master16.data",
+			     dns_masterformat_text);
+	ATF_REQUIRE_EQ(result, ISC_R_SUCCESS);
+
+	dns_test_end();
+}
+
 /* DNSKEY test */
 ATF_TC(dnskey);
 ATF_TC_HEAD(dnskey, tc) {
@@ -596,6 +641,8 @@ ATF_TP_ADD_TCS(tp) {
 	ATF_TP_ADD_TC(tp, totext);
 	ATF_TP_ADD_TC(tp, loadraw);
 	ATF_TP_ADD_TC(tp, dumpraw);
+	ATF_TP_ADD_TC(tp, toobig);
+	ATF_TP_ADD_TC(tp, maxrdata);
 
 	return (atf_no_error());
 }

lib/dns/tests/rdata_test.c

@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+ * REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+ * INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+ * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+ * OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+ * PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/* $Id$ */
+
+/*! \file */
+
+#include <config.h>
+
+#include <atf-c.h>
+
+#include <unistd.h>
+
+#include <isc/types.h>
+
+#include <dns/rdata.h>
+
+#include "dnstest.h"
+
+
+/*
+ * Individual unit tests
+ */
+
+/* Successful load test */
+ATF_TC(hip);
+ATF_TC_HEAD(hip, tc) {
+	atf_tc_set_md_var(tc, "descr", "that a oversized HIP record will "
+				       "be rejected");
+}
+ATF_TC_BODY(hip, tc) {
+	unsigned char hipwire[DNS_RDATA_MAXLENGTH] = {
+				    0x01, 0x00, 0x00, 0x01, 0x00, 0x00,
+				    0x04, 0x41, 0x42, 0x43, 0x44, 0x00 };
+	unsigned char buf[1024*1024];
+	isc_buffer_t source, target;
+	dns_rdata_t rdata;
+	dns_decompress_t dctx;
+	isc_result_t result;
+	size_t i;
+
+	UNUSED(tc);
+
+	/*
+	 * Fill the rest of input buffer with compression pointers.
+	 */
+	for (i = 12; i < sizeof(hipwire) - 2; i += 2) {
+		hipwire[i] = 0xc0;
+		hipwire[i+1] = 0x06;
+	}
+
+	isc_buffer_init(&source, hipwire, sizeof(hipwire));
+	isc_buffer_add(&source, sizeof(hipwire));
+	isc_buffer_setactive(&source, i);
+	isc_buffer_init(&target, buf, sizeof(buf));
+	dns_rdata_init(&rdata);
+	dns_decompress_init(&dctx, -1, DNS_DECOMPRESS_ANY);
+	result = dns_rdata_fromwire(&rdata, dns_rdataclass_in,
+				    dns_rdatatype_hip, &source, &dctx,
+				    0, &target);
+	dns_decompress_invalidate(&dctx);
+	ATF_REQUIRE_EQ(result, DNS_R_FORMERR);
+}
+
+/*
+ * Main
+ */
+ATF_TP_ADD_TCS(tp) {
+	ATF_TP_ADD_TC(tp, hip);
+
+	return (atf_no_error());
+}
+