From d1024a4ab37a4a72fb382568bac48a80ea5ab0fc Mon Sep 17 00:00:00 2001
From: Willem Toorop <willem@NLnetLabs.nl>
Date: Fri, 24 Aug 2012 13:26:41 +0000
Subject: [PATCH] Changes from codereview from CZ.NIC and Paul Wouters - use of
 pkt pointer before test for NULL in ldns_dnssec_build_data_chain - Memory
 leak on memory error in ldns_tsig_mac_new - Smaller stack ocuupation in
 read_key_file in drill_util.c - Potential for filedescriptor leak in
 ldns_init_random - Memory leak on memory error in ldns_str2rdf_apl - Memory
 leak (not freeing hexdata) in ldns_rr_new_frm_str_internal - Memory leak when
 testing for parse in ldns_rr_new_frm_str_interal - Memory leak when testing
 for parse in ldns_rr_new_frm_fp_l - Memory leak on memory error in
 ldns_rr_list_sort - Memory leak when popping zero items with
 ldns_rr_list_pop_rr_list

---
 Changelog          |  1 +
 dnssec_verify.c    |  2 ++
 drill/drill_util.c |  2 +-
 rr.c               | 17 ++++++++++++++---
 str2host.c         |  1 +
 tsig.c             |  8 +++++---
 util.c             |  1 +
 7 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/Changelog b/Changelog
index a8fe7fa6..ff454158 100644
--- a/Changelog
+++ b/Changelog
@@ -1,4 +1,5 @@
 1.6.14
+	* Various bugfixes from code reviews from CZ.NIC and Paul Wouters
 	* ldns-notify TSIG option argument checking
 	* Let ldns_resolver_nameservers_randomize keep nameservers and rtt's
 	  in sync.
diff --git a/dnssec_verify.c b/dnssec_verify.c
index 1d93e7ba..1af7f182 100644
--- a/dnssec_verify.c
+++ b/dnssec_verify.c
@@ -285,6 +285,8 @@ ldns_dnssec_build_data_chain(ldns_resolver *res,
 	ldns_rr_class c = 0;
 
 	bool other_rrset = false;
+
+	assert(pkt != NULL);
 	
 	ldns_dnssec_data_chain *new_chain = ldns_dnssec_data_chain_new();
 
diff --git a/drill/drill_util.c b/drill/drill_util.c
index 98d88e79..a3753c82 100644
--- a/drill/drill_util.c
+++ b/drill/drill_util.c
@@ -40,7 +40,7 @@ read_key_file(const char *filename, ldns_rr_list *key_list)
 	int line_len = 0;
 	int line_nr = 0;
 	int key_count = 0;
-	char line[LDNS_MAX_PACKETLEN];
+	char line[LDNS_MAX_LINELEN];
 	ldns_status status;
 	FILE *input_file;
 	ldns_rr *rr;
diff --git a/rr.c b/rr.c
index 0254d476..72076d40 100644
--- a/rr.c
+++ b/rr.c
@@ -477,6 +477,7 @@ ldns_rr_new_frm_str_internal(ldns_rr **newrr, const char *str,
                                                                 ldns_buffer_free(rr_buf);
                                                                 LDNS_FREE(rdata);
                                                                 ldns_rr_free(new);
+								LDNS_FREE(hex_data);
                                                                 return s;
                                                         }
 							LDNS_FREE(hex_data);
@@ -600,6 +601,9 @@ ldns_rr_new_frm_str_internal(ldns_rr **newrr, const char *str,
 
 	if (newrr) {
 		*newrr = new;
+	} else {
+		/* Maybe the caller just wanted to see if it would parse? */
+		ldns_rr_free(new);
 	}
 	return LDNS_STATUS_OK;
 
@@ -724,8 +728,13 @@ ldns_rr_new_frm_fp_l(ldns_rr **newrr, FILE *fp, uint32_t *default_ttl, ldns_rdf
 		}
 	}
 	LDNS_FREE(line);
-	if (newrr && s == LDNS_STATUS_OK) {
-		*newrr = rr;
+	if (s == LDNS_STATUS_OK) {
+		if (newrr) {
+			*newrr = rr;
+		} else {
+			/* Just testing if it would parse? */
+			ldns_rr_free(rr);
+		}
 	}
 	return s;
 }
@@ -1156,7 +1165,8 @@ ldns_rr_list_pop_rr_list(ldns_rr_list *rr_list, size_t howmany)
 		i--;
 	}
 
-	if (i == howmany) {
+	if (i == howmany) { /* so i <= 0 */
+		ldns_rr_list_free(popped);
 		return NULL;
 	} else {
 		return popped;
@@ -1480,6 +1490,7 @@ ldns_rr_list_sort(ldns_rr_list *unsorted)
                                         LDNS_FREE(sortables[i]);
                                 }
                                 /* no way to return error */
+				LDNS_FREE(sortables);
                                 return;
                         }
 			sortables[i]->original_object = ldns_rr_list_rr(unsorted, i);
diff --git a/str2host.c b/str2host.c
index 3553c0d5..b5e4be10 100644
--- a/str2host.c
+++ b/str2host.c
@@ -534,6 +534,7 @@ ldns_str2rdf_apl(ldns_rdf **rd, const char *str)
 
 	data = LDNS_XMALLOC(uint8_t, 4 + afdlength);
         if(!data) {
+		LDNS_FREE(afdpart);
 		LDNS_FREE(my_ip_str);
 		return LDNS_STATUS_INVALID_STR;
         }
diff --git a/tsig.c b/tsig.c
index f2f0a3f3..41693463 100644
--- a/tsig.c
+++ b/tsig.c
@@ -179,10 +179,12 @@ ldns_tsig_mac_new(ldns_rdf **tsig_mac, uint8_t *pkt_wire, size_t pkt_wire_size,
 		return LDNS_STATUS_NULL;
 	}
 	canonical_key_name_rdf  = ldns_rdf_clone(key_name_rdf);
+	if (canonical_key_name_rdf == NULL) {
+		return LDNS_STATUS_MEM_ERR;
+	}
 	canonical_algorithm_rdf = ldns_rdf_clone(algorithm_rdf);
-
-	if (canonical_key_name_rdf == NULL 
-			|| canonical_algorithm_rdf  == NULL) {
+	if (canonical_algorithm_rdf == NULL) {
+		ldns_rdf_deep_free(canonical_key_name_rdf);
 		return LDNS_STATUS_MEM_ERR;
 	}
 	/*
diff --git a/util.c b/util.c
index 475d2326..f5462c4b 100644
--- a/util.c
+++ b/util.c
@@ -404,6 +404,7 @@ ldns_init_random(FILE *fd, unsigned int size)
 
 	if (read < size) {
 		LDNS_FREE(seed);
+		if (!fd) fclose(rand_f);
 		return 1;
 	} else {
 #ifdef HAVE_SSL
-- 
2.47.3