Final code review thingies:

Bufferoverflow in data_buffer2wire in ldns-testpkts.c
Print unknown rcodes in ldns_axfr_next in resolver.c
Handle errors in main in ldns-keyfetcher.c
Continue in correct loop in ldns_resolver_new_frm_fp_l in resolver.c
Skip set delimeters (to del) when tokenread i.s.o. delim in ldns_bget_token and ldns_fget_token in parse.c
Test reply when verifying tsig in ldns_send_buffer in net.c
Assert that verify_next_hashed_name is only called with nsecs in the zone with ldns-verify-zone.c
Set inception and expiration on keys after they are read from the engine in main in ldns-signzone.c
Gracefully return from ldns_dnssec_zone_add_empty_nonterminals in a broken rbtree.
Check if tree->rr is null before use in ldns_dnssec_trust_tree_contains_keys in dnssec_verify.c
Check data for null before use in ldns_dnssec_create_nsec_bitmap in dnssec.c
Dead code in ldns_str2rdf_wks, ldns_resolver_query, examples/ldns_testpkts/data_buffer2wire, drill/work/packetbuffromfile & ldns_dnssec_verify_denial_nsec3_match.

diff --git a/dnssec.c b/dnssec.c
index 5469fb186d91239fc0206817f46a032f52a0396b..37fd6e97d6e3c3951634d94acc794964de5d47df 100644 (file)
--- a/dnssec.c
+++ b/dnssec.c
@@ -743,7 +743,10 @@ ldns_dnssec_create_nsec_bitmap(ldns_rr_type rr_type_list[],
                memcpy(data + cur_data_size + 2, cur_data, cur_window_max+1);
                cur_data_size += cur_window_max + 3;
        }
-
+       if (! data) {
+               LDNS_FREE(bitmap);
+               return NULL;
+       }
        bitmap_rdf = ldns_rdf_new_frm_data(LDNS_RDF_TYPE_NSEC,
                                                                cur_data_size,
                                                                data);

diff --git a/dnssec_verify.c b/dnssec_verify.c
index a8844300126d66e8a4be5984568f06b801e9d6f4..d435eedf6afd168ba257b52824504f29efac2be3 100644 (file)
--- a/dnssec_verify.c
+++ b/dnssec_verify.c
@@ -1078,7 +1078,8 @@ ldns_dnssec_trust_tree_contains_keys(ldns_dnssec_trust_tree *tree,
                                        if (tree->parent_status[i] != LDNS_STATUS_OK) {
                                                result = tree->parent_status[i];
                                        } else {
-                                               if (ldns_rr_get_type(tree->rr)
+                                               if (tree->rr &&
+                                                   ldns_rr_get_type(tree->rr)
                                                    == LDNS_RR_TYPE_NSEC &&
                                                    parent_result == LDNS_STATUS_OK
                                                    ) {
@@ -1613,7 +1614,7 @@ ldns_dnssec_verify_denial_nsec3_match( ldns_rr *rr
                                                   ldns_rr_get_type(rr),
                                                   nsecs);
                 if(!closest_encloser) {
-                        result = LDNS_STATUS_NSEC3_ERR;
+                        result = LDNS_STATUS_DNSSEC_NSEC_RR_NOT_COVERED;
                         goto done;
                 }
 
@@ -1637,16 +1638,14 @@ ldns_dnssec_verify_denial_nsec3_match( ldns_rr *rr
                        ldns_rdf_deep_free(hashed_wildcard_name);
                }
 
-               ldns_rdf_deep_free(closest_encloser);
-               ldns_rdf_deep_free(wildcard);
-
-               if (!wildcard_covered) {
+               if (! wildcard_covered) {
                        result = LDNS_STATUS_DNSSEC_NSEC_WILDCARD_NOT_COVERED;
-               } else if (closest_encloser && wildcard_covered) {
-                       result = LDNS_STATUS_OK;
                } else {
-                       result = LDNS_STATUS_DNSSEC_NSEC_RR_NOT_COVERED;
+                       result = LDNS_STATUS_OK;
                }
+               ldns_rdf_deep_free(closest_encloser);
+               ldns_rdf_deep_free(wildcard);
+
        } else if (packet_nodata && packet_qtype != LDNS_RR_TYPE_DS) {
                /* section 8.5 */
                hashed_name = ldns_nsec3_hash_name_frm_nsec3(

diff --git a/dnssec_zone.c b/dnssec_zone.c
index 464a19434a4f425a2590480e8ca51fb826616025..df71a23c7edeed176ed3e1087c29d79242961b09 100644 (file)
--- a/dnssec_zone.c
+++ b/dnssec_zone.c
@@ -1012,7 +1012,9 @@ ldns_dnssec_zone_add_empty_nonterminals(ldns_dnssec_zone *zone)
                if (next_node == LDNS_RBTREE_NULL) {
                        next_node = ldns_rbtree_first(zone->names);
                }
-
+               if (! cur_node->data || ! next_node->data) {
+                       return LDNS_STATUS_ERR;
+               }
                cur_name = ((ldns_dnssec_name *)cur_node->data)->name;
                next_name = ((ldns_dnssec_name *)next_node->data)->name;
                cur_label_count = ldns_dname_label_count(cur_name);

diff --git a/drill/drill_util.c b/drill/drill_util.c
index 4efb1f005ed4d78ca150dba8158cbbe78ecd03fa..950cffbc08a05336ac386ce1639ccbcb0cff553f 100644 (file)
--- a/drill/drill_util.c
+++ b/drill/drill_util.c
@@ -20,7 +20,7 @@ read_line(FILE *input, char *line, size_t len)
        
        char c;
        for (i = 0; i < len-1; i++) {
-               c = getc(input);
+               c = (int)getc(input);
                if (c == EOF) {
                        return -1;
                } else if (c != '\n') {

diff --git a/drill/work.c b/drill/work.c
index 150cd7ed037d4089046d20b5eeec035e842f8b69..339907b0b72cb734deb48dc00caa26fee346a90f 100644 (file)
--- a/drill/work.c
+++ b/drill/work.c
@@ -122,11 +122,6 @@ packetbuffromfile(char *filename, uint8_t *wire)
                                hexbuf[hexbufpos] = (uint8_t) c;
                                hexbufpos++;
                                break;
-                       default:
-                               warning("unknown state while reading %s", filename);
-                               xfree(hexbuf);
-                               return 0;
-                               break;
                }
                c = fgetc(fp);
        }

diff --git a/examples/ldns-keyfetcher.c b/examples/ldns-keyfetcher.c
index 267822f8868f949a25e46ac17ed02063d1ee2b1b..ee06aea9f252c7ffc172c83c54a0f945f068a820 100644 (file)
--- a/examples/ldns-keyfetcher.c
+++ b/examples/ldns-keyfetcher.c
@@ -649,10 +649,21 @@ main(int argc, char *argv[])
                fprintf(stderr, "Warning: Unable to create stub resolver from /etc/resolv.conf:\n");
                fprintf(stderr, "%s\n", ldns_get_errorstr_by_id(status));
                fprintf(stderr, "defaulting to nameserver at 127.0.0.1 for separate nameserver name lookups\n");
-               res = ldns_resolver_new();
-               ns = ldns_rdf_new_frm_str(LDNS_RDF_TYPE_A, "127.0.0.1");
-               status = ldns_resolver_push_nameserver(res, ns);
-               if (status != LDNS_STATUS_OK) {
+               for (;;) {
+                       res = ldns_resolver_new();
+                       if (res) {
+                               ns = ldns_rdf_new_frm_str(LDNS_RDF_TYPE_A,
+                                               "127.0.0.1");
+                               if (ns) {
+                                       status = ldns_resolver_push_nameserver(
+                                                       res, ns);
+                                       if (status == LDNS_STATUS_OK) {
+                                               break;
+                                       }
+                                       ldns_rdf_deep_free(ns);
+                               }
+                               ldns_resolver_free(res);
+                       }
                        fprintf(stderr, "Unable to create stub resolver: %s\n", ldns_get_errorstr_by_id(status));
                        exit(EXIT_FAILURE);
                }

diff --git a/examples/ldns-read-zone.c b/examples/ldns-read-zone.c
index 6e00a3fc47bcb0ff1281b52111afdd9229c1022f..efe187e6a223cc97e9a6d361fec845c37d47a9e2 100644 (file)
--- a/examples/ldns-read-zone.c
+++ b/examples/ldns-read-zone.c
@@ -60,7 +60,7 @@ main(int argc, char **argv)
                                }
                                break;
                        case 'h':
-                               printf("Usage: %s [-c] [-v] [-z] <zonefile>\n", argv[0]);
+                               printf("Usage: %s [OPTIONS] <zonefile>\n", argv[0]);
                                printf("\tReads the zonefile and prints it.\n");
                                printf("\tThe RR count of the zone is printed to stderr.\n");
                                printf("\t-b include bubblebabble of DS's.\n");

diff --git a/examples/ldns-signzone.c b/examples/ldns-signzone.c
index 3461503963576b3fbac0a0bee112fa64da8888df..25ece3a651e9a2cf4506d084210526eff2122be9 100644 (file)
--- a/examples/ldns-signzone.c
+++ b/examples/ldns-signzone.c
@@ -513,13 +513,6 @@ main(int argc, char *argv[])
 
                                printf("Engine key id: %s, algo %d\n", eng_key_id, eng_key_algo);
 
-                               if (expiration != 0) {
-                                       ldns_key_set_expiration(key, expiration);
-                               }
-                               if (inception != 0) {
-                                       ldns_key_set_inception(key, inception);
-                               }
-
                                s = ldns_key_new_frm_engine(&key, engine, eng_key_id, eng_key_algo);
                                if (s == LDNS_STATUS_OK) {
                                        /* must be dnssec key */
@@ -544,6 +537,14 @@ main(int argc, char *argv[])
                                                fprintf(stderr, "Warning, key not suitable for signing, ignoring key with algorithm %u\n", ldns_key_algorithm(key));
                                                break;
                                        }
+                                       if (expiration != 0) {
+                                               ldns_key_set_expiration(key,
+                                                               expiration);
+                                       }
+                                       if (inception != 0) {
+                                               ldns_key_set_inception(key,
+                                                               inception);
+                                       }
                                } else {
                                        printf("Error reading key '%s' from engine: %s\n", eng_key_id, ldns_get_errorstr_by_id(s));
                                        #ifdef HAVE_SSL

diff --git a/examples/ldns-testpkts.c b/examples/ldns-testpkts.c
index 033ea15d70a48a4c92a4488aba583ddcae8f4e26..7a6c020e8698ca4b2772a7c1c6ee0633f6c95bf3 100644 (file)
--- a/examples/ldns-testpkts.c
+++ b/examples/ldns-testpkts.c
@@ -340,6 +340,12 @@ data_buffer2wire(ldns_buffer *data_buffer)
                                        (c >= 'a' && c <= 'f') ||
                                        (c >= 'A' && c <= 'F') )
                                {
+                                       if (hexbufpos >= LDNS_MAX_PACKETLEN) {
+                                               error("buffer overflow");
+                                               LDNS_FREE(hexbuf);
+                                               return 0;
+
+                                       }
                                        hexbuf[hexbufpos] = (uint8_t) c;
                                        hexbufpos++;
                                } else if (c == ';') {
@@ -354,14 +360,14 @@ data_buffer2wire(ldns_buffer *data_buffer)
                                }
                                break;
                        case 2:
+                               if (hexbufpos >= LDNS_MAX_PACKETLEN) {
+                                       error("buffer overflow");
+                                       LDNS_FREE(hexbuf);
+                                       return 0;
+                               }
                                hexbuf[hexbufpos] = (uint8_t) c;
                                hexbufpos++;
                                break;
-                       default:
-                               error("unknown state while reading");
-                               LDNS_FREE(hexbuf);
-                               return 0;
-                               break;
                }
        }
 
@@ -371,6 +377,11 @@ data_buffer2wire(ldns_buffer *data_buffer)
        
        /* lenient mode: length must be multiple of 2 */
        if (hexbufpos % 2 != 0) {
+               if (hexbufpos >= LDNS_MAX_PACKETLEN) {
+                       error("buffer overflow");
+                       LDNS_FREE(hexbuf);
+                       return 0;
+               }
                hexbuf[hexbufpos] = (uint8_t) '0';
                hexbufpos++;
        }

diff --git a/examples/ldns-verify-zone.c b/examples/ldns-verify-zone.c
index 7829c43d0354ad82597c77f7b1aff4d62cb0d232..373d47010f30cb12b12e788217d4a4305e79d077 100644 (file)
--- a/examples/ldns-verify-zone.c
+++ b/examples/ldns-verify-zone.c
@@ -309,6 +309,9 @@ verify_next_hashed_name(ldns_dnssec_zone* zone, ldns_dnssec_name *name)
        if (!cur_next_name) {
                cur_next_name = cur_first_name;
        }
+       assert(cur_next_name); /* Because this function is called on nsec
+                               * occurrence, it must be there!
+                               */
 
        next_owner_str = ldns_rdf2str(ldns_nsec3_next_owner(name->nsec));
        next_owner_dname = ldns_dname_new_frm_str(next_owner_str);

diff --git a/net.c b/net.c
index 6241d2d15510a1c44fc9686197eb77785f8add63..6b444da677bc1d85a054562d91975a3205b86d29 100644 (file)
--- a/net.c
+++ b/net.c
@@ -206,7 +206,7 @@ ldns_send_buffer(ldns_pkt **result, ldns_resolver *r, ldns_buffer *qb, ldns_rdf
                return LDNS_STATUS_RES_NO_NS;
        }
 #ifdef HAVE_SSL
-       if (tsig_mac && reply_bytes) {
+       if (tsig_mac && reply && reply_bytes) {
                if (!ldns_pkt_tsig_verify(reply,
                                          reply_bytes,
                                          reply_size,

diff --git a/parse.c b/parse.c
index ac9bdbdd556461cab23f73c7c818e974d1251c6c..ea5ffad026fdc73cfe208cb5c98998b87690b86c 100644 (file)
--- a/parse.c
+++ b/parse.c
@@ -161,7 +161,7 @@ ldns_fget_token_l(FILE *f, char *token, const char *delim, size_t limit, int *li
        return (ssize_t)i;
 
 tokenread:
-       ldns_fskipcs_l(f, delim, line_nr);
+       ldns_fskipcs_l(f, del, line_nr);
        *t = '\0';
        if (p != 0) {
                return -1;
@@ -331,7 +331,7 @@ ldns_bget_token(ldns_buffer *b, char *token, const char *delim, size_t limit)
        return (ssize_t)i;
 
 tokenread:
-       ldns_bskipcs(b, delim);
+       ldns_bskipcs(b, del);
        *t = '\0';
 
        if (p != 0) {

diff --git a/resolver.c b/resolver.c
index 086814530eb4260b0b57d85c691f5d927283a103..2cee9fff194a1320c3ea616fe0fe51e1f18575ef 100644 (file)
--- a/resolver.c
+++ b/resolver.c
@@ -809,8 +809,7 @@ ldns_resolver_new_frm_fp_l(ldns_resolver **res, FILE *fp, int *line_nr)
                                        gtr -= bgtr;
                                         if(word[0] == '#') {
                                                 expect = LDNS_RESOLV_KEYWORD;
-                                               ldns_buffer_free(b);
-                                                continue;
+                                                break;
                                         }
                                        tmp = ldns_rdf_new_frm_str(LDNS_RDF_TYPE_DNAME, word);
                                        if (!tmp) {
@@ -826,8 +825,10 @@ ldns_resolver_new_frm_fp_l(ldns_resolver **res, FILE *fp, int *line_nr)
                                            (size_t) gtr + 1);
                                }
                                ldns_buffer_free(b);
-                               gtr = 1;
-                               expect = LDNS_RESOLV_KEYWORD;
+                               if (expect != LDNS_RESOLV_KEYWORD) {
+                                       gtr = 1;
+                                       expect = LDNS_RESOLV_KEYWORD;
+                               }
                                break;
                        case LDNS_RESOLV_SORTLIST:
                                gtr = ldns_fget_token_l(fp, word, LDNS_PARSE_SKIP_SPACE, 0, line_nr);
@@ -1024,9 +1025,6 @@ ldns_resolver_query(const ldns_resolver *r, const ldns_rdf *name,
 
        newname = ldns_dname_cat_clone((const ldns_rdf*)name, ldns_resolver_domain(r));
        if (!newname) {
-               if (pkt) {
-                       ldns_pkt_free(pkt);
-               }
                return NULL;
        }
 
@@ -1303,7 +1301,14 @@ ldns_axfr_next(ldns_resolver *resolver)
                        return NULL;
                } else if (ldns_pkt_get_rcode(resolver->_cur_axfr_pkt) != 0) {
                        rcode = ldns_lookup_by_id(ldns_rcodes, (int) ldns_pkt_get_rcode(resolver->_cur_axfr_pkt));
-                       fprintf(stderr, "Error in AXFR: %s\n", rcode->name);
+                       if (rcode) {
+                               fprintf(stderr, "Error in AXFR: %s\n", 
+                                               rcode->name);
+                       } else {
+                               fprintf(stderr, "Error in AXFR: %d\n", 
+                                               (int) ldns_pkt_get_rcode(
+                                               resolver->_cur_axfr_pkt));
+                       }
 
                        /* RoRi: we must now also close the socket, otherwise subsequent uses of the
                           same resolver structure will fail because the link is still open or

diff --git a/str2host.c b/str2host.c
index b5e4be10df769148a01f6891c7430bf979f0a8ac..51357cc3176cdad15bae345ade5fb4588ae5ad4f 100644 (file)
--- a/str2host.c
+++ b/str2host.c
@@ -1105,8 +1105,6 @@ ldns_str2rdf_wks(ldns_rdf **rd, const char *str)
                data[0] = (uint8_t) proto->p_proto;
        } else if (proto_str) {
                data[0] = (uint8_t) atoi(proto_str);
-       } else {
-               data[0] = 0;
        }
        memcpy(data + 1, bitmap, (size_t) bm_len);