[master] Corrects high impact issues reported by Coverity.

Merges in rt36712

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
 

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.

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);

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,

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;
 }
 

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,
+	/* Set the key id after we create because somehow this got missed. */
-				    dlen);*/
+	pubkey = dst_buffer_to_key(in_name, alg, flags, proto,
-	return dst_buffer_to_key(in_name, alg,
+			           deckey, (unsigned)dlen);
-				 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)
 		return (0);
 
 	switch (mode) {
-	case DST_RAND_SEMI: 
+	case DST_RAND_SEMI: {
-		bp = buff = (u_int32_t *) malloc(wanted+sizeof(u_int32_t));
+		u_int32_t *op = (u_int32_t *)outran;
-		if (bp == NULL) {
+		int i;
-			EREPORT(("malloc() failed for buff in function dst_random\n"));
+		for (i = 0; i < wanted; i+= sizeof(u_int32_t), op++) {
-			return (0);
+			*op = dst_s_quick_random(i);
 		}
-		for (i = 0; i < wanted; i+= sizeof(u_int32_t), bp++) {
+
-			*bp = 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);
 	}
 }
-

dst/dst_support.c

@@ -428,23 +428,26 @@ 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') {
+    /* Make sure the length is ok before we try to build it. */
-		strncpy(pathname, dst_path, PATH_MAX);
+    if ((strlen(dst_path) + strlen(filename)) > PATH_MAX - 1) {
-		plen -= strlen(pathname);
+        /* set errno in case anyone bothers to look */
-	}
+        errno = ENAMETOOLONG;
-	else 
-		pathname[0] = '\0';
-
-	if (plen > strlen(filename))
-		strncpy(&pathname[PATH_MAX - plen], filename, plen-1);
-	else 
         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 (perm)
+    if ((fp != NULL) && (perm != 0)) {
-		chmod(pathname, perm);
+	if (chmod(pathname, perm) < 0) {
+		fclose(fp);
+		return (NULL);
+	}
+    }
+
     return (fp);
 }
 

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. */

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), 
+		strcpy(ddns_address, piaddr(ddns_cb->address));
-			MAX_ADDRESS_STRING_LEN);
 	}
 #if defined (DEBUG_DNS_UPDATES)
 	log_info("%s(%d): Updating lease_ptr for ddns_cp=%p (addr=%s)",

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;