From: Willem Toorop Date: Fri, 31 Aug 2012 12:03:18 +0000 (+0000) Subject: Changes from codereview from CZ.NIC and Paul Wouters X-Git-Tag: release-1.6.14rc1~38 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d1d8006728794ae676ea043c38046218e82f63ec;p=thirdparty%2Fldns.git Changes from codereview from CZ.NIC and Paul Wouters - Memory leak on crypto errors when tsig signing in ldns_resolver_send - Memory leak in ldns_resolver_search - free resolver also if just testing for parse in ldns_resolver_new_frm_file - don't leak empty rr_list in ldns_pkt_rr_list_by_name - free packet also if just testing for parse ldns_pkt_query_new_frm_str - free packet on memory error in ldns_pkt_query_new - free ns on continue in ldns_send_buffer - free query and ns on early exits in ldns_axfr_start - free key also if just testing for parse in ldns_key_new_frm_fp_l - rewrite of memory allocations in ldns_key_new_frm_fp_hmac_l - don't alloc a rr when no key is given in ldns_key2rr - free b64_bignum before error exit in ldns_gost_key2buffer_str - memory leak in ldns_get_rr_list_name_by_addr - memory leak in open_keyfiles in ldns-zsplit example program - close filehandle in read_key_file in ldns-verify-zone example program - memory leak on memory error in ldns_update_send_simple_addr in ldns-update example program - memory leaks in ldns_update_resolver_new in ldns-update example program - memory leak on broken syntax in read_entry in ldns-testns example program - free filepointer in read_hex_buffer in work.c in drill program - free wire data in dump_hex in work.c in drill program more are coming... --- diff --git a/dname.c b/dname.c index f3770fea..80b0f3e0 100644 --- a/dname.c +++ b/dname.c @@ -519,6 +519,18 @@ ldns_dname_str_absolute(const char *dname_str) return 0; } +bool +ldns_dname_absolute(const ldns_rdf *rdf) +{ + char *str = ldns_rdf2str(rdf); + if (str) { + bool r = ldns_dname_str_absolute(str); + LDNS_FREE(str); + return r; + } + return false; +} + ldns_rdf * ldns_dname_label(const ldns_rdf *rdf, uint8_t labelpos) { diff --git a/drill/work.c b/drill/work.c index 3a9cb585..2a28d79c 100644 --- a/drill/work.c +++ b/drill/work.c @@ -201,6 +201,10 @@ read_hex_buffer(char *filename) ldns_buffer_set_position(result_buffer, ldns_buffer_capacity(result_buffer)); xfree(wire); + + if (fp != stdin) { + fclose(fp); + } return result_buffer; } @@ -236,7 +240,7 @@ read_hex_pkt(char *filename) void dump_hex(const ldns_pkt *pkt, const char *filename) { - uint8_t *wire; + uint8_t *wire = NULL; size_t size, i; FILE *fp; ldns_status status; @@ -273,4 +277,5 @@ dump_hex(const ldns_pkt *pkt, const char *filename) } fprintf(fp, "\n"); fclose(fp); + LDNS_FREE(wire); } diff --git a/examples/ldns-testpkts.c b/examples/ldns-testpkts.c index 0bc4a7ed..033ea15d 100644 --- a/examples/ldns-testpkts.c +++ b/examples/ldns-testpkts.c @@ -485,7 +485,10 @@ read_entry(FILE* in, const char* name, int *lineno, uint32_t* default_ttl, reading_hex = false; cur_reply->reply_from_hex = data_buffer2wire(hex_data_buffer); ldns_buffer_free(hex_data_buffer); + hex_data_buffer = NULL; } else if(str_keyword(&parse, "ENTRY_END")) { + if (hex_data_buffer) + ldns_buffer_free(hex_data_buffer); return current; } else if(reading_hex) { ldns_buffer_printf(hex_data_buffer, line); diff --git a/examples/ldns-update.c b/examples/ldns-update.c index f619eec4..dca4cbe7 100644 --- a/examples/ldns-update.c +++ b/examples/ldns-update.c @@ -19,7 +19,7 @@ ldns_update_resolver_new(const char *fqdn, const char *zone, ldns_resolver *r1, *r2; ldns_pkt *query = NULL, *resp; ldns_rr_list *nslist, *iplist; - ldns_rdf *soa_zone, *soa_mname, *ns_name; + ldns_rdf *soa_zone, *soa_mname = NULL, *ns_name; size_t i; ldns_status s; @@ -96,6 +96,7 @@ ldns_update_resolver_new(const char *fqdn, const char *zone, /* Match */ iplist = ldns_get_rr_list_addr_by_name(r1, ns_name, class, 0); (void) ldns_resolver_push_nameserver_rr_list(r2, iplist); + ldns_rr_list_deep_free(iplist); break; } } @@ -109,12 +110,15 @@ ldns_update_resolver_new(const char *fqdn, const char *zone, /* No match, add it now. */ iplist = ldns_get_rr_list_addr_by_name(r1, ns_name, class, 0); (void) ldns_resolver_push_nameserver_rr_list(r2, iplist); + ldns_rr_list_deep_free(iplist); } } ldns_resolver_set_random(r2, false); ldns_pkt_free(resp); ldns_resolver_deep_free(r1); + if (soa_mname) + ldns_rdf_deep_free(soa_mname); return r2; bad: @@ -126,6 +130,8 @@ ldns_update_resolver_new(const char *fqdn, const char *zone, ldns_pkt_free(query); if (resp) ldns_pkt_free(resp); + if (soa_mname) + ldns_rdf_deep_free(soa_mname); return NULL; } @@ -138,7 +144,7 @@ ldns_update_send_simple_addr(const char *fqdn, const char *zone, ldns_pkt *u_pkt = NULL, *r_pkt; ldns_rr_list *up_rrlist; ldns_rr *up_rr; - ldns_rdf *zone_rdf; + ldns_rdf *zone_rdf = NULL; char *rrstr; uint32_t rrstrlen, status = LDNS_STATUS_OK; @@ -231,6 +237,8 @@ ldns_update_send_simple_addr(const char *fqdn, const char *zone, ldns_resolver_deep_free(res); if (u_pkt) ldns_pkt_free(u_pkt); + if (zone_rdf) + ldns_rdf_deep_free(zone_rdf); return LDNS_STATUS_ERR; } diff --git a/examples/ldns-verify-zone.c b/examples/ldns-verify-zone.c index bfdf6452..7829c43d 100644 --- a/examples/ldns-verify-zone.c +++ b/examples/ldns-verify-zone.c @@ -92,6 +92,7 @@ read_key_file(const char *filename, ldns_rr_list *keys) else break; } + fclose(fp); return status; } diff --git a/examples/ldns-zsplit.c b/examples/ldns-zsplit.c index 84f2ddb9..0843c589 100644 --- a/examples/ldns-zsplit.c +++ b/examples/ldns-zsplit.c @@ -63,6 +63,7 @@ open_keyfiles(char **files, uint16_t filec) } if (ldns_rr_new_frm_fp(&k, kfp, NULL, NULL, NULL) != LDNS_STATUS_OK) { fprintf(stderr, "Error parsing the key file %s: %s\n", files[i], strerror(errno)); + ldns_rr_list_deep_free(pubkeys); return NULL; } fclose(kfp); diff --git a/higher.c b/higher.c index c9eb1731..990fb6af 100644 --- a/higher.c +++ b/higher.c @@ -126,6 +126,7 @@ ldns_get_rr_list_name_by_addr(ldns_resolver *res, ldns_rdf *addr, ldns_rr_class /* add the RD flags, because we want an answer */ pkt = ldns_resolver_query(res, name, LDNS_RR_TYPE_PTR, c, flags | LDNS_RD); + ldns_rdf_deep_free(name); if (pkt) { /* extract the data we need */ names = ldns_pkt_rr_list_by_type(pkt, diff --git a/host2str.c b/host2str.c index ff750b5a..45e647b7 100644 --- a/host2str.c +++ b/host2str.c @@ -1743,6 +1743,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1754,6 +1755,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1767,6 +1769,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1783,6 +1786,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1799,6 +1803,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1815,6 +1820,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1831,6 +1837,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1847,6 +1854,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1880,6 +1888,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1896,6 +1905,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1912,6 +1922,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1928,6 +1939,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1944,6 +1956,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -1987,6 +2000,7 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } b64_bignum = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_B64, i, bignum); if (ldns_rdf2buffer_str(output, b64_bignum) != LDNS_STATUS_OK) { + ldns_rdf_deep_free(b64_bignum); goto error; } ldns_rdf_deep_free(b64_bignum); @@ -2020,9 +2034,6 @@ ldns_key2buffer_str(ldns_buffer *output, const ldns_key *k) } #endif /* HAVE_SSL */ } else { -#ifdef HAVE_SSL - LDNS_FREE(b64_bignum); -#endif LDNS_FREE(bignum); return ldns_buffer_status(output); } diff --git a/keys.c b/keys.c index e4e5754b..de7c9461 100644 --- a/keys.c +++ b/keys.c @@ -503,6 +503,7 @@ ldns_key_new_frm_fp_l(ldns_key **key, FILE *fp, int *line_nr) *key = k; return LDNS_STATUS_OK; } + ldns_key_free(k); return LDNS_STATUS_ERR; } @@ -749,28 +750,21 @@ ldns_key_new_frm_fp_hmac_l( FILE *f , size_t *hmac_size ) { - size_t i; - char *d; - unsigned char *buf; - - d = LDNS_XMALLOC(char, LDNS_MAX_LINELEN); - buf = LDNS_XMALLOC(unsigned char, LDNS_MAX_LINELEN); - if(!d || !buf) { - goto error; - } + size_t i, bufsz; + char d[LDNS_MAX_LINELEN]; + unsigned char *buf = NULL; if (ldns_fget_keyword_data_l(f, "Key", ": ", d, "\n", LDNS_MAX_LINELEN, line_nr) == -1) { goto error; } - i = (size_t) ldns_b64_pton((const char*)d, - buf, - ldns_b64_ntop_calculate_size(strlen(d))); + bufsz = ldns_b64_ntop_calculate_size(strlen(d)); + buf = LDNS_XMALLOC(unsigned char, bufsz); + i = (size_t) ldns_b64_pton((const char*)d, buf, bufsz); *hmac_size = i; return buf; error: - LDNS_FREE(d); LDNS_FREE(buf); *hmac_size = 0; return NULL; @@ -1381,10 +1375,10 @@ ldns_key2rr(const ldns_key *k) #endif int internal_data = 0; - pubkey = ldns_rr_new(); if (!k) { return NULL; } + pubkey = ldns_rr_new(); switch (ldns_key_algorithm(k)) { case LDNS_SIGN_HMACMD5: diff --git a/ldns/dname.h b/ldns/dname.h index a91f0752..bc06a715 100644 --- a/ldns/dname.h +++ b/ldns/dname.h @@ -177,6 +177,13 @@ int ldns_dname_interval(const ldns_rdf *prev, const ldns_rdf *middle, const ldns */ bool ldns_dname_str_absolute(const char *dname_str); +/** + * Checks whether the given dname is absolute (i.e. ends with a '.') + * \param[in] *dname a rdf representing the dname + * \return true or false + */ +bool ldns_dname_absolute(const ldns_rdf *rdf); + /** * look inside the rdf and if it is an LDNS_RDF_TYPE_DNAME * try and retrieve a specific label. The labels are numbered diff --git a/ldns_symbols.def b/ldns_symbols.def index acdb0674..fdd7de54 100644 --- a/ldns_symbols.def +++ b/ldns_symbols.def @@ -48,6 +48,7 @@ ldns_digest_evp ldns_directive_types ldns_dname2buffer_wire ldns_dname2canonical +ldns_dname_absolute ldns_dname_cat ldns_dname_cat_clone ldns_dname_clone_from diff --git a/net.c b/net.c index a61276c9..6241d2d1 100644 --- a/net.c +++ b/net.c @@ -110,12 +110,14 @@ ldns_send_buffer(ldns_pkt **result, ldns_resolver *r, ldns_buffer *qb, ldns_rdf if ((ns->ss_family == AF_INET) && (ldns_resolver_ip6(r) == LDNS_RESOLV_INET6)) { /* not reachable */ + LDNS_FREE(ns); continue; } if ((ns->ss_family == AF_INET6) && (ldns_resolver_ip6(r) == LDNS_RESOLV_INET)) { /* not reachable */ + LDNS_FREE(ns); continue; } #endif @@ -808,6 +810,9 @@ ldns_axfr_start(ldns_resolver *resolver, ldns_rdf *domain, ldns_rr_class class) ns_i < ldns_resolver_nameserver_count(resolver) && resolver->_socket == 0; ns_i++) { + if (ns != NULL) { + LDNS_FREE(ns); + } ns = ldns_rdf2native_sockaddr_storage( resolver->_nameservers[ns_i], ldns_resolver_port(resolver), &ns_len); @@ -838,6 +843,9 @@ ldns_axfr_start(ldns_resolver *resolver, ldns_rdf *domain, ldns_rr_class class) #endif resolver->_socket = 0; + ldns_pkt_free(query); + LDNS_FREE(ns); + return LDNS_STATUS_CRYPTO_TSIG_ERR; } } diff --git a/packet.c b/packet.c index 68a6704c..b902c5eb 100644 --- a/packet.c +++ b/packet.c @@ -255,7 +255,6 @@ ldns_pkt_rr_list_by_name(ldns_pkt *packet, ldns_pkt_section sec) { ldns_rr_list *rrs; - ldns_rr_list *new; ldns_rr_list *ret; uint16_t i; @@ -264,7 +263,6 @@ ldns_pkt_rr_list_by_name(ldns_pkt *packet, } rrs = ldns_pkt_get_section_clone(packet, sec); - new = ldns_rr_list_new(); ret = NULL; for(i = 0; i < ldns_rr_list_rr_count(rrs); i++) { @@ -272,8 +270,10 @@ ldns_pkt_rr_list_by_name(ldns_pkt *packet, ldns_rr_list_rr(rrs, i)), ownername) == 0) { /* owner names match */ - ldns_rr_list_push_rr(new, ldns_rr_list_rr(rrs, i)); - ret = new; + if (ret == NULL) { + ret = ldns_rr_list_new(); + } + ldns_rr_list_push_rr(ret, ldns_rr_list_rr(rrs, i)); } } return ret; @@ -875,6 +875,7 @@ ldns_pkt_query_new_frm_str(ldns_pkt **p, const char *name, ldns_rr_type rr_type, *p = packet; return LDNS_STATUS_OK; } else { + ldns_pkt_free(packet); return LDNS_STATUS_NULL; } } @@ -897,6 +898,7 @@ ldns_pkt_query_new(ldns_rdf *rr_name, ldns_rr_type rr_type, ldns_rr_class rr_cla question_rr = ldns_rr_new(); if (!question_rr) { + ldns_pkt_free(packet); return NULL; } diff --git a/resolver.c b/resolver.c index 707568cf..fd08d8e9 100644 --- a/resolver.c +++ b/resolver.c @@ -892,6 +892,7 @@ ldns_resolver_new_frm_file(ldns_resolver **res, const char *filename) *res = r; return LDNS_STATUS_OK; } else { + ldns_resolver_free(r); return LDNS_STATUS_NULL; } } @@ -954,15 +955,12 @@ ldns_resolver_search(const ldns_resolver *r,const ldns_rdf *name, ldns_rr_type t, ldns_rr_class c, uint16_t flags) { - char *str_dname; ldns_rdf *new_name; ldns_rdf **search_list; size_t i; ldns_pkt *p; - str_dname = ldns_rdf2str(name); - - if (ldns_dname_str_absolute(str_dname)) { + if (ldns_dname_absolute(name)) { /* query as-is */ return ldns_resolver_query(r, name, t, c, flags); } else if (ldns_resolver_dnsrch(r)) { @@ -1219,9 +1217,11 @@ ldns_resolver_send(ldns_pkt **answer, ldns_resolver *r, const ldns_rdf *name, ldns_resolver_tsig_keydata(r), 300, ldns_resolver_tsig_algorithm(r), NULL); if (status != LDNS_STATUS_OK) { + ldns_pkt_free(query_pkt); return LDNS_STATUS_CRYPTO_TSIG_ERR; } #else + ldns_pkt_free(query_pkt); return LDNS_STATUS_CRYPTO_TSIG_ERR; #endif /* HAVE_SSL */ }