[master] Corrects high impact issues reported by Coverity.

    Merges in rt36712

diff --git a/RELNOTES b/RELNOTES
index 1480cb83b684cb5a8317be1b57550120d79ea036..d39629a1170881cf3030ff2149030ca67e683f37 100644 (file)
--- a/RELNOTES
+++ b/RELNOTES
@@ -52,9 +52,10 @@ ISC DHCP is open source software maintained by Internet Systems
 Consortium.  This product includes cryptographic software written
 by Eric Young (eay@cryptsoft.com).
 
-                       Changes since 4.3.1rc1
+                       Changes since 4.3.1
 
-- None
+- Addressed Coverity issues reported as of 07-31-2014:
+  [ISC-Bugs #36712] Corrects Coverity reported "high" impact issues
 
                        Changes since 4.3.1b1
 

diff --git a/common/conflex.c b/common/conflex.c
index 2e708dabc65f4b4736628c800ddd10ec9c74a730..093ac036d5e475e6240ce2d4d6587fc1ba2cf3dc 100644 (file)
--- a/common/conflex.c
+++ b/common/conflex.c
@@ -464,7 +464,7 @@ read_whitespace(int c, struct parse *cfile) {
         */
        ofs = 0;
        do {
-               if (ofs >= sizeof(cfile->tokbuf)) {
+               if (ofs >= (sizeof(cfile->tokbuf) - 1)) {
                        /*
                         * As the file includes a huge amount of whitespace,
                         * it's probably broken.

diff --git a/common/discover.c b/common/discover.c
index aeb2fc50ba34d163859eff90a1389a3c64bd7c82..3cd64a75c1becd4430ed96ddbfe53deac9127de8 100644 (file)
--- a/common/discover.c
+++ b/common/discover.c
@@ -547,7 +547,7 @@ next_iface4(struct iface_info *info, int *err, struct iface_conf_list *ifaces) {
                                log_error("Interface name '%s' too long", name);
                                return 0;
                        }
-                       strcpy(info->name, name);
+                       strncpy(info->name, name, sizeof(info->name) - 1);
 
 #ifdef ALIAS_NAMED_PERMUTED
                        /* interface aliases look like "eth0:1" or "wlan1:3" */
@@ -564,7 +564,7 @@ next_iface4(struct iface_info *info, int *err, struct iface_conf_list *ifaces) {
 #endif
 
                memset(&tmp, 0, sizeof(tmp));
-               strcpy(tmp.ifr_name, name);
+               strncpy(tmp.ifr_name, name, sizeof(tmp.ifr_name) - 1);
                if (ioctl(ifaces->sock, SIOCGIFADDR, &tmp) < 0) {
                        if (errno == EADDRNOTAVAIL) {
                                continue;
@@ -577,7 +577,7 @@ next_iface4(struct iface_info *info, int *err, struct iface_conf_list *ifaces) {
                memcpy(&info->addr, &tmp.ifr_addr, sizeof(tmp.ifr_addr));
 
                memset(&tmp, 0, sizeof(tmp));
-               strcpy(tmp.ifr_name, name);
+               strncpy(tmp.ifr_name, name, sizeof(tmp.ifr_name) - 1);
                if (ioctl(ifaces->sock, SIOCGIFFLAGS, &tmp) < 0) {
                        log_error("Error getting interface flags for '%s'; %m", 
                                name);

diff --git a/common/lpf.c b/common/lpf.c
index ba05206392a768fca3d8cdeb44a5dbed4b6e2985..a63d61ba8e86b13a86f93469213e3fe0b14f4e96 100644 (file)
--- a/common/lpf.c
+++ b/common/lpf.c
@@ -95,6 +95,7 @@ int if_register_lpf (info)
        memset (&sa, 0, sizeof sa);
        sa.sa_family = AF_PACKET;
        strncpy (sa.sa_data, (const char *)info -> ifp, sizeof sa.sa_data);
+       sa.sa_data[sizeof(sa.sa_data)-1] = '\0';
        if (bind (sock, &sa, sizeof sa)) {
                if (errno == ENOPROTOOPT || errno == EPROTONOSUPPORT ||
                    errno == ESOCKTNOSUPPORT || errno == EPFNOSUPPORT ||
@@ -107,6 +108,7 @@ int if_register_lpf (info)
                        log_fatal ("configuration!");
                }
                log_fatal ("Bind socket to interface: %m");
+
        }
 
        get_hw_addr(info->name, &info->hw_address);
@@ -328,6 +330,7 @@ ssize_t send_packet (interface, packet, raw, len, from, to, hto)
        sa.spkt_family = AF_PACKET;
        strncpy ((char *)sa.spkt_device,
                 (const char *)interface -> ifp, sizeof sa.spkt_device);
+       sa.spkt_device[sizeof(sa.spkt_device) - 1] = '\0';
        sa.spkt_protocol = htons(ETH_P_IP);
 
        result = sendto (interface -> wfdesc,

diff --git a/dhcpctl/dhcpctl.c b/dhcpctl/dhcpctl.c
index c997e17f3c5a66f9212b309cfa9726d254da1cad..d4d2178731339d6025a62cca2c7f5666cd737918 100644 (file)
--- a/dhcpctl/dhcpctl.c
+++ b/dhcpctl/dhcpctl.c
@@ -243,6 +243,7 @@ dhcpctl_status dhcpctl_get_boolean (int *result,
        }
        memcpy (&rv, data -> value, sizeof rv);
        *result = ntohl (rv);
+       omapi_data_string_dereference (&data, MDL);
        return ISC_R_SUCCESS;
 }
 

diff --git a/dst/dst_api.c b/dst/dst_api.c
index 35c7a7df8a1a0794306f7498775c00d08e54accb..f93ee810d0d7fdff0bc7dd0c652ce2654bcac725 100644 (file)
--- a/dst/dst_api.c
+++ b/dst/dst_api.c
@@ -339,7 +339,6 @@ DST_KEY *
 dst_read_key(const char *in_keyname, const unsigned in_id, 
             const int in_alg, const int type)
 {
-       char keyname[PATH_MAX];
        DST_KEY *dg_key = NULL, *pubkey = NULL;
 
        if (!dst_check_algorithm(in_alg)) { /* make sure alg is available */
@@ -352,22 +351,21 @@ dst_read_key(const char *in_keyname, const unsigned in_id,
        if (in_keyname == NULL) {
                EREPORT(("dst_read_private_key(): Null key name passed in\n"));
                return (NULL);
-       } else
-               strncpy(keyname, in_keyname, PATH_MAX);
+    }
 
        /* before I read in the public key, check if it is allowed to sign */
-       if ((pubkey = dst_s_read_public_key(keyname, in_id, in_alg)) == NULL)
+       if ((pubkey = dst_s_read_public_key(in_keyname, in_id, in_alg)) == NULL)
                return (NULL);
 
        if (type == DST_PUBLIC) 
                return pubkey; 
 
-       if (!(dg_key = dst_s_get_key_struct(keyname, pubkey->dk_alg,
+       if (!(dg_key = dst_s_get_key_struct(in_keyname, pubkey->dk_alg,
                                            pubkey->dk_flags, pubkey->dk_proto,
                                            0)))
                return (dg_key);
        /* Fill in private key and some fields in the general key structure */
-       if (dst_s_read_private_key_file(keyname, dg_key, pubkey->dk_id,
+       if (dst_s_read_private_key_file((char *)(in_keyname), dg_key, pubkey->dk_id,
                                        pubkey->dk_alg) == 0)
                dg_key = dst_free_key(dg_key);
 
@@ -405,6 +403,7 @@ dst_write_key(const DST_KEY *key, const int type)
  *     K<key->dk_name>+<key->dk_alg>+<key->dk_id>.<private key suffix>.
  *     If there is already a file with this name, an error is returned.
  *
+ *
  *  Parameters
  *     key     A DST managed key structure that contains
  *           all information needed about a key.
@@ -482,6 +481,7 @@ dst_s_read_public_key(const char *in_name, const unsigned in_id, int in_alg)
         unsigned char *notspace;
        u_char deckey[RAW_KEY_SIZE];
        FILE *fp;
+       DST_KEY *pubkey = NULL;
 
        if (in_name == NULL) {
                EREPORT(("dst_read_public_key(): No key name given\n"));
@@ -584,11 +584,16 @@ dst_s_read_public_key(const char *in_name, const unsigned in_id, int in_alg)
                         dlen));
                return (NULL);
        }
+
        /* store key and info in a key structure that is returned */
-/*     return dst_store_public_key(in_name, alg, proto, 666, flags, deckey,
-                                   dlen);*/
-       return dst_buffer_to_key(in_name, alg,
-                                flags, proto, deckey, (unsigned)dlen);
+       /* Set the key id after we create because somehow this got missed. */
+       pubkey = dst_buffer_to_key(in_name, alg, flags, proto,
+                                  deckey, (unsigned)dlen);
+       if (pubkey) {
+               pubkey->dk_id = in_id;
+       }
+
+       return (pubkey);
 }
 
 
@@ -844,7 +849,7 @@ dst_s_read_private_key_file(char *name, DST_KEY *pk_key, unsigned in_id,
        int cnt, alg, len, major, minor, file_major, file_minor;
        int id;
        char filename[PATH_MAX];
-       u_char in_buff[RAW_KEY_SIZE];
+       u_char in_buff[RAW_KEY_SIZE + 1];
        char *p;
        FILE *fp;
 
@@ -866,8 +871,9 @@ dst_s_read_private_key_file(char *name, DST_KEY *pk_key, unsigned in_id,
                         (char *) getcwd(NULL, PATH_MAX - 1)));
                return (0);
        }
+
        /* now read the header info from the file */
-       if ((cnt = fread(in_buff, 1, sizeof(in_buff), fp)) < 5) {
+       if ((cnt = fread(in_buff, 1, sizeof(in_buff) - 1, fp)) < 5) {
                fclose(fp);
                EREPORT(("dst_s_read_private_key_file: error reading file %s (empty file)\n",
                         filename));
@@ -875,6 +881,8 @@ dst_s_read_private_key_file(char *name, DST_KEY *pk_key, unsigned in_id,
        }
        /* decrypt key */
        fclose(fp);
+       in_buff[cnt] = '\0';
+
        if (memcmp(in_buff, "Private-key-format: v", 20) != 0)
                goto fail;
        len = cnt;
@@ -1075,24 +1083,19 @@ dst_sig_size(DST_KEY *key) {
 int 
 dst_random(const int mode, unsigned wanted, u_char *outran)
 {
-       u_int32_t *buff = NULL, *bp = NULL;
-       int i;
-       if (wanted <= 0 || outran == NULL) 
+       if (wanted <= 0 || outran == NULL)
                return (0);
 
        switch (mode) {
-       case DST_RAND_SEMI: 
-               bp = buff = (u_int32_t *) malloc(wanted+sizeof(u_int32_t));
-               if (bp == NULL) {
-                       EREPORT(("malloc() failed for buff in function dst_random\n"));
-                       return (0);
-               }
-               for (i = 0; i < wanted; i+= sizeof(u_int32_t), bp++) {
-                       *bp = dst_s_quick_random(i);
+       case DST_RAND_SEMI: {
+               u_int32_t *op = (u_int32_t *)outran;
+               int i;
+               for (i = 0; i < wanted; i+= sizeof(u_int32_t), op++) {
+                       *op = dst_s_quick_random(i);
                }
-               memcpy(outran, buff, (unsigned)wanted);
-               SAFE_FREE(buff);
+
                return (wanted);
+               }
        case DST_RAND_STD:
                return (dst_s_semi_random(outran, wanted));
        case DST_RAND_KEY:
@@ -1103,4 +1106,3 @@ dst_random(const int mode, unsigned wanted, u_char *outran)
                return (0);
        }
 }
-

diff --git a/dst/dst_support.c b/dst/dst_support.c
index 95de020423f58f1c9ea7ead0e3a43080b69207ba..8e08a0c09987c03135cc37e4614b1ddf38c672d6 100644 (file)
--- a/dst/dst_support.c
+++ b/dst/dst_support.c
@@ -426,26 +426,29 @@ dst_s_build_filename(char *filename, const char *name, unsigned id,
 FILE *
 dst_s_fopen(const char *filename, const char *mode, unsigned perm)
 {
-       FILE *fp;
-       char pathname[PATH_MAX];
-       unsigned plen = sizeof(pathname);
-
-       if (*dst_path != '\0') {
-               strncpy(pathname, dst_path, PATH_MAX);
-               plen -= strlen(pathname);
+    FILE *fp;
+    char pathname[PATH_MAX];
+
+    /* Make sure the length is ok before we try to build it. */
+    if ((strlen(dst_path) + strlen(filename)) > PATH_MAX - 1) {
+        /* set errno in case anyone bothers to look */
+        errno = ENAMETOOLONG;
+        return (NULL);
+    }
+
+    /* dst_path if not empty has a terminating "/" already */
+    strcpy(pathname, dst_path);
+    strcpy(pathname + strlen(pathname), filename);
+
+    fp = fopen(pathname, mode);
+    if ((fp != NULL) && (perm != 0)) {
+       if (chmod(pathname, perm) < 0) {
+               fclose(fp);
+               return (NULL);
        }
-       else 
-               pathname[0] = '\0';
+    }
 
-       if (plen > strlen(filename))
-               strncpy(&pathname[PATH_MAX - plen], filename, plen-1);
-       else 
-               return (NULL);
-       
-       fp = fopen(pathname, mode);
-       if (perm)
-               chmod(pathname, perm);
-       return (fp);
+    return (fp);
 }
 
 #if 0

diff --git a/server/confpars.c b/server/confpars.c
index 8a5a4b33dab3ea32ccca0200b4dba4d08ba716ef..a10c98eb0202c6a05703bfa84bdcd188a3021245 100644 (file)
--- a/server/confpars.c
+++ b/server/confpars.c
@@ -658,10 +658,10 @@ int parse_statement (cfile, group, type, host_decl, declaration)
              case POOL6:
                skip_token(&val, NULL, cfile);
                if (type == POOL_DECL) {
-                       parse_warn (cfile, "pool declared within pool.");
+                       parse_warn (cfile, "pool6 declared within pool.");
                        skip_to_semi(cfile);
                } else if (type != SUBNET_DECL) {
-                       parse_warn (cfile, "pool declared outside of network");
+                       parse_warn (cfile, "pool6 declared outside of network");
                        skip_to_semi(cfile);
                } else 
                        parse_pool6_statement (cfile, group, type);
@@ -906,7 +906,6 @@ void parse_failover_peer (cfile, group, type)
 
        token = next_token (&val, (unsigned *)0, cfile);
        if (token == SEMI) {
-               dfree (name, MDL);
                if (type != SHARED_NET_DECL)
                        parse_warn (cfile, "failover peer reference not %s",
                                    "in shared-network declaration");
@@ -914,6 +913,7 @@ void parse_failover_peer (cfile, group, type)
                        if (!peer) {
                                parse_warn (cfile, "reference to unknown%s%s",
                                            " failover peer ", name);
+                                dfree (name, MDL);
                                return;
                        }
                        dhcp_failover_state_reference
@@ -921,15 +921,18 @@ void parse_failover_peer (cfile, group, type)
                                 peer, MDL);
                }
                dhcp_failover_state_dereference (&peer, MDL);
+                dfree (name, MDL);
                return;
        } else if (token == STATE) {
                if (!peer) {
                        parse_warn (cfile, "state declaration for unknown%s%s",
                                    " failover peer ", name);
+                        dfree (name, MDL);
                        return;
                }
                parse_failover_state_declaration (cfile, peer);
                dhcp_failover_state_dereference (&peer, MDL);
+                dfree (name, MDL);
                return;
        } else if (token != LBRACE) {
                parse_warn (cfile, "expecting left brace");
@@ -941,6 +944,7 @@ void parse_failover_peer (cfile, group, type)
                parse_warn (cfile, "redeclaration of failover peer %s", name);
                skip_to_rbrace (cfile, 1);
                dhcp_failover_state_dereference (&peer, MDL);
+                dfree (name, MDL);
                return;
        }
 
@@ -4304,8 +4308,9 @@ void parse_pool6_statement (cfile, group, type)
                                         group->subnet->shared_network,
                                         MDL);
        else {
-               parse_warn(cfile, "Dynamic pool6s are only valid inside "
+               parse_warn(cfile, "pool6s are only valid inside "
                                  "subnet statements.");
+               ipv6_pond_dereference(&pond, MDL);
                skip_to_semi(cfile);
                return;
        }
@@ -4456,6 +4461,7 @@ int parse_allow_deny (oc, cfile, flag)
              default:
                parse_warn (cfile, "expecting allow/deny key");
                skip_to_semi (cfile);
+               expression_dereference (&data, MDL);
                return 0;
        }
        /* Reference on option is passed to option cache. */

diff --git a/server/ddns.c b/server/ddns.c
index 1328fc6b108628c3b30c51ee604981e05dba2ee7..aba57e7fef15ab0da811e34f1e09b87e83e98098 100644 (file)
--- a/server/ddns.c
+++ b/server/ddns.c
@@ -1135,8 +1135,7 @@ ddns_update_lease_ptr(struct lease    *lease,
                return (ISC_R_FAILURE);
        }
        else {
-               strncpy(ddns_address, piaddr(ddns_cb->address), 
-                       MAX_ADDRESS_STRING_LEN);
+               strcpy(ddns_address, piaddr(ddns_cb->address));
        }
 #if defined (DEBUG_DNS_UPDATES)
        log_info("%s(%d): Updating lease_ptr for ddns_cp=%p (addr=%s)",

diff --git a/server/omapi.c b/server/omapi.c
index 84a1bd89dba10a0adfc08f819f9267cbdf354e4e..081787171c83b7a3d4e470696bf83fb5643f7acc 100644 (file)
--- a/server/omapi.c
+++ b/server/omapi.c
@@ -1733,21 +1733,14 @@ class_set_value (omapi_object_t *h,
        class = (struct class *)h;
 
        if (!omapi_ds_strcmp(name, "name")) {
-               char *tname;
-
                if (class->name)
                        return ISC_R_EXISTS;
 
-               if ((tname = dmalloc(value->u.buffer.len + 1, MDL)) == NULL) {
-                       return ISC_R_NOMEMORY;
-               }
-
-               /* tname is null terminated from dmalloc() */
-               memcpy(tname, value->u.buffer.value, value->u.buffer.len);
-
                if (issubclass) {
+                       char tname[value->u.buffer.len + 1];
+                       memcpy(tname, value->u.buffer.value, value->u.buffer.len);
+                       tname[sizeof(tname)-1] = '\0';
                        status = find_class(&superclass, tname, MDL);
-                       dfree(tname, MDL);
 
                        if (status == ISC_R_NOTFOUND)
                                return status;