First convert packet to lease, which validates option data and

discards bad options. THEN check to see if any required options are
missing and reject both OFFER and ACK packets that lack required
options. Since it is the latter's lease we actual bind.

Move required option check into packet_to_lease() instead of
duplicating it.

1 files changed, 19 insertions, 19 deletions

diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
index 18a9e14b153..1169fad774c 100644
--- a/sbin/dhclient/dhclient.c
+++ b/sbin/dhclient/dhclient.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dhclient.c,v 1.204 2013/01/13 22:09:38 krw Exp $	*/
+/*	$OpenBSD: dhclient.c,v 1.205 2013/01/14 02:46:29 krw Exp $	*/
 
 /*
  * Copyright 2004 Henning Brauer <henning@openbsd.org>
@@ -638,7 +638,7 @@ dhcpack(struct in_addr client_addr, struct option_data *options)
 
 	lease = packet_to_lease(client_addr, options);
 	if (!lease) {
-		note("packet_to_lease failed.");
+		note("DHCPACK isn't satisfactory.");
 		return;
 	}
 
@@ -793,7 +793,6 @@ void
 dhcpoffer(struct in_addr client_addr, struct option_data *options)
 {
 	struct client_lease *lease, *lp;
-	int i;
 	time_t stop_selecting;
 	char *name = options[DHO_DHCP_MESSAGE_TYPE].len ? "DHCPOFFER" :
 	    "BOOTREPLY";
@@ -801,33 +800,24 @@ dhcpoffer(struct in_addr client_addr, struct option_data *options)
 	if (client->state != S_SELECTING)
 		return;
 
-	/* If this lease doesn't supply the minimum required parameters,
-	   blow it off. */
-	for (i = 0; i < config->required_option_count; i++) {
-		if (!options[config->required_options[i]].len) {
-			note("%s isn't satisfactory.", name);
-			return;
-		}
+	lease = packet_to_lease(client_addr, options);
+	if (!lease) {
+		note("%s isn't satisfactory.", name);
+		return;
 	}
 
 	/* If we've already seen this lease, don't record it again. */
-	for (lease = client->offered_leases;
-	    lease; lease = lease->next) {
-		if (!memcmp(&lease->address.s_addr, &client->packet.yiaddr,
+	for (lp = client->offered_leases; lp; lp = lp->next) {
+		if (!memcmp(&lp->address.s_addr, &client->packet.yiaddr,
 		    sizeof(in_addr_t))) {
 #ifdef DEBUG
 			debug("%s already seen.", name);
 #endif
+			free_client_lease(lease);
 			return;
 		}
 	}
 
-	lease = packet_to_lease(client_addr, options);
-	if (!lease) {
-		note("packet_to_lease failed.");
-		return;
-	}
-
 	/*
 	 * Reject offers whose subnet is already configured on another
 	 * interface.
@@ -921,6 +911,16 @@ packet_to_lease(struct in_addr client_addr, struct option_data *options)
 		options[i].len = 0;
 	}
 
+	/*
+	 * If this lease doesn't supply a required parameter, blow it off.
+	 */
+	for (i = 0; i < config->required_option_count; i++) {
+		if (!lease->options[config->required_options[i]].len) {
+			free_client_lease(lease);
+			return (NULL);
+		}
+	}
+
 	memcpy(&lease->address.s_addr, &client->packet.yiaddr,
 	    sizeof(in_addr_t));