[master] Corrects medium impact issues reported by Coverity.

    Merges in rt36933

diff --git a/RELNOTES b/RELNOTES
index d39629a1170881cf3030ff2149030ca67e683f37..19a660a31db1b58e19d6be2dcebf499a1d067b2c 100644 (file)
--- a/RELNOTES
+++ b/RELNOTES
@@ -54,6 +54,9 @@ by Eric Young (eay@cryptsoft.com).
 
                        Changes since 4.3.1
 
+- Addressed Coverity issues reported as of 07-31-2014:
+  [ISC-Bugs #36933] Corrects Coverity reported "medium" impact issues
+
 - Addressed Coverity issues reported as of 07-31-2014:
   [ISC-Bugs #36712] Corrects Coverity reported "high" impact issues
 

diff --git a/common/options.c b/common/options.c
index 374cb573954fc30f323e1ebb0f5143d2d4ca4a13..dc96d8275d97321418c312e57530cc4659e7facd 100644 (file)
--- a/common/options.c
+++ b/common/options.c
@@ -1849,6 +1849,15 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes)
                         * of the last format type and we add 1 to
                         * cover the entire first record.
                         */
+
+                       /* If format string had no valid entries prior to
+                        * 'a' hunkinc will be 0. Ex: "a", "oa", "aA" */
+                       if (hunkinc == 0) {
+                               log_error ("%s: invalid 'a' format: %s",
+                                          option->name, option->format);
+                               return ("<error>");
+                       }
+
                        numhunk = ((len - hunksize) / hunkinc) + 1;
                        len_used = hunksize + ((numhunk - 1) * hunkinc);
                } else {
@@ -1856,6 +1865,15 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes)
                         * It is an 'A' type array - we repeat the
                         * entire record
                         */
+
+                       /* If format string had no valid entries prior to
+                        * 'A' hunksize will be 0. Ex: "A", "oA", "foA" */
+                       if (hunksize == 0) {
+                               log_error ("%s: invalid 'A' format: %s",
+                                          option->name, option->format);
+                               return ("<error>");
+                       }
+
                        numhunk = len / hunksize;
                        len_used = numhunk * hunksize;
                }

diff --git a/common/parse.c b/common/parse.c
index 2e363c2cc43d41d9f7b7bd33dffe4e334cd79a9f..65e0b31457aaea985d2d1926ba66f94ec4741e0a 100644 (file)
--- a/common/parse.c
+++ b/common/parse.c
@@ -4732,14 +4732,6 @@ int parse_expression (expr, cfile, lose, context, plhs, binop)
        tmp = (struct expression *)0;
        rhs = (struct expression *)0;
 
-       /* Recursions don't return until we have parsed the end of the
-          expression, so if we recursed earlier, we can now return what
-          we got. */
-       if (next_op == expr_none) {
-               *expr = lhs;
-               return 1;
-       }
-
        binop = next_op;
        goto new_rhs;
 }      

diff --git a/dst/dst_api.c b/dst/dst_api.c
index f93ee810d0d7fdff0bc7dd0c652ce2654bcac725..30dc66f5e678a436911701103c87c262110a1d28 100644 (file)
--- a/dst/dst_api.c
+++ b/dst/dst_api.c
@@ -819,7 +819,7 @@ dst_key_to_buffer(DST_KEY *key, u_char *out_buff, unsigned buf_len)
   /* this function will extract the secret of HMAC into a buffer */
        if(key == NULL) 
                return (0);
-       if(key->dk_func != NULL && key->dk_func != NULL) {
+       if(key->dk_func != NULL && key->dk_func->to_dns_key != NULL) {
                len = key->dk_func->to_dns_key(key, out_buff, buf_len);
                if (len < 0)
                        return (0);

diff --git a/dst/prandom.c b/dst/prandom.c
index ea02a27518d3e98c731d92dd980c174e6191f02f..7a24d332cd56293d155bd021b88166df4a8f7697 100644 (file)
--- a/dst/prandom.c
+++ b/dst/prandom.c
@@ -492,8 +492,7 @@ digest_file(dst_work *work)
                if (i > 0) 
                        work->filled += i;
        }
-       else if (i > 0)
-               my_digest(work, buf, (unsigned)i);
+
        my_digest(work, (const u_char *)name, strlen(name));
        return (no + strlen(name));
 }

diff --git a/omapip/test.c b/omapip/test.c
index f05896dbe165ed9dcbdc283c43c6f051db6a4a6d..98437acd1bbcb6fe330af448de42a8af7806319e 100644 (file)
--- a/omapip/test.c
+++ b/omapip/test.c
@@ -53,7 +53,12 @@ int main (int argc, char **argv)
                exit(1);
        }
 
-       omapi_init ();
+       status = omapi_init ();
+       if (status != ISC_R_SUCCESS) {
+               fprintf(stderr, "omapi_init failed: %s\n",
+                       isc_result_totext(status));
+               exit(1);
+       }
 
        if (argc > 1 && !strcmp (argv [1], "listen")) {
                if (argc < 3) {

diff --git a/omapip/trace.c b/omapip/trace.c
index 23e4e50606ede8183802332a97c02ce1f966ec3a..f4115c143b1aad6febf52057e825cb829dab7d38 100644 (file)
--- a/omapip/trace.c
+++ b/omapip/trace.c
@@ -606,7 +606,9 @@ isc_result_t trace_get_next_packet (trace_type_t **ttp,
        paylen = tpkt -> length;
        if (paylen % 8)
                paylen += 8 - (tpkt -> length % 8);
-       if (paylen > (*bufmax)) {
+
+       /* allocate a buffer if we need one or current buffer is too small */
+       if ((*buf == NULL) || (paylen > (*bufmax))) {
                if ((*buf))
                        dfree ((*buf), MDL);
                (*bufmax) = ((paylen + 1023) & ~1023U);
@@ -617,7 +619,7 @@ isc_result_t trace_get_next_packet (trace_type_t **ttp,
                        return ISC_R_NOMEMORY;
                }
        }
-       
+
        status = fread ((*buf), 1, paylen, traceinfile);
        if (status < paylen) {
                if (ferror (traceinfile))

diff --git a/server/db.c b/server/db.c
index bc126065349d1567a2028fc086d4e7e7a24d095b..e1f1beb759f8c70bc0f1096e982b0ef798cf3b7a 100644 (file)
--- a/server/db.c
+++ b/server/db.c
@@ -1206,7 +1206,7 @@ int new_lease_file ()
       fail:
        lease_file_is_corrupt = db_validity;
       fdfail:
-       unlink (newfname);
+       (void)unlink (newfname);
        return 0;
 }
 

diff --git a/server/ddns.c b/server/ddns.c
index aba57e7fef15ab0da811e34f1e09b87e83e98098..6cbd3e3d23c38f9a684ee7ce5c1a80469f7181a0 100644 (file)
--- a/server/ddns.c
+++ b/server/ddns.c
@@ -236,10 +236,9 @@ ddns_updates(struct packet *packet, struct lease *lease, struct lease *old,
                        goto out;
                }
 
-               buffer_allocate (&ddns_fwd_name.buffer,
-                                ddns_hostname.len + ddns_domainname.len + 2,
-                                MDL);
-               if (ddns_fwd_name.buffer) {
+               if (buffer_allocate (&ddns_fwd_name.buffer,
+                                    ddns_hostname.len +
+                                    ddns_domainname.len + 2, MDL)) {
                        ddns_fwd_name.data = ddns_fwd_name.buffer->data;
                        data_string_append (&ddns_fwd_name, &ddns_hostname);
                        ddns_fwd_name.buffer->data[ddns_fwd_name.len] = '.';
@@ -438,8 +437,8 @@ ddns_updates(struct packet *packet, struct lease *lease, struct lease *old,
        }
 
        if (s1) {
-               buffer_allocate(&ddns_cb->rev_name.buffer, rev_name_len, MDL);
-               if (ddns_cb->rev_name.buffer != NULL) {
+               if (buffer_allocate(&ddns_cb->rev_name.buffer,
+                                   rev_name_len, MDL)) {
                        struct data_string *rname = &ddns_cb->rev_name;
                        rname->data = rname->buffer->data;
 

diff --git a/server/dhcp.c b/server/dhcp.c
index dbc2d5666c56db9dc75973e6bca11768c131bb72..a3d7255819d95532f0e138f14420f30ae8eb9b56 100644 (file)
--- a/server/dhcp.c
+++ b/server/dhcp.c
@@ -1081,7 +1081,7 @@ void dhcpinform (packet, ms_nulltp)
        if (subnet == NULL) {
                log_info("%s: unknown subnet for %s address %s",
                         msgbuf, addr_type, piaddr(sip));
-               option_state_dereference (&options, MDL);
+               option_state_dereference(&options, MDL);
                return;
        }
 
@@ -1089,48 +1089,47 @@ void dhcpinform (packet, ms_nulltp)
           It would be nice if a per-host value could override this, but
           there's overhead involved in checking this, so let's see how people
           react first. */
-       if (subnet && !subnet -> group -> authoritative) {
+       if (!subnet->group->authoritative) {
                static int eso = 0;
-               log_info ("%s: not authoritative for subnet %s",
+               log_info("%s: not authoritative for subnet %s",
                          msgbuf, piaddr (subnet -> net));
                if (!eso) {
-                       log_info ("If this DHCP server is authoritative for%s",
+                       log_info("If this DHCP server is authoritative for%s",
                                  " that subnet,");
-                       log_info ("please write an `authoritative;' directi%s",
+                       log_info("please write an `authoritative;' directi%s",
                                  "ve either in the");
-                       log_info ("subnet declaration or in some scope that%s",
+                       log_info("subnet declaration or in some scope that%s",
                                  " encloses the");
-                       log_info ("subnet declaration - for example, write %s",
+                       log_info("subnet declaration - for example, write %s",
                                  "it at the top");
-                       log_info ("of the dhcpd.conf file.");
+                       log_info("of the dhcpd.conf file.");
                }
                if (eso++ == 100)
                        eso = 0;
-               subnet_dereference (&subnet, MDL);
-               option_state_dereference (&options, MDL);
+               subnet_dereference(&subnet, MDL);
+               option_state_dereference(&options, MDL);
                return;
        }
        
-       memset (&outgoing, 0, sizeof outgoing);
-       memset (&raw, 0, sizeof raw);
+       memset(&outgoing, 0, sizeof outgoing);
+       memset(&raw, 0, sizeof raw);
        outgoing.raw = &raw;
 
        maybe_return_agent_options(packet, options);
 
        /* Execute statements in scope starting with the subnet scope. */
-       if (subnet)
-               execute_statements_in_scope (NULL, packet, NULL, NULL,
-                                            packet->options, options,
-                                            &global_scope, subnet->group,
-                                            NULL, NULL);
+       execute_statements_in_scope(NULL, packet, NULL, NULL,
+                                   packet->options, options,
+                                   &global_scope, subnet->group,
+                                   NULL, NULL);
                
        /* Execute statements in the class scopes. */
-       for (i = packet -> class_count; i > 0; i--) {
+       for (i = packet->class_count; i > 0; i--) {
                execute_statements_in_scope(NULL, packet, NULL, NULL,
                                            packet->options, options,
                                            &global_scope,
                                            packet->classes[i - 1]->group,
-                                           subnet ? subnet->group : NULL,
+                                           subnet->group,
                                            NULL);
        }
 

diff --git a/server/dhcpv6.c b/server/dhcpv6.c
index 18962389303a557bcc586f9b96209a59d05f7f64..138101de0226c24b0bd72ebd13b3b191cbd649ed 100644 (file)
--- a/server/dhcpv6.c
+++ b/server/dhcpv6.c
@@ -3488,6 +3488,8 @@ lease_compare(struct iasubopt *alpha, struct iasubopt *beta) {
                        if (alpha->hard_lifetime_end_time <
                            beta->hard_lifetime_end_time)
                                return alpha;
+                       else
+                               return beta;
 
                      default:
                        log_fatal("Impossible condition at %s:%d.", MDL);
@@ -4700,6 +4702,8 @@ prefix_compare(struct reply_state *reply,
                        if (alpha->hard_lifetime_end_time <
                            beta->hard_lifetime_end_time)
                                return alpha;
+                       else
+                               return beta;
 
                      default:
                        log_fatal("Impossible condition at %s:%d.", MDL);

diff --git a/server/failover.c b/server/failover.c
index a0bebc289713af11b52c5c9476b057efb5ec8967..19c3e087285217f8bb8f3e74bb1780bfd034aca7 100644 (file)
--- a/server/failover.c
+++ b/server/failover.c
@@ -720,7 +720,10 @@ static isc_result_t do_a_failover_option (c, link)
 
                /* FT_DDNS* are special - one or two bytes of status
                   followed by the client FQDN. */
-               if (ft_options [option_code].type == FT_DDNS1 ||
+       
+               /* Note: FT_DDNS* option support appears to be incomplete.
+                  ISC-Bugs #36996 has been opened to address this. */
+               if (ft_options [option_code].type == FT_DDNS ||
                    ft_options [option_code].type == FT_DDNS1) {
                        ddns_fqdn_t *ddns =
                                ((ddns_fqdn_t *)
@@ -2285,6 +2288,8 @@ isc_result_t dhcp_failover_peer_state_changed (dhcp_failover_state_t *state,
                switch (new_state) {
                      case recover_done:
                        log_error("Both servers have entered recover-done!");
+                       /* Fall through and tranistion to normal anyway */
+
                      case normal:
                        dhcp_failover_set_state (state, normal);
                        break;

diff --git a/server/omapi.c b/server/omapi.c
index 081787171c83b7a3d4e470696bf83fb5643f7acc..154f8327d4c2a2c25fcd0e9c0028667faf1af7b5 100644 (file)
--- a/server/omapi.c
+++ b/server/omapi.c
@@ -2190,7 +2190,9 @@ isc_result_t dhcp_class_create (omapi_object_t **lp,
        if (status != ISC_R_SUCCESS)
                return (status);
 
-       clone_group(&cp->group, root_group, MDL);
+       if (clone_group(&cp->group, root_group, MDL) == 0)
+               return (ISC_R_NOMEMORY);
+
        cp->flags = CLASS_DECL_DYNAMIC;
        status = omapi_object_reference(lp, (omapi_object_t *)cp, MDL);
        class_dereference(&cp, MDL);