[master]

[rt23833]
Clean up a number of items identified by the Coverity
static analysis tool.  Runs courtesy of Red Hat.

29 files changed, 309 insertions, 182 deletions

diff --git a/RELNOTES b/RELNOTES
index fa26756e..d36f98a1 100644
--- a/RELNOTES
+++ b/RELNOTES
@@ -142,6 +142,11 @@ work on other platforms. Please report any problems and suggested fixes to
   is only useful when doing load balancing within failover.
   [ISC-Bugs #26108]
 
+- Fix a set of issues that were discovered via a code inspection
+  tool.  Thanks to Jiri Popelka and Tomas Hozza Red Hat for the logs
+  and patches.
+  [ISC-Bugs #23833]
+
 			Changes since 4.2.3
 
 ! Add a check for a null pointer before calling the regexec function.
diff --git a/client/dhc6.c b/client/dhc6.c
index eb019c87..f9220486 100644
--- a/client/dhc6.c
+++ b/client/dhc6.c
@@ -214,8 +214,10 @@ dhcpv6_client_assignments(void)
 	memset(&DHCPv6DestAddr, 0, sizeof(DHCPv6DestAddr));
 	DHCPv6DestAddr.sin6_family = AF_INET6;
 	DHCPv6DestAddr.sin6_port = remote_port;
-	inet_pton(AF_INET6, All_DHCP_Relay_Agents_and_Servers,
-		  &DHCPv6DestAddr.sin6_addr);
+	if (inet_pton(AF_INET6, All_DHCP_Relay_Agents_and_Servers,
+		      &DHCPv6DestAddr.sin6_addr) <= 0) {
+		log_fatal("Bad address %s", All_DHCP_Relay_Agents_and_Servers);
+	}
 
 	code = D6O_CLIENTID;
 	if (!option_code_hash_lookup(&clientid_option,
@@ -656,7 +658,8 @@ dhc6_leaseify(struct packet *packet)
 	 * not sure based on what additional keys now).
 	 */
 	oc = lookup_option(&dhcpv6_universe, packet->options, D6O_SERVERID);
-	if (!evaluate_option_cache(&lease->server_id, packet, NULL, NULL,
+	if ((oc == NULL) ||
+	    !evaluate_option_cache(&lease->server_id, packet, NULL, NULL,
 				   lease->options, NULL, &global_scope,
 				   oc, MDL) ||
 	    lease->server_id.len == 0) {
diff --git a/common/alloc.c b/common/alloc.c
index f2ddc8ad..3cbd140a 100644
--- a/common/alloc.c
+++ b/common/alloc.c
@@ -3,7 +3,8 @@
    Memory allocation... */
 
 /*
- * Copyright (c) 2004-2007,2009 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2009,2012 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004-2007 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 1996-2003 by Internet Software Consortium
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -831,11 +832,11 @@ int dns_host_entry_dereference (ptr, file, line)
 #endif
 	}
 
-	(*ptr) -> refcnt--;
-	rc_register (file, line, ptr, *ptr, (*ptr) -> refcnt, 1, RC_MISC);
-	if (!(*ptr) -> refcnt)
+	(*ptr)->refcnt--;
+	rc_register (file, line, ptr, *ptr, (*ptr)->refcnt, 1, RC_MISC);
+	if ((*ptr)->refcnt == 0) {
 		dfree ((*ptr), file, line);
-	if ((*ptr) -> refcnt < 0) {
+	} else if ((*ptr)->refcnt < 0) {
 		log_error ("%s(%d): negative refcnt!", file, line);
 #if defined (DEBUG_RC_HISTORY)
 		dump_rc_history (*ptr);
diff --git a/common/comapi.c b/common/comapi.c
index 90d2262c..a9ae043c 100644
--- a/common/comapi.c
+++ b/common/comapi.c
@@ -692,7 +692,6 @@ isc_result_t dhcp_subnet_signal_handler (omapi_object_t *h,
 	/* In this function h should be a (struct subnet *) */
 
 	isc_result_t status;
-	int updatep = 0;
 
 	if (h -> type != dhcp_type_subnet)
 		return DHCP_R_INVALIDARG;
@@ -706,8 +705,7 @@ isc_result_t dhcp_subnet_signal_handler (omapi_object_t *h,
 		if (status == ISC_R_SUCCESS)
 			return status;
 	}
-	if (updatep)
-		return ISC_R_SUCCESS;
+
 	return ISC_R_NOTFOUND;
 }
 
@@ -861,7 +859,6 @@ isc_result_t dhcp_shared_network_signal_handler (omapi_object_t *h,
 	/* In this function h should be a (struct shared_network *) */
 
 	isc_result_t status;
-	int updatep = 0;
 
 	if (h -> type != dhcp_type_shared_network)
 		return DHCP_R_INVALIDARG;
@@ -875,8 +872,7 @@ isc_result_t dhcp_shared_network_signal_handler (omapi_object_t *h,
 		if (status == ISC_R_SUCCESS)
 			return status;
 	}
-	if (updatep)
-		return ISC_R_SUCCESS;
+
 	return ISC_R_NOTFOUND;
 }
 
diff --git a/common/lpf.c b/common/lpf.c
index 0f0ebe4c..ba052063 100644
--- a/common/lpf.c
+++ b/common/lpf.c
@@ -4,7 +4,8 @@
    Support Services in Vancouver, B.C. */
 
 /*
- * Copyright (c) 2004,2007,2009 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2009,2012 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004,2007 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 1996-2003 by Internet Software Consortium
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -463,9 +464,9 @@ get_hw_addr(const char *name, struct hardware *hw) {
 			memcpy(&hw->hbuf[1], sa->sa_data, 6);
 			break;
 		case ARPHRD_FDDI:
-			hw->hlen = 17;
+			hw->hlen = 7;
 			hw->hbuf[0] = HTYPE_FDDI;
-			memcpy(&hw->hbuf[1], sa->sa_data, 16);
+			memcpy(&hw->hbuf[1], sa->sa_data, 6);
 			break;
 		default:
 			log_fatal("Unsupported device type %ld for \"%s\"",
diff --git a/common/options.c b/common/options.c
index 79db3669..02bddb47 100644
--- a/common/options.c
+++ b/common/options.c
@@ -258,9 +258,14 @@ int parse_option_buffer (options, buffer, length, universe)
 				option_cache_reference(&op->next, nop, MDL);
 				option_cache_dereference(&nop, MDL);
 			} else {
-				save_option_buffer(universe, options, bp,
-						   bp->data + offset, len,
-						   code, 1);
+				if (save_option_buffer(universe, options, bp,
+						       bp->data + offset, len,
+						       code, 1) == 0) {
+					log_error("parse_option_buffer: "
+						  "save_option_buffer failed");
+					buffer_dereference(&bp, MDL);
+					return 0;
+				}
 			}
 		}
 		option_dereference(&option, MDL);
@@ -517,6 +522,8 @@ int fqdn_universe_decode (struct option_state *options,
  * Load all options into a buffer, and then split them out into the three
  * separate fields in the dhcp packet (options, file, and sname) where
  * options can be stored.
+ *
+ * returns 0 on error, length of packet on success
  */
 int
 cons_options(struct packet *inpacket, struct dhcp_packet *outpacket,
@@ -553,10 +560,10 @@ cons_options(struct packet *inpacket, struct dhcp_packet *outpacket,
 
 	if (inpacket &&
 	    (op = lookup_option(&dhcp_universe, inpacket->options,
-				 DHO_DHCP_MAX_MESSAGE_SIZE))) {
-		evaluate_option_cache(&ds, inpacket,
-				       lease, client_state, in_options,
-				       cfg_options, scope, op, MDL);
+				DHO_DHCP_MAX_MESSAGE_SIZE)) &&
+	    (evaluate_option_cache(&ds, inpacket, lease,
+				   client_state, in_options,
+				   cfg_options, scope, op, MDL) != 0)) {
 		if (ds.len >= sizeof (u_int16_t)) {
 			i = getUShort(ds.data);
 			if(!mms || (i < mms))
@@ -679,7 +686,7 @@ cons_options(struct packet *inpacket, struct dhcp_packet *outpacket,
 		 * in the packet if there is space.  Note that the option
 		 * may only be included if the client supplied one.
 		 */
-		if ((priority_len < PRIORITY_COUNT) &&
+		if ((inpacket != NULL) && (priority_len < PRIORITY_COUNT) &&
 		    (lookup_option(&fqdn_universe, inpacket->options,
 				   FQDN_ENCODED) != NULL))
 			priority_list[priority_len++] = DHO_FQDN;
@@ -695,7 +702,7 @@ cons_options(struct packet *inpacket, struct dhcp_packet *outpacket,
 		 * DHCPINFORM or DHCPLEASEQUERY responses (if the client
 		 * didn't request it).
 		 */
-		if ((priority_len < PRIORITY_COUNT) &&
+		if ((inpacket != NULL) && (priority_len < PRIORITY_COUNT) &&
 		    ((inpacket->packet_type == DHCPDISCOVER) ||
 		     (inpacket->packet_type == DHCPREQUEST)))
 			priority_list[priority_len++] = DHO_SUBNET_MASK;
@@ -1266,11 +1273,12 @@ store_options(int *ocount,
 						 cfg_options,
 						 vendor_cfg_option -> code);
 			    if (tmp)
-				evaluate_option_cache (&name, packet, lease,
-						       client_state,
-						       in_options,
-						       cfg_options,
-						       scope, tmp, MDL);
+				/* No need to check the return as we check name.len below */
+				(void) evaluate_option_cache (&name, packet, lease,
+							      client_state,
+							      in_options,
+							      cfg_options,
+							      scope, tmp, MDL);
 			} else if (vuname) {
 			    name.data = (unsigned char *)s;
 			    name.len = strlen (s);
@@ -1308,9 +1316,10 @@ store_options(int *ocount,
 	    /* Find the value of the option... */
 	    od.len = 0;
 	    if (oc) {
-		evaluate_option_cache (&od, packet,
-				       lease, client_state, in_options,
-				       cfg_options, scope, oc, MDL);
+		/* No need to check the return as we check od.len below */
+		(void) evaluate_option_cache (&od, packet,
+					      lease, client_state, in_options,
+					      cfg_options, scope, oc, MDL);
 
 		/* If we have encapsulation for this option, and an oc
 		 * lookup succeeded, but the evaluation failed, it is
@@ -3143,9 +3152,11 @@ int fqdn_option_space_encapsulate (result, packet, lease, client_state,
 		struct option_cache *oc = (struct option_cache *)(ocp -> car);
 		if (oc -> option -> code > FQDN_SUBOPTION_COUNT)
 			continue;
-		evaluate_option_cache (&results [oc -> option -> code],
-				       packet, lease, client_state, in_options,
-				       cfg_options, scope,  oc, MDL);
+		/* No need to check the return code, we check the length later */
+		(void) evaluate_option_cache (&results[oc->option->code],
+					      packet, lease, client_state,
+					      in_options, cfg_options, scope,
+					      oc, MDL);
 	}
 	/* We add a byte for the flags field.
 	 * We add two bytes for the two RCODE fields.
@@ -3310,10 +3321,10 @@ fqdn6_option_space_encapsulate(struct data_string *result,
 		oc = (struct option_cache *)(ocp->car);
 		if (oc->option->code > FQDN_SUBOPTION_COUNT)
 			log_fatal("Impossible condition at %s:%d.", MDL);
-
-		evaluate_option_cache(&results[oc->option->code], packet,
-				      lease, client_state, in_options,
-				      cfg_options, scope, oc, MDL);
+		/* No need to check the return code, we check the length later */
+		(void) evaluate_option_cache(&results[oc->option->code], packet,
+					     lease, client_state, in_options,
+					     cfg_options, scope, oc, MDL);
 	}
 
 	/* We add a byte for the flags field at the start of the option.
diff --git a/common/parse.c b/common/parse.c
index f72c0d65..1a4be3a7 100644
--- a/common/parse.c
+++ b/common/parse.c
@@ -675,7 +675,23 @@ void parse_lease_time (cfile, timep)
    the token specified in separator.  If max is zero, any number of
    numbers will be parsed; otherwise, exactly max numbers are
    expected.  Base and size tell us how to internalize the numbers
-   once they've been tokenized. */
+   once they've been tokenized.
+
+   buf - A pointer to space to return the parsed value, if it is null
+   then the function will allocate space for the return.
+
+   max - The maximum number of items to store.  If zero there is no
+   maximum.  When buf is null and the function needs to allocate space
+   it will do an allocation of max size at the beginning if max is non
+   zero.  If max is zero then the allocation will be done later, after
+   the function has determined the size necessary for the incoming
+   string.
+
+   returns NULL on errors or a pointer to the value string on success.
+   The pointer will either be buf if it was non-NULL or newly allocated
+   space if buf was NULL
+ */
+
 
 unsigned char *parse_numeric_aggregate (cfile, buf,
 					max, separator, base, size)
@@ -696,9 +712,8 @@ unsigned char *parse_numeric_aggregate (cfile, buf,
 		bufp = (unsigned char *)dmalloc (*max * size / 8, MDL);
 		if (!bufp)
 			log_fatal ("no space for numeric aggregate");
-		s = 0;
-	} else
-		s = bufp;
+	}
+	s = bufp;
 
 	do {
 		if (count) {
@@ -713,6 +728,9 @@ unsigned char *parse_numeric_aggregate (cfile, buf,
 				parse_warn (cfile, "too few numbers.");
 				if (token != SEMI)
 					skip_to_semi (cfile);
+				/* free bufp if it was allocated */
+				if ((bufp != NULL) && (bufp != buf))
+					dfree(bufp, MDL);
 				return (unsigned char *)0;
 			}
 			token = next_token (&val, (unsigned *)0, cfile);
@@ -729,7 +747,17 @@ unsigned char *parse_numeric_aggregate (cfile, buf,
 		    (base != 16 || token != NUMBER_OR_NAME)) {
 			parse_warn (cfile, "expecting numeric value.");
 			skip_to_semi (cfile);
-			return (unsigned char *)0;
+			/* free bufp if it was allocated */
+			if ((bufp != NULL) && (bufp != buf))
+				dfree(bufp, MDL);
+			/* free any linked numbers we may have allocated */
+			while (c) {
+				pair cdr = c->cdr;
+				dfree(c->car, MDL);
+				dfree(c, MDL);
+				c = cdr;
+			}
+			return (NULL);
 		}
 		/* If we can, convert the number now; otherwise, build
 		   a linked list of all the numbers. */
@@ -747,6 +775,10 @@ unsigned char *parse_numeric_aggregate (cfile, buf,
 
 	/* If we had to cons up a list, convert it now. */
 	if (c) {
+		/*
+		 * No need to cleanup bufp, to get here we didn't allocate
+		 * bufp above
+		 */
 		bufp = (unsigned char *)dmalloc (count * size / 8, MDL);
 		if (!bufp)
 			log_fatal ("no space for numeric aggregate.");
@@ -1870,7 +1902,7 @@ int parse_base64 (data, cfile)
 		parse_warn (cfile, "can't allocate buffer for base64 data.");
 		data -> len = 0;
 		data -> data = (unsigned char *)0;
-		return 0;
+		goto out;
 	}
 		
 	j = k = 0;
@@ -5258,6 +5290,8 @@ int parse_option_token (rv, cfile, fmt, expr, uniform, lookups)
 			return 0;
 		}
 		*fmt = g;
+		/* FALL THROUGH */
+		/* to get string value for the option */
 	      case 'X':
 		token = peek_token (&val, (unsigned *)0, cfile);
 		if (token == NUMBER_OR_NAME || token == NUMBER) {
@@ -5549,6 +5583,8 @@ int parse_option_decl (oc, cfile)
 						    "encapsulation format");
 					goto parse_exit;
 				}
+				/* FALL THROUGH */
+				/* to get string value for the option */
 			      case 'X':
 				len = parse_X (cfile, &hunkbuf [hunkix],
 					       sizeof hunkbuf - hunkix);
@@ -5760,8 +5796,6 @@ int parse_option_decl (oc, cfile)
 	bp = (struct buffer *)0;
 	if (!buffer_allocate (&bp, hunkix + nul_term, MDL))
 		log_fatal ("no memory to store option declaration.");
-	if (!bp -> data)
-		log_fatal ("out of memory allocating option data.");
 	memcpy (bp -> data, hunkbuf, hunkix + nul_term);
 	
 	if (!option_cache_allocate (oc, MDL))
diff --git a/common/print.c b/common/print.c
index d5d0ae07..b113bec1 100644
--- a/common/print.c
+++ b/common/print.c
@@ -1134,6 +1134,7 @@ static unsigned print_subexpression (expr, buf, len)
 			buf [rv] = 0;
 			return rv;
 		}
+		break;
 
 	      case expr_gethostname:
 		if (len > 13) {
@@ -1245,7 +1246,12 @@ int token_print_indent (FILE *file, int col, int indent,
 			const char *prefix,
 			const char *suffix, const char *buf)
 {
-	int len = strlen (buf) + strlen (prefix);
+	int len = 0;
+	if (prefix != NULL)
+		len = strlen (prefix);
+	if (buf != NULL)
+		len += strlen (buf);
+
 	if (col + len > 79) {
 		if (indent + len < 79) {
 			indent_spaces (file, indent);
@@ -1258,8 +1264,10 @@ int token_print_indent (FILE *file, int col, int indent,
 		fputs (prefix, file);
 		col += strlen (prefix);
 	}
-	fputs (buf, file);
-	col += len;
+	if ((buf != NULL) && (*buf != 0)) {
+		fputs (buf, file);
+		col += strlen(buf);
+	}
 	if (suffix && *suffix) {
 		if (col + strlen (suffix) > 79) {
 			indent_spaces (file, indent);
diff --git a/common/tree.c b/common/tree.c
index c82ac271..17db4c06 100644
--- a/common/tree.c
+++ b/common/tree.c
@@ -2902,10 +2902,17 @@ int evaluate_numeric_expression (result, packet, lease, client_state,
 	return 0;
 }
 
-/* Return data hanging off of an option cache structure, or if there
-   isn't any, evaluate the expression hanging off of it and return the
-   result of that evaluation.   There should never be both an expression
-   and a valid data_string. */
+/*
+ * Return data hanging off of an option cache structure, or if there
+ * isn't any, evaluate the expression hanging off of it and return the
+ * result of that evaluation.   There should never be both an expression
+ * and a valid data_string.
+ *
+ * returns 0 if there wasn't an expression or it couldn't be evaluated
+ * returns non-zero if there was an expression or string that was evaluated
+ * When it returns zero the arguements, in particualr resutl,  should not
+ * be modified
+ */
 
 int evaluate_option_cache (result, packet, lease, client_state,
 			   in_options, cfg_options, scope, oc, file, line)
@@ -3613,6 +3620,7 @@ int write_expression (file, expr, col, indent, firstp)
 		col = write_expression (file, expr -> data.suffix.len,
 					col, scol, 0);
 		col = token_print_indent (file, col, indent, "", "", ")");
+		break;
 
 	      case expr_lcase:
 		col = token_print_indent(file, col, indent, "", "", "lcase");
@@ -4148,19 +4156,10 @@ int fundef_dereference (ptr, file, line)
 	const char *file;
 	int line;
 {
-	struct fundef *bp = *ptr;
+	struct fundef *bp;
 	struct string_list *sp, *next;
 
-	if (!ptr) {
-		log_error ("%s(%d): null pointer", file, line);
-#if defined (POINTER_DEBUG)
-		abort ();
-#else
-		return 0;
-#endif
-	}
-
-	if (!bp) {
+	if ((ptr == NULL) || (*ptr == NULL)) {
 		log_error ("%s(%d): null pointer", file, line);
 #if defined (POINTER_DEBUG)
 		abort ();
@@ -4169,6 +4168,7 @@ int fundef_dereference (ptr, file, line)
 #endif
 	}
 
+	bp = *ptr;
 	bp -> refcnt--;
 	rc_register (file, line, ptr, bp, bp -> refcnt, 1, RC_MISC);
 	if (bp -> refcnt < 0) {
@@ -4440,11 +4440,11 @@ int find_bound_string (struct data_string *value,
 	if (binding -> value -> value.data.terminated) {
 		data_string_copy (value, &binding -> value -> value.data, MDL);
 	} else {
-		buffer_allocate (&value -> buffer,
-				 binding -> value -> value.data.len,
-				 MDL);
-		if (!value -> buffer)
+		if (buffer_allocate (&value->buffer,
+				     binding->value->value.data.len,
+				     MDL) == 0) {
 			return 0;
+		}
 
 		memcpy (value -> buffer -> data,
 			binding -> value -> value.data.data,
diff --git a/dst/dst_api.c b/dst/dst_api.c
index 0d18b611..85f2ec6a 100644
--- a/dst/dst_api.c
+++ b/dst/dst_api.c
@@ -349,7 +349,7 @@ dst_read_key(const char *in_keyname, const unsigned in_id,
 		EREPORT(("dst_read_private_key(): Null key name passed in\n"));
 		return (NULL);
 	} else
-		strcpy(keyname, in_keyname);
+		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)
@@ -443,6 +443,7 @@ dst_s_write_private_key(const DST_KEY *key)
 		if ((nn = fwrite(encoded_block, 1, len, fp)) != len) {
 			EREPORT(("dst_write_private_key(): Write failure on %s %d != %d errno=%d\n",
 				 file, out_len, nn, errno));
+			fclose(fp);
 			return (-5);
 		}
 		fclose(fp);
@@ -663,7 +664,7 @@ dst_dnskey_to_key(const char *in_name,
 	int alg ;
 	int start = DST_KEY_START;
 
-	if (rdata == NULL || len <= DST_KEY_ALG) /* no data */
+	if (in_name == NULL || rdata == NULL || len <= DST_KEY_ALG) /* no data */
 		return (NULL);
 	alg = (u_int8_t) rdata[DST_KEY_ALG];
 	if (!dst_check_algorithm(alg)) { /* make sure alg is available */
@@ -674,8 +675,6 @@ dst_dnskey_to_key(const char *in_name,
 	if ((key_st = dst_s_get_key_struct(in_name, alg, 0, 0, 0)) == NULL)
 		return (NULL);
 
-	if (in_name == NULL)
-		return (NULL);
 	key_st->dk_flags = dst_s_get_int16(rdata);
 	key_st->dk_proto = (u_int16_t) rdata[DST_KEY_PROT];
 	if (key_st->dk_flags & DST_EXTEND_FLAG) {
@@ -795,10 +794,12 @@ dst_buffer_to_key(const char *key_name,		/* name of the key */
 	    dkey->dk_func->from_dns_key != NULL) {
 		if (dkey->dk_func->from_dns_key(dkey, key_buf, key_len) < 0) {
 			EREPORT(("dst_buffer_to_key(): dst_buffer_to_hmac failed\n"));
-			return (dst_free_key(dkey));
+			(void) (dst_free_key(dkey));
+			return (NULL);
 		}
 		return (dkey);
 	}
+	(void) (dst_free_key(dkey));
 	return (NULL);
 }
 
@@ -1012,11 +1013,9 @@ dst_free_key(DST_KEY *f_key)
 	else {
 		EREPORT(("dst_free_key(): Unknown key alg %d\n",
 			 f_key->dk_alg));
-		free(f_key->dk_KEY_struct);	/* SHOULD NOT happen */
 	}
 	if (f_key->dk_KEY_struct) {
-		free(f_key->dk_KEY_struct);
-		f_key->dk_KEY_struct = NULL;
+		SAFE_FREE(f_key->dk_KEY_struct);
 	}
 	if (f_key->dk_key_name)
 		SAFE_FREE(f_key->dk_key_name);
diff --git a/dst/dst_support.c b/dst/dst_support.c
index c7a71c03..5d0c432a 100644
--- a/dst/dst_support.c
+++ b/dst/dst_support.c
@@ -4,6 +4,7 @@ static const char rcsid[] = "$Header: /tmp/cvstest/DHCP/dst/dst_support.c,v 1.7
 /*
  * Portions Copyright (c) 1995-1998 by Trusted Information Systems, Inc.
  * Portions Copyright (c) 2007,2009 by Internet Systems Consortium, Inc. ("ISC")
+ * Portions Copyright (c) 2012 by Internet Systems Consortium, Inc. ("ISC")
  *
  * Permission to use, copy modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -58,6 +59,7 @@ dst_s_conv_bignum_u8_to_b64(char *out_buf, const unsigned out_len,
 {
 	const u_char *bp = bin_data;
 	char *op = out_buf;
+	int res = 0;
 	unsigned lenh = 0, len64 = 0;
 	unsigned local_in_len = bin_len;
 	unsigned local_out_len = out_len;
@@ -81,9 +83,10 @@ dst_s_conv_bignum_u8_to_b64(char *out_buf, const unsigned out_len,
 		local_out_len -= lenh;
 		op += lenh;
 	}
-	len64 = b64_ntop(bp, local_in_len, op, local_out_len - 2);
-	if (len64 < 0)
+	res = b64_ntop(bp, local_in_len, op, local_out_len - 2);
+	if (res < 0)
 		return (-1);
+	len64 = (unsigned) res;
 	op += len64++;
 	*(op++) = '\n';		/* put CR in the output */
 	*op = '\0';		/* make sure output is 0 terminated */
@@ -150,6 +153,7 @@ dst_s_conv_bignum_b64_to_u8(const char **buf,
 	unsigned blen;
 	char *bp;
 	u_char bstr[RAW_KEY_SIZE];
+	int res = 0;
 
 	if (buf == NULL || *buf == NULL) {	/* error checks */
 		EREPORT(("dst_s_conv_bignum_b64_to_u8: null input buffer.\n"));
@@ -159,12 +163,13 @@ dst_s_conv_bignum_b64_to_u8(const char **buf,
 	if (bp != NULL)
 		*bp = '\0';
 
-	blen = b64_pton(*buf, bstr, sizeof(bstr));
-	if (blen <= 0) {
+	res = b64_pton(*buf, bstr, sizeof(bstr));
+	if (res <= 0) {
 		EREPORT(("dst_s_conv_bignum_b64_to_u8: decoded value is null.\n"));
 		return (0);
 	}
-	else if (loclen < blen) {
+	blen = (unsigned) res;
+	if (loclen < blen) {
 		EREPORT(("dst_s_conv_bignum_b64_to_u8: decoded value is longer than output buffer.\n"));
 		return (0);
 	}
@@ -429,7 +434,7 @@ dst_s_fopen(const char *filename, const char *mode, unsigned perm)
 	unsigned plen = sizeof(pathname);
 
 	if (*dst_path != '\0') {
-		strcpy(pathname, dst_path);
+		strncpy(pathname, dst_path, PATH_MAX);
 		plen -= strlen(pathname);
 	}
 	else 
diff --git a/dst/hmac_link.c b/dst/hmac_link.c
index 277b54a8..1fa36caf 100644
--- a/dst/hmac_link.c
+++ b/dst/hmac_link.c
@@ -5,6 +5,7 @@ static const char rcsid[] = "$Header: /tmp/cvstest/DHCP/dst/hmac_link.c,v 1.6 20
 /*
  * Portions Copyright (c) 1995-1998 by Trusted Information Systems, Inc.
  * Portions Copyright (c) 2007,2009 by Internet Systems Consortium, Inc. ("ISC")
+ * Portions Copyright (c) 2012 by Internet Systems Consortium, Inc. ("ISC")
  *
  * Permission to use, copy modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -87,6 +88,10 @@ dst_hmac_md5_sign(const int mode, DST_KEY *d_key, void **context,
 	int sign_len = 0;
 	MD5_CTX *ctx = NULL;
 
+	if (d_key == NULL || d_key->dk_KEY_struct == NULL)
+		return (-1);
+	key = (HMAC_Key *) d_key->dk_KEY_struct;
+
 	if (mode & SIG_MODE_INIT) 
 		ctx = (MD5_CTX *) malloc(sizeof(*ctx));
 	else if (context)
@@ -94,10 +99,6 @@ dst_hmac_md5_sign(const int mode, DST_KEY *d_key, void **context,
 	if (ctx == NULL) 
 		return (-1);
 
-	if (d_key == NULL || d_key->dk_KEY_struct == NULL)
-		return (-1);
-	key = (HMAC_Key *) d_key->dk_KEY_struct;
-
 	if (mode & SIG_MODE_INIT) {
 		MD5Init(ctx);
 		MD5Update(ctx, key->hk_ipad, HMAC_LEN);
@@ -154,6 +155,10 @@ dst_hmac_md5_verify(const int mode, DST_KEY *d_key, void **context,
 	HMAC_Key *key;
 	MD5_CTX *ctx = NULL;
 
+	if (d_key == NULL || d_key->dk_KEY_struct == NULL)
+		return (-1);
+	key = (HMAC_Key *) d_key->dk_KEY_struct;
+
 	if (mode & SIG_MODE_INIT) 
 		ctx = (MD5_CTX *) malloc(sizeof(*ctx));
 	else if (context)
@@ -161,10 +166,6 @@ dst_hmac_md5_verify(const int mode, DST_KEY *d_key, void **context,
 	if (ctx == NULL) 
 		return (-1);
 
-	if (d_key == NULL || d_key->dk_KEY_struct == NULL)
-		return (-1);
-
-	key = (HMAC_Key *) d_key->dk_KEY_struct;
 	if (mode & SIG_MODE_INIT) {
 		MD5Init(ctx);
 		MD5Update(ctx, key->hk_ipad, HMAC_LEN);
@@ -216,8 +217,11 @@ dst_buffer_to_hmac_md5(DST_KEY *dkey, const u_char *key, const unsigned keylen)
 	HMAC_Key *hkey = NULL;
 	MD5_CTX ctx;
 	unsigned local_keylen = keylen;
+	u_char tk[MD5_LEN];
 
-	if (dkey == NULL || key == NULL || keylen < 0)
+	/* Do we need to check if keylen == 0?  The original
+	 * code didn't, so we don't currently */
+	if (dkey == NULL || key == NULL)
 		return (-1);
 
 	if ((hkey = (HMAC_Key *) malloc(sizeof(HMAC_Key))) == NULL)
@@ -228,7 +232,7 @@ dst_buffer_to_hmac_md5(DST_KEY *dkey, const u_char *key, const unsigned keylen)
 
 	/* if key is longer than HMAC_LEN bytes reset it to key=MD5(key) */
 	if (keylen > HMAC_LEN) {
-		u_char tk[MD5_LEN];
+		memset(tk, 0, sizeof(tk));
 		MD5Init(&ctx);
 		MD5Update(&ctx, (const unsigned char *)key, keylen);
 		MD5Final(tk, &ctx);
@@ -269,7 +273,7 @@ dst_hmac_md5_key_to_file_format(const DST_KEY *dkey, char *buff,
 			    const unsigned buff_len)
 {
 	char *bp;
-	int i;
+	int i, res;
 	unsigned len, b_len, key_len;
 	u_char key[HMAC_LEN];
 	HMAC_Key *hkey;
@@ -299,9 +303,10 @@ dst_hmac_md5_key_to_file_format(const DST_KEY *dkey, char *buff,
 	bp += strlen("Key: ");
 	b_len = buff_len - (bp - buff);
 
-	len = b64_ntop(key, key_len, bp, b_len);
-	if (len < 0) 
+	res = b64_ntop(key, key_len, bp, b_len);
+	if (res < 0) 
 		return (-1);
+	len = (unsigned) res;
 	bp += len;
 	*(bp++) = '\n';
 	*bp = '\0';
diff --git a/dst/prandom.c b/dst/prandom.c
index d17aa3e1..ba87094f 100644
--- a/dst/prandom.c
+++ b/dst/prandom.c
@@ -453,12 +453,12 @@ digest_file(dst_work *work)
 	struct timeval tv;
 	u_char buf[1024];
 
+	name = files[f_cnt++]; 
 	if (f_round == 0 || files[f_cnt] == NULL || work->file_digest == NULL) 
 		if (gettimeofday(&tv, NULL)) /* only do this if needed */
 			return (0);
 	if (f_round == 0)   /* first time called set to one hour ago */
 		f_round = (tv.tv_sec - MAX_OLD); 
-	name = files[f_cnt++]; 
 	if (files[f_cnt] == NULL) {  /* end of list of files */
 		if(f_cnt <= 1)       /* list is too short */
 			return (0);
@@ -676,8 +676,10 @@ get_hmac_key(int step, int block)
 	new->step = step;
 	new->block = block;
 	new->key = new_key;
-	if (dst_sign_data(SIG_MODE_INIT, new_key, &new->ctx, NULL, 0, NULL, 0))
+	if (dst_sign_data(SIG_MODE_INIT, new_key, &new->ctx, NULL, 0, NULL, 0)) {
+		SAFE_FREE(new);
 		return (NULL);
+	}
 
 	return (new);
 }
@@ -821,8 +823,10 @@ dst_s_random(u_char *output, unsigned size)
 						    DST_HASH_SIZE *
 						    DST_NUM_HASHES);
 		my_work->file_digest = NULL;
-		if (my_work->output == NULL)
+		if (my_work->output == NULL) {
+			SAFE_FREE(my_work);
 			return (n);
+		}
 		memset(my_work->output, 0x0, my_work->needed);
 /* allocate upto 4 different HMAC hash functions out of order */
 #if DST_NUM_HASHES >= 3
@@ -835,8 +839,17 @@ dst_s_random(u_char *output, unsigned size)
 		my_work->hash[3] = get_hmac_key(5, DST_RANDOM_BLOCK_SIZE / 4);
 #endif
 		my_work->hash[0] = get_hmac_key(1, DST_RANDOM_BLOCK_SIZE);
-		if (my_work->hash[0] == NULL)	/* if failure bail out */
+		if (my_work->hash[0] == NULL) {	/* if failure bail out */
+			for (i = 1; i < DST_NUM_HASHES; i++) {
+				if (my_work->hash[i] != NULL) {
+					dst_free_key(my_work->hash[i]->key);
+					SAFE_FREE(my_work->hash[i]);
+				}
+			}
+			SAFE_FREE(my_work->output);
+			SAFE_FREE(my_work);
 			return (n);
+		}
 		s = own_random(my_work);
 /* if more generated than needed store it for future use */
 		if (s >= my_work->needed) {
@@ -846,6 +859,9 @@ dst_s_random(u_char *output, unsigned size)
 			n += my_work->needed;
 			/* saving unused data for next time */
 			unused = s - my_work->needed;
+			if (unused > sizeof(old_unused)) {
+				unused = sizeof(old_unused);
+			}
 			memcpy(old_unused, &my_work->output[my_work->needed],
 			       unused);
 		} else {
@@ -857,8 +873,10 @@ dst_s_random(u_char *output, unsigned size)
 
 /* delete the allocated work area */
 		for (i = 0; i < DST_NUM_HASHES; i++) {
-			dst_free_key(my_work->hash[i]->key);
-			SAFE_FREE(my_work->hash[i]);
+			if (my_work->hash[i] != NULL) {
+				dst_free_key(my_work->hash[i]->key);
+				SAFE_FREE(my_work->hash[i]);
+			}
 		}
 		SAFE_FREE(my_work->output);
 		SAFE_FREE(my_work);
@@ -892,7 +910,7 @@ dst_s_semi_random(u_char *output, unsigned size)
 	prand_hash *hash;
 	unsigned out = 0;
 	unsigned i;
-	int n;
+	int n, res;
 
 	if (output == NULL || size <= 0)
 		return (-2);
@@ -941,9 +959,13 @@ dst_s_semi_random(u_char *output, unsigned size)
 		for (n = 0; n < DST_NUMBER_OF_COUNTERS; n++)
 			i = (int) counter[n]++;
 
-		i = dst_sign_data(SIG_MODE_ALL, my_key, NULL, 
+		res = dst_sign_data(SIG_MODE_ALL, my_key, NULL, 
 				  (u_char *) counter, hb_size,
 				  semi_old, sizeof(semi_old));
+		if (res < 0) {
+			return res;
+		}
+		i = (unsigned) res;
 		if (i != hb_size)
 			EREPORT(("HMAC SIGNATURE FAILURE %d\n", i));
 		cnt++;
diff --git a/omapip/buffer.c b/omapip/buffer.c
index 4efff9f4..4104cfb7 100644
--- a/omapip/buffer.c
+++ b/omapip/buffer.c
@@ -3,7 +3,8 @@
    Buffer access functions for the object management protocol... */
 
 /*
- * Copyright (c) 2004,2005,2007,2009 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2009,2012 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004,2005,2007 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 1999-2003 by Internet Software Consortium
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -279,9 +280,7 @@ isc_result_t omapi_connection_copyin (omapi_object_t *h,
 	int sig_flags = SIG_MODE_UPDATE;
 	omapi_connection_object_t *c;
 
-	/* Make sure len is valid. */
-	if (len < 0)
-		return DHCP_R_INVALIDARG;
+	/* no need to verify len as it's unsigned */
 	if (!h || h -> type != omapi_type_connection)
 		return DHCP_R_INVALIDARG;
 	c = (omapi_connection_object_t *)h;
diff --git a/omapip/connection.c b/omapip/connection.c
index a9199683..d08524d2 100644
--- a/omapip/connection.c
+++ b/omapip/connection.c
@@ -3,7 +3,7 @@
    Subroutines for dealing with connections. */
 
 /*
- * Copyright (c) 2009-2011 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2009-2012 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 2004,2007 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 1999-2003 by Internet Software Consortium
  *
@@ -396,22 +396,23 @@ static void trace_connect_input (trace_type_t *ttype,
 	/* Find the matching connect object, if there is one. */
 	omapi_array_foreach_begin (omapi_connections,
 				   omapi_connection_object_t, lp) {
-	    for (i = 0; (lp -> connect_list &&
-			 i < lp -> connect_list -> count); i++) {
+	    for (i = 0; (lp->connect_list &&
+			 i < lp->connect_list->count); i++) {
 		    if (!memcmp (&remote.sin_addr,
-				 &lp -> connect_list -> addresses [i].address,
+				 &lp->connect_list->addresses[i].address,
 				 sizeof remote.sin_addr) &&
 			(ntohs (remote.sin_port) ==
-			 lp -> connect_list -> addresses [i].port))
-			lp -> state = omapi_connection_connected;
-			lp -> remote_addr = remote;
-			lp -> remote_addr.sin_family = AF_INET;
-			omapi_addr_list_dereference (&lp -> connect_list, MDL);
-			lp -> index = connect_index;
-			status = omapi_signal_in ((omapi_object_t *)lp,
-						  "connect");
-			omapi_connection_dereference (&lp, MDL);
-			return;
+			 lp->connect_list->addresses[i].port)) {
+			    lp->state = omapi_connection_connected;
+			    lp->remote_addr = remote;
+			    lp->remote_addr.sin_family = AF_INET;
+			    omapi_addr_list_dereference(&lp->connect_list, MDL);
+			    lp->index = connect_index;
+			    status = omapi_signal_in((omapi_object_t *)lp,
+						     "connect");
+			    omapi_connection_dereference (&lp, MDL);
+			    return;
+		    }
 		}
 	} omapi_array_foreach_end (omapi_connections,
 				   omapi_connection_object_t, lp);
@@ -628,7 +629,7 @@ isc_result_t omapi_connection_connect (omapi_object_t *h)
 
 static isc_result_t omapi_connection_connect_internal (omapi_object_t *h)
 {
-	int error;
+	int error = 0;
 	omapi_connection_object_t *c;
 	socklen_t sl;
 	isc_result_t status;
diff --git a/omapip/listener.c b/omapip/listener.c
index 30259eb6..138b0f23 100644
--- a/omapip/listener.c
+++ b/omapip/listener.c
@@ -84,7 +84,14 @@ isc_result_t omapi_listen_addr (omapi_object_t *h,
 	obj = (omapi_listener_object_t *)0;
 	status = omapi_listener_allocate (&obj, MDL);
 	if (status != ISC_R_SUCCESS)
-		return status;
+		/*
+		 * we could simply return here but by going to
+		 * error_exit we keep the code check tools happy
+		 * without removing the NULL check on obj at
+		 * the exit, which we could skip curently but
+		 * might want in the future.
+		 */
+		goto error_exit;
 	obj->socket = -1;
 
 	/* Connect this object to the inner object. */
diff --git a/omapip/protocol.c b/omapip/protocol.c
index 99064898..112bf128 100644
--- a/omapip/protocol.c
+++ b/omapip/protocol.c
@@ -354,7 +354,7 @@ isc_result_t omapi_protocol_signal_handler (omapi_object_t *h,
 	omapi_protocol_object_t *p;
 	omapi_object_t *c;
 	omapi_message_object_t *m;
-	omapi_value_t *signature;
+	omapi_value_t *signature = NULL;
 	u_int16_t nlen;
 	u_int32_t vlen;
 	u_int32_t th;
@@ -680,7 +680,6 @@ isc_result_t omapi_protocol_signal_handler (omapi_object_t *h,
 	      case omapi_protocol_signature_wait:
 		if (p -> message -> id_object) {
 			/* Compute the signature of the message. */
-			signature = (omapi_value_t *)0;
 			status = omapi_get_value_str (c, (omapi_object_t *)0,
 						      "input-signature",
 						      &signature);
@@ -707,7 +706,9 @@ isc_result_t omapi_protocol_signal_handler (omapi_object_t *h,
 					       p -> message -> authlen);
 			
 		if (status != ISC_R_SUCCESS) {
-			omapi_value_dereference (&signature, MDL);
+			if (signature != NULL) {
+				omapi_value_dereference (&signature, MDL);
+			}
 			omapi_disconnect (c, 1);
 			return ISC_R_NOMEMORY;
 		}
@@ -726,7 +727,9 @@ isc_result_t omapi_protocol_signal_handler (omapi_object_t *h,
 			p->verify_result = DHCP_R_INVALIDKEY;
 		}
 
-		omapi_value_dereference (&signature, MDL);
+		if (signature != NULL) {
+			omapi_value_dereference (&signature, MDL);
+		}
 
 		/* Process the message. */
 	      message_done:
@@ -860,10 +863,10 @@ isc_result_t omapi_protocol_set_value (omapi_object_t *h,
 	p = (omapi_protocol_object_t *)h;
 
 	if (omapi_ds_strcmp (name, "default-authenticator") == 0) {
-		if (value -> type != omapi_datatype_object)
+		if (!value || value -> type != omapi_datatype_object)
 			return DHCP_R_INVALIDARG;
 
-		if (!value || !value -> u.object) {
+		if (!value -> u.object) {
 			p -> default_auth = (omapi_remote_auth_t *)0;
 		} else {
 			for (r = p -> remote_auth_list; r; r = r -> next)
@@ -987,7 +990,11 @@ isc_result_t omapi_protocol_configure_security (omapi_object_t *h,
 	l -> verify_auth = verify_auth;
 	l -> insecure = 0;
 
-	return omapi_listener_configure_security (h -> outer, verify_addr);
+	if (h -> outer != NULL) {
+		return omapi_listener_configure_security (h -> outer, verify_addr);
+	} else {
+		return DHCP_R_INVALIDARG;
+	}
 }
 					      
 
diff --git a/omapip/support.c b/omapip/support.c
index c82dcc8c..7768c6e6 100644
--- a/omapip/support.c
+++ b/omapip/support.c
@@ -3,7 +3,8 @@
    Subroutines providing general support for objects. */
 
 /*
- * Copyright (c) 2004-2007,2009 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2009,2012 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004-2007 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 1999-2003 by Internet Software Consortium
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -551,8 +552,14 @@ isc_result_t omapi_object_update (omapi_object_t *obj, omapi_object_t *id,
 		if (status != ISC_R_SUCCESS && status != DHCP_R_UNCHANGED)
 			return status;
 	}
+
+	/*
+	 * For now ignore the return value.  I'm not sure if we want to
+	 * generate an error if we can't set the handle value.  If we
+	 * do add a check we probably should allow unchanged and notfound
+	 */
 	if (handle)
-		omapi_set_int_value (obj, id, "remote-handle", (int)handle);
+		(void) omapi_set_int_value (obj, id, "remote-handle", (int)handle);
 	status = omapi_signal (obj, "updated");
 	if (status != ISC_R_NOTFOUND)
 		return status;
diff --git a/omapip/trace.c b/omapip/trace.c
index 9fd3fb5e..8e203cd4 100644
--- a/omapip/trace.c
+++ b/omapip/trace.c
@@ -5,6 +5,7 @@
    transactions... */
 
 /*
+ * Copyright (c) 2012 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 2009-2010 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 2004-2007 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 2001-2003 by Internet Software Consortium
@@ -235,6 +236,7 @@ isc_result_t trace_write_packet_iov (trace_type_t *ttype,
 
 	/* We have to swap out the data, because it may be read back on a
 	   machine of different endianness. */
+	memset(&tmp, 0, sizeof(tmp));
 	tmp.type_index = htonl (ttype -> index);
 	tmp.when = htonl (time ((time_t *)0)); /* XXX */
 	tmp.length = htonl (length);
@@ -690,27 +692,30 @@ isc_result_t trace_get_file (trace_type_t *ttype,
 	}
 
 	result = trace_get_next_packet (&ttype, tpkt, buf, len, &max);
+	/* done with tpkt, free it */
+	dfree (tpkt, MDL);
 	if (result != ISC_R_SUCCESS) {
-		dfree (tpkt, MDL);
-		if (*buf)
+		if (*buf) {
 			dfree (*buf, MDL);
+			*buf = NULL;
+		}
 		return result;
 	}
 
 	/* Make sure the filename is right. */
 	if (strcmp (filename, *buf)) {
 		log_error ("Read file %s when expecting %s", *buf, filename);
+		dfree (*buf, MDL);
+		*buf = NULL;
+
 		status = fsetpos (traceinfile, &curpos);
 		if (status < 0) {
 			log_error ("fsetpos in tracefile failed: %m");
-			dfree (tpkt, MDL);
-			dfree (*buf, MDL);
 			return DHCP_R_PROTOCOLERROR;
 		}
 		return ISC_R_UNEXPECTEDTOKEN;
 	}
 
-	dfree (tpkt, MDL);
 	return ISC_R_SUCCESS;
 }
 #endif /* TRACING */
diff --git a/relay/dhcrelay.c b/relay/dhcrelay.c
index 5dcec623..771aab5d 100644
--- a/relay/dhcrelay.c
+++ b/relay/dhcrelay.c
@@ -3,7 +3,7 @@
    DHCP/BOOTP Relay Agent. */
 
 /*
- * Copyright(c) 2004-2011 by Internet Systems Consortium, Inc.("ISC")
+ * Copyright(c) 2004-2012 by Internet Systems Consortium, Inc.("ISC")
  * Copyright(c) 1997-2003 by Internet Software Consortium
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -257,13 +257,19 @@ main(int argc, char **argv) {
 			local_family_set = 1;
 			local_family = AF_INET;
 #endif
+			if (++i == argc) {
+				usage();
+			}
+			if (strlen(argv[i]) >= sizeof(tmp->name)) {
+				log_fatal("%s: interface name too long "
+					  "(is %ld)",
+					  argv[i], (long)strlen(argv[i]));
+			}
 			status = interface_allocate(&tmp, MDL);
-			if (status != ISC_R_SUCCESS)
+			if (status != ISC_R_SUCCESS) {
 				log_fatal("%s: interface_allocate: %s",
 					  argv[i],
 					  isc_result_totext(status));
-			if (++i == argc) {
-				usage();
 			}
 			strcpy(tmp->name, argv[i]);
 			interface_snorf(tmp, INTERFACE_REQUESTED);
@@ -608,9 +614,10 @@ do_relay4(struct interface_info *ip, struct dhcp_packet *packet,
 
 			for (i = 0 ; i < out->address_count ; i++ ) {
 				if (out->addresses[i].s_addr ==
-				    packet->giaddr.s_addr)
+				    packet->giaddr.s_addr) {
 					i = -1;
 					break;
+				}
 			}
 
 			if (i == -1)
diff --git a/server/class.c b/server/class.c
index 2f92255e..b8eac87c 100644
--- a/server/class.c
+++ b/server/class.c
@@ -3,7 +3,8 @@
    Handling for client classes. */
 
 /*
- * Copyright (c) 2004,2007,2009 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2009,2012 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004,2007 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 1998-2003 by Internet Software Consortium
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -185,7 +186,7 @@ int check_collection (packet, lease, collection)
 					}
 					memset (nc -> billed_leases, 0,
 						(nc -> lease_limit *
-						 sizeof nc -> billed_leases));
+						 sizeof (struct lease *)));
 				}
 				data_string_copy (&nc -> hash_string, &data,
 						  MDL);
diff --git a/server/confpars.c b/server/confpars.c
index 1c9c4802..1c492343 100644
--- a/server/confpars.c
+++ b/server/confpars.c
@@ -1046,7 +1046,6 @@ void parse_failover_peer (cfile, group, type)
 			if (hba_len != 32) {
 				parse_warn (cfile,
 					    "HBA must be exactly 32 bytes.");
-				dfree (hba, MDL);
 				break;
 			}
 		      make_hba:
@@ -2123,7 +2122,7 @@ int parse_class_declaration (cp, cfile, group, type)
 					log_fatal ("no memory for billing");
 				memset (class -> billed_leases, 0,
 					(class -> lease_limit *
-					 sizeof class -> billed_leases));
+					 sizeof (struct lease *)));
 			}
 			data_string_copy (&class -> hash_string, &data, MDL);
 			if (!pc -> hash &&
@@ -2319,7 +2318,7 @@ int parse_class_declaration (cp, cfile, group, type)
 				log_fatal ("no memory for billed leases.");
 			memset (class -> billed_leases, 0,
 				(class -> lease_limit *
-				 sizeof class -> billed_leases));
+				 sizeof (struct lease *)));
 			have_billing_classes = 1;
 			parse_semi (cfile);
 		} else {
@@ -2383,7 +2382,9 @@ void parse_shared_net_declaration (cfile, group)
 	if (status != ISC_R_SUCCESS)
 		log_fatal ("Can't allocate shared subnet: %s",
 			   isc_result_totext (status));
-	clone_group (&share -> group, group, MDL);
+	if (clone_group (&share -> group, group, MDL) == 0) {
+		log_fatal ("Can't clone group for shared net");
+	}
 	shared_network_reference (&share -> group -> shared_network,
 				  share, MDL);
 
@@ -2794,9 +2795,9 @@ void parse_group_declaration (cfile, group)
 					  val, isc_result_totext(status));
 			group_reference(&t->group, g, MDL);
 			t->name = name;
+			/* no need to include deletedp as it's handled above */
 			t->flags = ((staticp ? GROUP_OBJECT_STATIC : 0) |
-				    (dynamicp ? GROUP_OBJECT_DYNAMIC : 0) |
-				    (deletedp ? GROUP_OBJECT_DELETED : 0));
+				    (dynamicp ? GROUP_OBJECT_DYNAMIC : 0));
 			supersede_group(t, 0);
 		}
 		if (t != NULL)
diff --git a/server/db.c b/server/db.c
index 5be1684a..de185293 100644
--- a/server/db.c
+++ b/server/db.c
@@ -66,10 +66,9 @@ write_binding_scope(FILE *db_file, struct binding *bnd, char *prepend) {
 				errno = 0;
 				fprintf(db_file, "%sset %s = \"%s\";",
 					prepend, bnd->name, s);
+				dfree(s, MDL);
 				if (errno)
 					return ISC_R_FAILURE;
-
-				dfree(s, MDL);
 			} else {
 			    return ISC_R_FAILURE;
 			}
diff --git a/server/ddns.c b/server/ddns.c
index 90217bd9..80418721 100644
--- a/server/ddns.c
+++ b/server/ddns.c
@@ -1093,7 +1093,12 @@ ddns_update_lease_ptr(struct lease    *lease,
 {
 	char ddns_address[MAX_ADDRESS_STRING_LEN];
 	sprintf(ddns_address, "unknown");
-	if (ddns_cb) {
+	if (ddns_cb == NULL) {
+		log_info("%s(%d): No control block for lease update",
+			 file, line);
+		return (ISC_R_FAILURE);
+	}
+	else {
 		strncpy(ddns_address, piaddr(ddns_cb->address), 
 			MAX_ADDRESS_STRING_LEN);
 	}
diff --git a/server/dhcpv6.c b/server/dhcpv6.c
index 12d31fca..f94ea0bd 100644
--- a/server/dhcpv6.c
+++ b/server/dhcpv6.c
@@ -572,6 +572,7 @@ valid_client_info_req(struct packet *packet, struct data_string *server_id) {
 
 	ret_val = 0;
 	memset(server_id, 0, sizeof(*server_id));
+	memset(&client_id, 0, sizeof(client_id));
 
 	/*
 	 * Make a string that we can print out to give more 
@@ -2680,9 +2681,11 @@ find_client_temporaries(struct reply_state *reply) {
 		if (status != ISC_R_SUCCESS) {
 			goto cleanup;
 		}
-		if (reply->lease != NULL) {
-			iasubopt_dereference(&reply->lease, MDL);
-		}
+		/*
+		 * reply->lease can't be null as we use it above
+		 * add check if that changes
+		 */
+		iasubopt_dereference(&reply->lease, MDL);
 	}
 
       cleanup:
diff --git a/server/failover.c b/server/failover.c
index 56b07aa9..e0a416e2 100644
--- a/server/failover.c
+++ b/server/failover.c
@@ -5046,7 +5046,7 @@ failover_lease_is_better(dhcp_failover_state_t *state, struct lease *lease,
 isc_result_t dhcp_failover_process_bind_update (dhcp_failover_state_t *state,
 					       failover_message_t *msg)
 {
-	struct lease *lt, *lease;
+	struct lease *lt = NULL, *lease = NULL;
 	struct iaddr ia;
 	int reason = FTR_MISC_REJECT;
 	const char *message;
@@ -5067,8 +5067,6 @@ isc_result_t dhcp_failover_process_bind_update (dhcp_failover_state_t *state,
 	ia.len = sizeof msg -> assigned_addr;
 	memcpy (ia.iabuf, &msg -> assigned_addr, ia.len);
 
-	lease = (struct lease *)0;
-	lt = (struct lease *)0;
 	if (!find_lease_by_ip_addr (&lease, ia, MDL)) {
 		message = "unknown IP address";
 		reason = FTR_ILLEGAL_IP_ADDR;
diff --git a/server/ldap.c b/server/ldap.c
index 5003228e..8a7d6955 100644
--- a/server/ldap.c
+++ b/server/ldap.c
@@ -1988,7 +1988,7 @@ find_subclass_in_ldap (struct class *class, struct class **newclass,
               return (0);
             }
           memset ((*newclass)->billed_leases, 0, 
-                ((*newclass)->lease_limit * sizeof (*newclass)->billed_leases));
+		  ((*newclass)->lease_limit * sizeof (struct lease *)));
         }
 
       data_string_copy (&(*newclass)->hash_string, data, MDL);
diff --git a/server/mdb.c b/server/mdb.c
index 3867ba58..18378b3b 100644
--- a/server/mdb.c
+++ b/server/mdb.c
@@ -308,16 +308,13 @@ isc_result_t enter_host (hd, dynamicp, commit)
 	/* See if there's a statement that sets the client identifier.
 	   This is a kludge - the client identifier really shouldn't be
 	   set with an executable statement. */
-	esp = (struct executable_statement *)0;
-	if (executable_statement_foreach (hd -> group -> statements,
+	esp = NULL;
+	if (executable_statement_foreach (hd->group->statements,
 					  find_uid_statement, &esp, 0)) {
-		evaluate_option_cache (&hd -> client_identifier,
-				       (struct packet *)0,
-				       (struct lease *)0,
-				       (struct client_state *)0,
-				       (struct option_state *)0,
-				       (struct option_state *)0, &global_scope,
-				       esp -> data.option, MDL);
+		(void) evaluate_option_cache (&hd->client_identifier,
+					      NULL, NULL, NULL, NULL, NULL, 
+					      &global_scope,
+					      esp->data.option, MDL);
 	}
 
 	/* If we got a client identifier, hash this entry by
@@ -1422,10 +1419,11 @@ int supersede_lease (comp, lease, commit, propogate, pimmediate)
 
 void make_binding_state_transition (struct lease *lease)
 {
+
 #if defined (FAILOVER_PROTOCOL)
 	dhcp_failover_state_t *peer;
 
-	if (lease && lease -> pool && lease -> pool -> failover_peer)
+	if (lease -> pool && lease -> pool -> failover_peer)
 		peer = lease -> pool -> failover_peer;
 	else
 		peer = (dhcp_failover_state_t *)0;
@@ -1572,17 +1570,18 @@ void make_binding_state_transition (struct lease *lease)
 	      case FTS_RELEASED:
 	      case FTS_ABANDONED:
 	      case FTS_RESET:
-		lease -> next_binding_state = FTS_FREE;
+		lease->next_binding_state = FTS_FREE;
 #if defined(FAILOVER_PROTOCOL)
 		/* If we are not in partner_down, leases don't go from
 		   EXPIRED to FREE on a timeout - only on an update.
 		   If we're in partner_down, they expire at mclt past
 		   the time we entered partner_down. */
-		if (lease -> pool -> failover_peer &&
-		    lease -> pool -> failover_peer -> me.state == partner_down)
-			lease -> tsfp =
-			    (lease -> pool -> failover_peer -> me.stos +
-			     lease -> pool -> failover_peer -> mclt);
+		if ((lease->pool != NULL) &&
+		    (lease->pool->failover_peer != NULL) &&
+		    (lease->pool->failover_peer->me.state == partner_down))
+			lease->tsfp =
+			    (lease->pool->failover_peer->me.stos +
+			     lease->pool->failover_peer->mclt);
 #endif /* FAILOVER_PROTOCOL */
 		break;
 
diff --git a/server/omapi.c b/server/omapi.c
index 19b3847c..5901cc55 100644
--- a/server/omapi.c
+++ b/server/omapi.c
@@ -1690,7 +1690,6 @@ isc_result_t dhcp_pool_signal_handler (omapi_object_t *h,
 {
 	/* h should point to (struct pool *) */
 	isc_result_t status;
-	int updatep = 0;
 
 	if (h -> type != dhcp_type_pool)
 		return DHCP_R_INVALIDARG;
@@ -1704,8 +1703,7 @@ isc_result_t dhcp_pool_signal_handler (omapi_object_t *h,
 		if (status == ISC_R_SUCCESS)
 			return status;
 	}
-	if (updatep)
-		return ISC_R_SUCCESS;
+
 	return ISC_R_NOTFOUND;
 }