fixed an exploitable hang bug

3383.	[security]	A certain combination of records in the RBT could
                        cause named to hang while populating the additional
                        section of a response. [RT #31090]

CHANGES

@@ -1,5 +1,11 @@
 	--- 9.6-ESV-R8 released ---
 
+3383.	[security]	A certain combination of records in the RBT could
+                        cause named to hang while populating the additional
+                        section of a response. [RT #31090]
+
+3373.	[bug]		win32: open raw files in binary mode. [RT #30944]
+
 3364.	[security]	Named could die on specially crafted record.
 			[RT #30416]
 

bin/check/check-tool.c

@@ -640,6 +640,9 @@ dump_zone(const char *zonename, dns_zone_t *zone, const char *filename,
 {
 	isc_result_t result;
 	FILE *output = stdout;
+	const char *flags;
+
+	flags = (fileformat == dns_masterformat_text) ? "w+" : "wb+";
 
 	if (debug) {
 		if (filename != NULL && strcmp(filename, "-") != 0)
@@ -650,7 +653,7 @@ dump_zone(const char *zonename, dns_zone_t *zone, const char *filename,
 	}
 
 	if (filename != NULL && strcmp(filename, "-") != 0) {
-		result = isc_stdio_open(filename, "w+", &output);
+		result = isc_stdio_open(filename, flags, &output);
 
 		if (result != ISC_R_SUCCESS) {
 			fprintf(stderr, "could not open output "

bin/dnssec/dnssec-signzone.c

@@ -3520,7 +3520,10 @@ main(int argc, char *argv[]) {
 	check_result(result, "isc_file_mktemplate");
 
 	fp = NULL;
-	result = isc_file_openunique(tempfile, &fp);
+	if (outputformat == dns_masterformat_text)
+		result = isc_file_openunique(tempfile, &fp);
+	else
+		result = isc_file_bopenunique(tempfile, &fp);
 	if (result != ISC_R_SUCCESS)
 		fatal("failed to open temporary output file: %s",
 		      isc_result_totext(result));

bin/named/query.c

@@ -1025,13 +1025,6 @@ query_isduplicate(ns_client_t *client, dns_name_t *name,
 		mname = NULL;
 	}
 
-	/*
-	 * If the dns_name_t we're looking up is already in the message,
-	 * we don't want to trigger the caller's name replacement logic.
-	 */
-	if (name == mname)
-		mname = NULL;
-
 	if (mnamep != NULL)
 		*mnamep = mname;
 
@@ -1230,6 +1223,7 @@ query_addadditional(void *arg, dns_name_t *name, dns_rdatatype_t qtype) {
 	if (dns_rdataset_isassociated(rdataset) &&
 	    !query_isduplicate(client, fname, type, &mname)) {
 		if (mname != NULL) {
+			INSIST(mname != fname);
 			query_releasename(client, &fname);
 			fname = mname;
 		} else
@@ -1292,11 +1286,13 @@ query_addadditional(void *arg, dns_name_t *name, dns_rdatatype_t qtype) {
 			mname = NULL;
 			if (!query_isduplicate(client, fname,
 					       dns_rdatatype_a, &mname)) {
-				if (mname != NULL) {
-					query_releasename(client, &fname);
-					fname = mname;
-				} else
-					need_addname = ISC_TRUE;
+				if (mname != fname) {
+					if (mname != NULL) {
+						query_releasename(client, &fname);
+						fname = mname;
+					} else
+						need_addname = ISC_TRUE;
+				}
 				ISC_LIST_APPEND(fname->list, rdataset, link);
 				added_something = ISC_TRUE;
 				if (sigrdataset != NULL &&
@@ -1338,11 +1334,13 @@ query_addadditional(void *arg, dns_name_t *name, dns_rdatatype_t qtype) {
 			mname = NULL;
 			if (!query_isduplicate(client, fname,
 					       dns_rdatatype_aaaa, &mname)) {
-				if (mname != NULL) {
-					query_releasename(client, &fname);
-					fname = mname;
-				} else
-					need_addname = ISC_TRUE;
+				if (mname != fname) {
+					if (mname != NULL) {
+						query_releasename(client, &fname);
+						fname = mname;
+					} else
+						need_addname = ISC_TRUE;
+				}
 				ISC_LIST_APPEND(fname->list, rdataset, link);
 				added_something = ISC_TRUE;
 				if (sigrdataset != NULL &&
@@ -1865,22 +1863,24 @@ query_addadditional2(void *arg, dns_name_t *name, dns_rdatatype_t qtype) {
 		    crdataset->type == dns_rdatatype_aaaa) {
 			if (!query_isduplicate(client, fname, crdataset->type,
 					       &mname)) {
-				if (mname != NULL) {
-					/*
-					 * A different type of this name is
-					 * already stored in the additional
-					 * section.  We'll reuse the name.
-					 * Note that this should happen at most
-					 * once.  Otherwise, fname->link could
-					 * leak below.
-					 */
-					INSIST(mname0 == NULL);
+				if (mname != fname) {
+					if (mname != NULL) {
+						/*
+						 * A different type of this name is
+						 * already stored in the additional
+						 * section.  We'll reuse the name.
+						 * Note that this should happen at most
+						 * once.  Otherwise, fname->link could
+						 * leak below.
+						 */
+						INSIST(mname0 == NULL);
 
-					query_releasename(client, &fname);
-					fname = mname;
-					mname0 = mname;
-				} else
-					need_addname = ISC_TRUE;
+						query_releasename(client, &fname);
+						fname = mname;
+						mname0 = mname;
+					} else
+						need_addname = ISC_TRUE;
+				}
 				ISC_LIST_UNLINK(cfname.list, crdataset, link);
 				ISC_LIST_APPEND(fname->list, crdataset, link);
 				added_something = ISC_TRUE;

lib/dns/master.c

@@ -34,6 +34,7 @@
 #include <isc/util.h>
 
 #include <dns/callbacks.h>
+#include <dns/compress.h>
 #include <dns/events.h>
 #include <dns/fixedname.h>
 #include <dns/master.h>
@@ -2031,19 +2032,21 @@ load_raw(dns_loadctx_t *lctx) {
 	unsigned int loop_cnt = 0;
 	dns_rdatacallbacks_t *callbacks;
 	unsigned char namebuf[DNS_NAME_MAXWIRE];
-	isc_region_t r;
-	dns_name_t name;
+	dns_fixedname_t fixed;
+	dns_name_t *name;
 	rdatalist_head_t head, dummy;
 	dns_rdatalist_t rdatalist;
 	isc_mem_t *mctx = lctx->mctx;
 	dns_rdata_t *rdata = NULL;
 	unsigned int rdata_size = 0;
 	int target_size = TSIZ;
-	isc_buffer_t target;
+	isc_buffer_t target, buf;
 	unsigned char *target_mem = NULL;
+	dns_decompress_t dctx;
 
 	REQUIRE(DNS_LCTX_VALID(lctx));
 	callbacks = lctx->callbacks;
+	dns_decompress_init(&dctx, -1, DNS_DECOMPRESS_NONE);
 
 	if (lctx->first) {
 		dns_masterrawheader_t header;
@@ -2100,6 +2103,9 @@ load_raw(dns_loadctx_t *lctx) {
 	}
 	isc_buffer_init(&target, target_mem, target_size);
 
+	dns_fixedname_init(&fixed);
+	name = dns_fixedname_name(&fixed);
+
 	/*
 	 * In the following loop, we regard any error fatal regardless of
 	 * whether "MANYERRORS" is set in the context option.  This is because
@@ -2111,7 +2117,7 @@ load_raw(dns_loadctx_t *lctx) {
 	for (loop_cnt = 0;
 	     (lctx->loop_cnt == 0 || loop_cnt < lctx->loop_cnt);
 	     loop_cnt++) {
-		unsigned int i, rdcount, consumed_name;
+		unsigned int i, rdcount;
 		isc_uint16_t namelen;
 		isc_uint32_t totallen;
 		size_t minlen, readlen;
@@ -2201,12 +2207,11 @@ load_raw(dns_loadctx_t *lctx) {
 					lctx->f);
 		if (result != ISC_R_SUCCESS)
 			goto cleanup;
+
 		isc_buffer_setactive(&target, (unsigned int)namelen);
-		isc_buffer_activeregion(&target, &r);
-		dns_name_init(&name, NULL);
-		dns_name_fromregion(&name, &r);
-		isc_buffer_forward(&target, (unsigned int)namelen);
-		consumed_name = isc_buffer_consumedlength(&target);
+		result = dns_name_fromwire(name, &target, &dctx, 0, NULL);
+		if (result != ISC_R_SUCCESS)
+			goto cleanup;
 
 		/* Rdata contents. */
 		if (rdcount > rdata_size) {
@@ -2237,7 +2242,7 @@ load_raw(dns_loadctx_t *lctx) {
 
 				/* Partial Commit. */
 				ISC_LIST_APPEND(head, &rdatalist, link);
-				result = commit(callbacks, lctx, &head, &name,
+				result = commit(callbacks, lctx, &head, name,
 						NULL, 0);
 				for (j = 0; j < i; j++) {
 					ISC_LIST_UNLINK(rdatalist.rdata,
@@ -2249,8 +2254,6 @@ load_raw(dns_loadctx_t *lctx) {
 
 				/* Rewind the buffer and continue */
 				isc_buffer_clear(&target);
-				isc_buffer_add(&target, consumed_name);
-				isc_buffer_forward(&target, consumed_name);
 
 				rdcount -= i;
 
@@ -2270,11 +2273,20 @@ load_raw(dns_loadctx_t *lctx) {
 			if (result != ISC_R_SUCCESS)
 				goto cleanup;
 			isc_buffer_setactive(&target, (unsigned int)rdlen);
-			isc_buffer_activeregion(&target, &r);
-			isc_buffer_forward(&target, (unsigned int)rdlen);
-			dns_rdata_fromregion(&rdata[i], rdatalist.rdclass,
-					     rdatalist.type, &r);
-
+			/*
+			 * It is safe to have the source active region and
+			 * the target available region be the same if
+			 * decompression is disabled (see dctx above) and we
+			 * are not downcasing names (options == 0).
+			 */
+			isc_buffer_init(&buf, isc_buffer_current(&target),
+					(unsigned int)rdlen);
+			result = dns_rdata_fromwire(&rdata[i],
+						    rdatalist.rdclass,
+						    rdatalist.type, &target,
+						    &dctx, 0, &buf);
+			if (result != ISC_R_SUCCESS)
+				goto cleanup;
 			ISC_LIST_APPEND(rdatalist.rdata, &rdata[i], link);
 		}
 
@@ -2291,7 +2303,7 @@ load_raw(dns_loadctx_t *lctx) {
 		ISC_LIST_APPEND(head, &rdatalist, link);
 
 		/* Commit this RRset.  rdatalist will be unlinked. */
-		result = commit(callbacks, lctx, &head, &name, NULL, 0);
+		result = commit(callbacks, lctx, &head, name, NULL, 0);
 
 		for (i = 0; i < rdcount; i++) {
 			ISC_LIST_UNLINK(rdatalist.rdata, &rdata[i], link);

lib/dns/masterdump.c

@@ -1483,7 +1483,8 @@ dns_master_dumptostream2(isc_mem_t *mctx, dns_db_t *db,
 }
 
 static isc_result_t
-opentmp(isc_mem_t *mctx, const char *file, char **tempp, FILE **fp) {
+opentmp(isc_mem_t *mctx, dns_masterformat_t format, const char *file,
+	char **tempp, FILE **fp) {
 	FILE *f = NULL;
 	isc_result_t result;
 	char *tempname = NULL;
@@ -1498,7 +1499,10 @@ opentmp(isc_mem_t *mctx, const char *file, char **tempp, FILE **fp) {
 	if (result != ISC_R_SUCCESS)
 		goto cleanup;
 
-	result = isc_file_openunique(tempname, &f);
+	if (format == dns_masterformat_text)
+		result = isc_file_openunique(tempname, &f);
+	else
+		result = isc_file_bopenunique(tempname, &f);
 	if (result != ISC_R_SUCCESS) {
 		isc_log_write(dns_lctx, ISC_LOGCATEGORY_GENERAL,
 			      DNS_LOGMODULE_MASTERDUMP, ISC_LOG_ERROR,
@@ -1542,7 +1546,7 @@ dns_master_dumpinc2(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version,
 	if (file == NULL)
 		return (ISC_R_NOMEMORY);
 
-	result = opentmp(mctx, filename, &tempname, &f);
+	result = opentmp(mctx, format, filename, &tempname, &f);
 	if (result != ISC_R_SUCCESS)
 		goto cleanup;
 
@@ -1596,7 +1600,7 @@ dns_master_dump2(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version,
 	char *tempname;
 	dns_dumpctx_t *dctx = NULL;
 
-	result = opentmp(mctx, filename, &tempname, &f);
+	result = opentmp(mctx, format, filename, &tempname, &f);
 	if (result != ISC_R_SUCCESS)
 		return (result);
 

lib/dns/rdata.c

@@ -421,6 +421,8 @@ dns_rdata_fromwire(dns_rdata_t *rdata, dns_rdataclass_t rdclass,
 		REQUIRE(DNS_RDATA_INITIALIZED(rdata));
 		REQUIRE(DNS_RDATA_VALIDFLAGS(rdata));
 	}
+	REQUIRE(source != NULL);
+	REQUIRE(target != NULL);
 
 	if (type == 0)
 		return (DNS_R_FORMERR);
@@ -521,13 +523,11 @@ rdata_validate(isc_buffer_t *src, isc_buffer_t *dest, dns_rdataclass_t rdclass,
 	    dns_rdatatype_t type)
 {
 	dns_decompress_t dctx;
-	dns_rdata_t rdata = DNS_RDATA_INIT;
 	isc_result_t result;
 
 	dns_decompress_init(&dctx, -1, DNS_DECOMPRESS_NONE);
 	isc_buffer_setactive(src, isc_buffer_usedlength(src));
-	result = dns_rdata_fromwire(&rdata, rdclass, type, src,
-				    &dctx, 0, dest);
+	result = dns_rdata_fromwire(NULL, rdclass, type, src, &dctx, 0, dest);
 	dns_decompress_invalidate(&dctx);
 
 	return (result);
@@ -1152,7 +1152,8 @@ txt_fromwire(isc_buffer_t *source, isc_buffer_t *target) {
 	if (n > tregion.length)
 		return (ISC_R_NOSPACE);
 
-	memcpy(tregion.base, sregion.base, n);
+	if (tregion.base != sregion.base)
+		memcpy(tregion.base, sregion.base, n);
 	isc_buffer_forward(source, n);
 	isc_buffer_add(target, n);
 	return (ISC_R_SUCCESS);
@@ -1326,7 +1327,8 @@ mem_tobuffer(isc_buffer_t *target, void *base, unsigned int length) {
 	isc_buffer_availableregion(target, &tr);
 	if (length > tr.length)
 		return (ISC_R_NOSPACE);
-	memcpy(tr.base, base, length);
+	if (tr.base != base)
+		memcpy(tr.base, base, length);
 	isc_buffer_add(target, length);
 	return (ISC_R_SUCCESS);
 }

lib/isc/api

@@ -3,6 +3,6 @@
 # 9.7: 60-79
 # 9.8: 80-89
 # 9.9: 90-109
-LIBINTERFACE = 57
-LIBREVISION = 4
-LIBAGE = 0
+LIBINTERFACE = 58
+LIBREVISION = 0
+LIBAGE = 1

lib/isc/include/isc/file.h

@@ -97,11 +97,14 @@ isc_file_mktemplate(const char *path, char *buf, size_t buflen);
  *				of the path with the internal template string.
  */
 
-
 isc_result_t
 isc_file_openunique(char *templet, FILE **fp);
+isc_result_t
+isc_file_bopenunique(char *templet, FILE **fp);
 /*!<
  * \brief Create and open a file with a unique name based on 'templet'.
+ *	isc_file_bopen*() open the file in binary mode in Windows. 
+ *	isc_file_open*() open the file in text mode in Windows. 
  *
  * Notes:
  *\li	'template' is a reserved work in C++.  If you want to complain

lib/isc/unix/file.c

@@ -302,6 +302,11 @@ isc_file_openunique(char *templet, FILE **fp) {
 	return (result);
 }
 
+isc_result_t
+isc_file_bopenunique(char *templet, FILE **fp) {
+	return (isc_file_openunique(templet, fp));
+}
+
 isc_result_t
 isc_file_remove(const char *filename) {
 	int r;

lib/isc/win32/file.c

@@ -43,10 +43,14 @@
  *
  */
 static int
-gettemp(char *path, int *doopen) {
+gettemp(char *path, isc_boolean_t binary, int *doopen) {
 	char *start, *trv;
 	struct stat sbuf;
 	int pid;
+	int flags = O_CREAT|O_EXCL|O_RDWR;
+
+	if (binary)
+		flags |= _O_BINARY;
 
 	trv = strrchr(path, 'X');
 	trv++;
@@ -79,8 +83,7 @@ gettemp(char *path, int *doopen) {
 	for (;;) {
 		if (doopen) {
 			if ((*doopen =
-			    open(path, O_CREAT|O_EXCL|O_RDWR,
-				 _S_IREAD | _S_IWRITE)) >= 0)
+			    open(path, flags, _S_IREAD | _S_IWRITE)) >= 0)
 				return (1);
 			if (errno != EEXIST)
 				return (0);
@@ -106,10 +109,10 @@ gettemp(char *path, int *doopen) {
 }
 
 static int
-mkstemp(char *path) {
+mkstemp(char *path, isc_boolean_t binary) {
 	int fd;
 
-	return (gettemp(path, &fd) ? fd : -1);
+	return (gettemp(path, binary, &fd) ? fd : -1);
 }
 
 /*
@@ -163,7 +166,7 @@ isc_file_safemovefile(const char *oldname, const char *newname) {
 		exists = TRUE;
 		strcpy(buf, newname);
 		strcat(buf, ".XXXXX");
-		tmpfd = mkstemp(buf);
+		tmpfd = mkstemp(buf, ISC_TRUE);
 		if (tmpfd > 0)
 			_close(tmpfd);
 		DeleteFile(buf);
@@ -298,7 +301,7 @@ isc_file_renameunique(const char *file, char *templet) {
 	REQUIRE(file != NULL);
 	REQUIRE(templet != NULL);
 
-	fd = mkstemp(templet);
+	fd = mkstemp(templet, ISC_TRUE);
 	if (fd == -1)
 		result = isc__errno2result(errno);
 	else
@@ -314,8 +317,8 @@ isc_file_renameunique(const char *file, char *templet) {
 	return (result);
 }
 
-isc_result_t
-isc_file_openunique(char *templet, FILE **fp) {
+static isc_result_t
+openuniquemode(char *templet, int mode, isc_boolean_t binary, FILE **fp) {
 	int fd;
 	FILE *f;
 	isc_result_t result = ISC_R_SUCCESS;
@@ -326,12 +329,17 @@ isc_file_openunique(char *templet, FILE **fp) {
 	/*
 	 * Win32 does not have mkstemp. Using emulation above.
 	 */
-	fd = mkstemp(templet);
+	fd = mkstemp(templet, binary);
 
 	if (fd == -1)
 		result = isc__errno2result(errno);
 	if (result == ISC_R_SUCCESS) {
-		f = fdopen(fd, "w+");
+#if 1
+		UNUSED(mode);
+#else
+		(void)fchmod(fd, mode);
+#endif
+		f = fdopen(fd, binary ? "wb+" : "w+");
 		if (f == NULL) {
 			result = isc__errno2result(errno);
 			(void)remove(templet);
@@ -343,6 +351,18 @@ isc_file_openunique(char *templet, FILE **fp) {
 	return (result);
 }
 
+isc_result_t
+isc_file_openunique(char *templet, FILE **fp) {
+	int mode = _S_IREAD | _S_IWRITE;
+	return (openuniquemode(templet, mode, ISC_FALSE, fp));
+}
+
+isc_result_t
+isc_file_bopenunique(char *templet, FILE **fp) {
+	int mode = _S_IREAD | _S_IWRITE;
+	return (openuniquemode(templet, mode, ISC_TRUE, fp));
+}
+
 isc_result_t
 isc_file_remove(const char *filename) {
 	int r;

lib/isc/win32/libisc.def

@@ -110,6 +110,7 @@ isc_event_allocate
 isc_event_free
 isc_file_absolutepath
 isc_file_basename
+isc_file_bopenunique
 isc_file_exists
 isc_file_getmodtime
 isc_file_isabsolute