[master] Corrects medium impact issues reported by Coverity.

Merges in rt36933

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
 

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

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

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

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

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

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

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

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;
 

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

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

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;

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