Skip to content

Commit

Permalink
Issues detected in ldns library by scan of Coverity
Browse files Browse the repository at this point in the history
Thanks Petr Menšík
  • Loading branch information
wtoorop committed Jul 11, 2019
1 parent 05134f5 commit db06eb4
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 42 deletions.
6 changes: 5 additions & 1 deletion dnssec.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ ldns_dnssec_nsec3_closest_encloser(const ldns_rdf *qname,
LDNS_FREE(salt);
ldns_rdf_deep_free(zone_name);
ldns_rdf_deep_free(sname);
ldns_rdf_deep_free(hashed_sname);
return NULL;
}

Expand Down Expand Up @@ -1561,6 +1562,7 @@ ldns_pkt_verify_time(const ldns_pkt *p, ldns_rr_type t, const ldns_rdf *o,
ldns_rr_list *sigs_covered;
ldns_rdf *rdf_t;
ldns_rr_type t_netorder;
ldns_status status;

if (!k) {
return LDNS_STATUS_ERR;
Expand Down Expand Up @@ -1612,7 +1614,9 @@ ldns_pkt_verify_time(const ldns_pkt *p, ldns_rr_type t, const ldns_rdf *o,
}
return LDNS_STATUS_ERR;
}
return ldns_verify_time(rrset, sigs, k, check_time, good_keys);
status = ldns_verify_time(rrset, sigs, k, check_time, good_keys);
ldns_rr_list_deep_free(rrset);
return status;
}

ldns_status
Expand Down
4 changes: 2 additions & 2 deletions dnssec_sign.c
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,6 @@ ldns_sign_public(ldns_rr_list *rrset, ldns_key_list *keys)

new_owner = NULL;

signatures = ldns_rr_list_new();

/* prepare a signature and add all the know data
* prepare the rrset. Sign this together. */
rrset_clone = ldns_rr_list_clone(rrset);
Expand All @@ -257,6 +255,8 @@ ldns_sign_public(ldns_rr_list *rrset, ldns_key_list *keys)
/* sort */
ldns_rr_list_sort(rrset_clone);

signatures = ldns_rr_list_new();

for (key_count = 0;
key_count < ldns_key_list_key_count(keys);
key_count++) {
Expand Down
18 changes: 6 additions & 12 deletions dnssec_verify.c
Original file line number Diff line number Diff line change
Expand Up @@ -1586,8 +1586,6 @@ ldns_dnssec_verify_denial_nsec3_match( ldns_rr *rr
bool wildcard_covered = false;
ldns_rdf *zone_name;
ldns_rdf *hashed_name;
/* self assignment to suppress uninitialized warning */
ldns_rdf *next_closer = next_closer;
ldns_rdf *hashed_next_closer;
size_t i;
ldns_status result = LDNS_STATUS_DNSSEC_NSEC_RR_NOT_COVERED;
Expand Down Expand Up @@ -1662,6 +1660,7 @@ ldns_dnssec_verify_denial_nsec3_match( ldns_rr *rr
}
}
}
ldns_rdf_deep_free(hashed_name);
result = LDNS_STATUS_DNSSEC_NSEC_RR_NOT_COVERED;
/* wildcard no data? section 8.7 */
closest_encloser = ldns_dnssec_nsec3_closest_encloser(
Expand Down Expand Up @@ -1751,7 +1750,9 @@ ldns_dnssec_verify_denial_nsec3_match( ldns_rr *rr
/* Query name *is* the "next closer". */
hashed_next_closer = hashed_name;
} else {

ldns_rdf *next_closer;

ldns_rdf_deep_free(hashed_name);
/* "next closer" has less labels than the query name.
* Create the name and hash it.
*/
Expand All @@ -1765,6 +1766,7 @@ ldns_dnssec_verify_denial_nsec3_match( ldns_rr *rr
next_closer
);
(void) ldns_dname_cat(hashed_next_closer, zone_name);
ldns_rdf_deep_free(next_closer);
}
/* Find the NSEC3 that covers the "next closer" */
for (i = 0; i < ldns_rr_list_rr_count(nsecs); i++) {
Expand All @@ -1779,15 +1781,7 @@ ldns_dnssec_verify_denial_nsec3_match( ldns_rr *rr
break;
}
}
if (ldns_dname_label_count(closest_encloser) + 1
< ldns_dname_label_count(ldns_rr_owner(rr))) {

/* "next closer" has less labels than the query name.
* Dispose of the temporary variables that held that name.
*/
ldns_rdf_deep_free(hashed_next_closer);
ldns_rdf_deep_free(next_closer);
}
ldns_rdf_deep_free(hashed_next_closer);
ldns_rdf_deep_free(closest_encloser);
}

Expand Down
23 changes: 17 additions & 6 deletions dnssec_zone.c
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,7 @@ ldns_dnssec_zone_new_frm_fp_l(ldns_dnssec_zone** z, FILE* fp, const ldns_rdf* or
newzone = NULL;
} else {
ldns_dnssec_zone_free(newzone);
newzone = NULL;
}

error:
Expand Down Expand Up @@ -1105,37 +1106,47 @@ ldns_dnssec_zone_add_empty_nonterminals_nsec3(
ldns_rdf *ent_name;

if (!(ent_name = ldns_dname_clone_from(
next_name, i)))
next_name, i))) {

ldns_rdf_deep_free(l1);
ldns_rdf_deep_free(l2);
return LDNS_STATUS_MEM_ERR;
}

if (nsec3s && zone->_nsec3params) {
ldns_rdf *ent_hashed_name;

if (!(ent_hashed_name =
ldns_nsec3_hash_name_frm_nsec3(
zone->_nsec3params,
ent_name)))
ent_name))) {
ldns_rdf_deep_free(l1);
ldns_rdf_deep_free(l2);
ldns_rdf_deep_free(ent_name);
return LDNS_STATUS_MEM_ERR;
}
node = ldns_rbtree_search(nsec3s,
ent_hashed_name);
if (!node) {
ldns_rdf_deep_free(l1);
ldns_rdf_deep_free(l2);
ldns_rdf_deep_free(ent_name);
continue;
}
}
new_name = ldns_dnssec_name_new();
if (!new_name) {
ldns_rdf_deep_free(l1);
ldns_rdf_deep_free(l2);
ldns_rdf_deep_free(ent_name);
return LDNS_STATUS_MEM_ERR;
}
new_name->name = ent_name;
if (!new_name->name) {
ldns_dnssec_name_free(new_name);
return LDNS_STATUS_MEM_ERR;
}
new_name->name_alloced = true;
new_node = LDNS_MALLOC(ldns_rbnode_t);
if (!new_node) {
ldns_rdf_deep_free(l1);
ldns_rdf_deep_free(l2);
ldns_dnssec_name_free(new_name);
return LDNS_STATUS_MEM_ERR;
}
Expand Down
26 changes: 14 additions & 12 deletions host2str.c
Original file line number Diff line number Diff line change
Expand Up @@ -1102,12 +1102,12 @@ ldns_rdf2buffer_str_ipseckey(ldns_buffer *output, const ldns_rdf *rdf)
/* no gateway */
break;
case 1:
gateway_data = LDNS_XMALLOC(uint8_t, LDNS_IP4ADDRLEN);
if(!gateway_data)
return LDNS_STATUS_MEM_ERR;
if (ldns_rdf_size(rdf) < offset + LDNS_IP4ADDRLEN) {
return LDNS_STATUS_ERR;
}
gateway_data = LDNS_XMALLOC(uint8_t, LDNS_IP4ADDRLEN);
if(!gateway_data)
return LDNS_STATUS_MEM_ERR;
memcpy(gateway_data, &data[offset], LDNS_IP4ADDRLEN);
gateway = ldns_rdf_new(LDNS_RDF_TYPE_A,
LDNS_IP4ADDRLEN , gateway_data);
Expand All @@ -1118,12 +1118,12 @@ ldns_rdf2buffer_str_ipseckey(ldns_buffer *output, const ldns_rdf *rdf)
}
break;
case 2:
gateway_data = LDNS_XMALLOC(uint8_t, LDNS_IP6ADDRLEN);
if(!gateway_data)
return LDNS_STATUS_MEM_ERR;
if (ldns_rdf_size(rdf) < offset + LDNS_IP6ADDRLEN) {
return LDNS_STATUS_ERR;
}
gateway_data = LDNS_XMALLOC(uint8_t, LDNS_IP6ADDRLEN);
if(!gateway_data)
return LDNS_STATUS_MEM_ERR;
memcpy(gateway_data, &data[offset], LDNS_IP6ADDRLEN);
offset += LDNS_IP6ADDRLEN;
gateway =
Expand All @@ -1146,6 +1146,7 @@ ldns_rdf2buffer_str_ipseckey(ldns_buffer *output, const ldns_rdf *rdf)
}

if (ldns_rdf_size(rdf) <= offset) {
ldns_rdf_deep_free(gateway);
return LDNS_STATUS_ERR;
}
public_key_size = ldns_rdf_size(rdf) - offset;
Expand Down Expand Up @@ -1329,12 +1330,12 @@ ldns_rdf2buffer_str_amtrelay(ldns_buffer *output, const ldns_rdf *rdf)
/* no relay */
break;
case 1:
relay_data = LDNS_XMALLOC(uint8_t, LDNS_IP4ADDRLEN);
if(!relay_data)
return LDNS_STATUS_MEM_ERR;
if (ldns_rdf_size(rdf) < offset + LDNS_IP4ADDRLEN) {
return LDNS_STATUS_ERR;
}
relay_data = LDNS_XMALLOC(uint8_t, LDNS_IP4ADDRLEN);
if(!relay_data)
return LDNS_STATUS_MEM_ERR;
memcpy(relay_data, &data[offset], LDNS_IP4ADDRLEN);
relay = ldns_rdf_new(LDNS_RDF_TYPE_A,
LDNS_IP4ADDRLEN , relay_data);
Expand All @@ -1345,12 +1346,12 @@ ldns_rdf2buffer_str_amtrelay(ldns_buffer *output, const ldns_rdf *rdf)
}
break;
case 2:
relay_data = LDNS_XMALLOC(uint8_t, LDNS_IP6ADDRLEN);
if(!relay_data)
return LDNS_STATUS_MEM_ERR;
if (ldns_rdf_size(rdf) < offset + LDNS_IP6ADDRLEN) {
return LDNS_STATUS_ERR;
}
relay_data = LDNS_XMALLOC(uint8_t, LDNS_IP6ADDRLEN);
if(!relay_data)
return LDNS_STATUS_MEM_ERR;
memcpy(relay_data, &data[offset], LDNS_IP6ADDRLEN);
offset += LDNS_IP6ADDRLEN;
relay =
Expand All @@ -1373,6 +1374,7 @@ ldns_rdf2buffer_str_amtrelay(ldns_buffer *output, const ldns_rdf *rdf)
}

if (ldns_rdf_size(rdf) != offset) {
ldns_rdf_deep_free(relay);
return LDNS_STATUS_ERR;
}
ldns_buffer_printf(output, "%u %u %u ",
Expand Down
22 changes: 16 additions & 6 deletions host2wire.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,27 @@ ldns_dname2buffer_wire_compress(ldns_buffer *buffer, const ldns_rdf *name, ldns_
{
/* Not found. Write cache entry, take off first label, write it, */
/* try again with the rest of the name. */
node = LDNS_MALLOC(ldns_rbnode_t);
if(!node)
{
return LDNS_STATUS_MEM_ERR;
}
if (ldns_buffer_position(buffer) < 16384) {
node->key = ldns_rdf_clone(name);
ldns_rdf *key;

node = LDNS_MALLOC(ldns_rbnode_t);
if(!node)
{
return LDNS_STATUS_MEM_ERR;
}

key = ldns_rdf_clone(name);
if (!key) {
LDNS_FREE(node);
return LDNS_STATUS_MEM_ERR;
}
node->key = key;
node->data = (void *) (intptr_t) ldns_buffer_position(buffer);
if(!ldns_rbtree_insert(compression_data,node))
{
/* fprintf(stderr,"Name not found but now it's there?\n"); */
ldns_rdf_deep_free(key);
LDNS_FREE(node);
}
}
label = ldns_dname_label(name, 0);
Expand Down
12 changes: 10 additions & 2 deletions net.c
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ ldns_tcp_connect_from(const struct sockaddr_storage *to, socklen_t tolen,
}
#endif
if (from && bind(sockfd, (const struct sockaddr*)from, fromlen) == SOCK_INVALID){
close_socket(sockfd);
return 0;
}

Expand Down Expand Up @@ -337,7 +338,7 @@ ldns_tcp_send_from(uint8_t **result, ldns_buffer *qbin,
answer = ldns_tcp_read_wire_timeout(sockfd, answer_size, timeout);
close_socket(sockfd);

if (*answer_size == 0) {
if (!answer) {
/* oops */
return LDNS_STATUS_NETWORK_ERR;
}
Expand Down Expand Up @@ -390,6 +391,7 @@ ldns_udp_bgsend_from(ldns_buffer *qbin,
}

if (from && bind(sockfd, (const struct sockaddr*)from, fromlen) == -1){
close_socket(sockfd);
return 0;
}

Expand Down Expand Up @@ -437,7 +439,7 @@ ldns_udp_send_from(uint8_t **result, ldns_buffer *qbin,
answer = ldns_udp_read_wire(sockfd, answer_size, NULL, NULL);
close_socket(sockfd);

if (*answer_size == 0) {
if (!answer) {
/* oops */
return LDNS_STATUS_NETWORK_ERR;
}
Expand Down Expand Up @@ -571,6 +573,9 @@ ldns_send_buffer(ldns_pkt **result, ldns_resolver *r, ldns_buffer *qb, ldns_rdf
if (!reply_bytes) {
/* the current nameserver seems to have a problem, blacklist it */
if (ldns_resolver_fail(r)) {
if(src) {
LDNS_FREE(src);
}
LDNS_FREE(ns);
return LDNS_STATUS_ERR;
} else {
Expand Down Expand Up @@ -915,6 +920,9 @@ ldns_axfr_start(ldns_resolver *resolver, const ldns_rdf *domain, ldns_rr_class c
src, (socklen_t)src_len,
ldns_resolver_timeout(resolver));
}
if (src) {
LDNS_FREE(src);
}

if (resolver->_socket == SOCK_INVALID) {
ldns_pkt_free(query);
Expand Down
2 changes: 2 additions & 0 deletions packet.c
Original file line number Diff line number Diff line change
Expand Up @@ -943,11 +943,13 @@ ldns_pkt_query_new_frm_str_internal(ldns_pkt **p, const char *name,
}

if (!ldns_pkt_set_flags(packet, flags)) {
ldns_pkt_free(packet);
return LDNS_STATUS_ERR;
}

question_rr = ldns_rr_new();
if (!question_rr) {
ldns_pkt_free(packet);
return LDNS_STATUS_MEM_ERR;
}

Expand Down
5 changes: 4 additions & 1 deletion radix.c
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,9 @@ ldns_radix_insert(ldns_radix_t* tree, uint8_t* key, radix_strlen_t len,
}
} else if (pos == len) {
/** Exact match found */
LDNS_FREE(add);
if (prefix->data) {
/* Element already exists */
LDNS_FREE(add);
return LDNS_STATUS_EXISTS_ERR;
}
prefix->data = data;
Expand Down Expand Up @@ -1120,12 +1120,15 @@ ldns_radix_array_split(ldns_radix_array_t* array, uint8_t* key,
if (array->len - common_len > 1) {
if (!ldns_radix_prefix_remainder(common_len+1,
array->str, array->len, &s1, &l1)) {
LDNS_FREE(common);
return 0;
}
}
if (strlen_to_add - common_len > 1) {
if (!ldns_radix_prefix_remainder(common_len+1,
str_to_add, strlen_to_add, &s2, &l2)) {
LDNS_FREE(common);
LDNS_FREE(s1);
return 0;
}
}
Expand Down
2 changes: 2 additions & 0 deletions str2host.c
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ ldns_str2rdf_str(ldns_rdf **rd, const char *str)
*++dp = ch;
}
if (! str) {
LDNS_FREE(data);
return LDNS_STATUS_SYNTAX_BAD_ESCAPE;
}
length = (size_t)(dp - data);
Expand Down Expand Up @@ -1542,6 +1543,7 @@ ldns_str2rdf_long_str(ldns_rdf **rd, const char *str)
}
}
if (! str) {
LDNS_FREE(data);
return LDNS_STATUS_SYNTAX_BAD_ESCAPE;
}
if (!(length = (size_t)(dp - data))) {
Expand Down

0 comments on commit db06eb4

Please sign in to comment.