Do not accept dhclient.conf(5) "prepend" or "append" statements when

the option data cannot be prepended or appended to. Instead, treat
"prepend" as "supersede" and "append" as "default". This preserves the
safe aspects of current behaviour. Issue a parsing warning when
appropriate to encourage people to fix their configuration files.

Eliminate egregious repeated code by abstracting merge_option_data().

5 files changed, 90 insertions, 62 deletions

diff --git a/sbin/dhclient/clparse.c b/sbin/dhclient/clparse.c
index 24ee95634fa..6998056e8fb 100644
--- a/sbin/dhclient/clparse.c
+++ b/sbin/dhclient/clparse.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: clparse.c,v 1.183 2019/02/12 16:50:44 krw Exp $	*/
+/*	$OpenBSD: clparse.c,v 1.184 2019/03/20 20:10:00 krw Exp $	*/
 
 /* Parser for dhclient config and lease files. */
 
@@ -257,7 +257,7 @@ parse_conf_decl(FILE *cfile, char *name)
 {
 	uint8_t			 list[DHO_COUNT];
 	char			*val;
-	int			 i, count, token;
+	int			 action, count, i, token;
 	uint32_t		 t;
 
 	token = next_token(NULL, cfile);
@@ -265,7 +265,10 @@ parse_conf_decl(FILE *cfile, char *name)
 	switch (token) {
 	case TOK_APPEND:
 		if (parse_option(cfile, &i, config->defaults) == 1) {
-			config->default_actions[i] = ACTION_APPEND;
+			action = code_to_action(i, ACTION_APPEND);
+			if (action == ACTION_DEFAULT)
+				parse_warn("'append' treated as 'default'");
+			config->default_actions[i] = action;
 			parse_semi(cfile);
 		}
 		break;
@@ -356,7 +359,10 @@ parse_conf_decl(FILE *cfile, char *name)
 		break;
 	case TOK_PREPEND:
 		if (parse_option(cfile, &i, config->defaults) == 1) {
-			config->default_actions[i] = ACTION_PREPEND;
+			action = code_to_action(i, ACTION_PREPEND);
+			if (action == ACTION_SUPERSEDE)
+				parse_warn("'prepend' treated as 'supersede'");
+			config->default_actions[i] = action;
 			parse_semi(cfile);
 		}
 		break;
diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
index 6e78491cbe7..0fd9a2a62a4 100644
--- a/sbin/dhclient/dhclient.c
+++ b/sbin/dhclient/dhclient.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dhclient.c,v 1.628 2019/03/19 00:50:11 krw Exp $	*/
+/*	$OpenBSD: dhclient.c,v 1.629 2019/03/20 20:10:00 krw Exp $	*/
 
 /*
  * Copyright 2004 Henning Brauer <henning@openbsd.org>
@@ -2244,6 +2244,7 @@ get_ifname(struct interface_info *ifi, int ioctlfd, char *arg)
 struct client_lease *
 apply_defaults(struct client_lease *lease)
 {
+	struct option_data	 emptyopt = {0, NULL};
 	struct client_lease	*newlease;
 	int			 i;
 
@@ -2273,59 +2274,24 @@ apply_defaults(struct client_lease *lease)
 			break;
 
 		case ACTION_SUPERSEDE:
-			free(newlease->options[i].data);
-			newlease->options[i].len = config->defaults[i].len;
-			newlease->options[i].data = calloc(1,
-			    config->defaults[i].len);
-			if (newlease->options[i].data == NULL)
-				goto cleanup;
-			memcpy(newlease->options[i].data,
-			    config->defaults[i].data, config->defaults[i].len);
+			merge_option_data(&config->defaults[i], &emptyopt,
+			    &newlease->options[i]);
 			break;
 
 		case ACTION_PREPEND:
-			free(newlease->options[i].data);
-			newlease->options[i].len = config->defaults[i].len +
-			    lease->options[i].len;
-			newlease->options[i].data = calloc(1,
-			    newlease->options[i].len);
-			if (newlease->options[i].data == NULL)
-				goto cleanup;
-			memcpy(newlease->options[i].data,
-			    config->defaults[i].data, config->defaults[i].len);
-			memcpy(newlease->options[i].data +
-			    config->defaults[i].len, lease->options[i].data,
-			    lease->options[i].len);
+			merge_option_data(&config->defaults[i],
+			    &lease->options[i], &newlease->options[i]);
 			break;
 
 		case ACTION_APPEND:
-			free(newlease->options[i].data);
-			newlease->options[i].len = config->defaults[i].len +
-			    lease->options[i].len;
-			newlease->options[i].data = calloc(1,
-			    newlease->options[i].len);
-			if (newlease->options[i].data == NULL)
-				goto cleanup;
-			memcpy(newlease->options[i].data,
-			    lease->options[i].data, lease->options[i].len);
-			memcpy(newlease->options[i].data +
-			    lease->options[i].len, config->defaults[i].data,
-			    config->defaults[i].len);
+			merge_option_data(&lease->options[i],
+			    &config->defaults[i], &newlease->options[i]);
 			break;
 
 		case ACTION_DEFAULT:
-			if ((newlease->options[i].len == 0) &&
-			    (config->defaults[i].len != 0)) {
-				newlease->options[i].len =
-				    config->defaults[i].len;
-				newlease->options[i].data = calloc(1,
-				    config->defaults[i].len);
-				if (newlease->options[i].data == NULL)
-					goto cleanup;
-				memcpy(newlease->options[i].data,
-				    config->defaults[i].data,
-				    config->defaults[i].len);
-			}
+			if (newlease->options[i].len == 0)
+				merge_option_data(&config->defaults[i],
+				    &emptyopt, &newlease->options[i]);
 			break;
 
 		default:
@@ -2353,15 +2319,6 @@ apply_defaults(struct client_lease *lease)
 	}
 
 	return newlease;
-
-cleanup:
-
-	free_client_lease(newlease);
-
-	fatalx("unable to apply defaults");
-	/* NOTREACHED */
-
-	return NULL;
 }
 
 struct client_lease *
diff --git a/sbin/dhclient/dhclient.conf.5 b/sbin/dhclient/dhclient.conf.5
index 8f3b701d956..2457d04c8f9 100644
--- a/sbin/dhclient/dhclient.conf.5
+++ b/sbin/dhclient/dhclient.conf.5
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: dhclient.conf.5,v 1.45 2019/01/26 22:55:10 krw Exp $
+.\"	$OpenBSD: dhclient.conf.5,v 1.46 2019/03/20 20:10:00 krw Exp $
 .\"
 .\" Copyright (c) 1997 The Internet Software Consortium.
 .\" All rights reserved.
@@ -36,7 +36,7 @@
 .\" see ``http://www.isc.org/isc''.  To learn more about Vixie
 .\" Enterprises, see ``http://www.vix.com''.
 .\"
-.Dd $Mdocdate: January 26 2019 $
+.Dd $Mdocdate: March 20 2019 $
 .Dt DHCLIENT.CONF 5
 .Os
 .Sh NAME
@@ -112,6 +112,12 @@ or
 .Ic supersede
 for
 .Ar option .
+.Pp
+If the option's data cannot be appended to,  e.g. it is an integer value,
+then
+.Ic append
+will be treated as
+.Ic default.
 .It Ic default Ar option option-value ;
 If no value for
 .Ar option
@@ -166,6 +172,12 @@ or
 .Ic supersede
 for
 .Ar option .
+.Pp
+If the option's data cannot be prepended to,  e.g. it is an integer value,
+then
+.Ic prepend
+will be treated as
+.Ic supersede .
 .It Ic request Op Ar option , ... ;
 Ask that any lease contain values
 for the listed options.
diff --git a/sbin/dhclient/dhcpd.h b/sbin/dhclient/dhcpd.h
index 1f0473ff2b2..766c552be33 100644
--- a/sbin/dhclient/dhcpd.h
+++ b/sbin/dhclient/dhcpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dhcpd.h,v 1.274 2019/03/18 22:26:56 krw Exp $	*/
+/*	$OpenBSD: dhcpd.h,v 1.275 2019/03/20 20:10:00 krw Exp $	*/
 
 /*
  * Copyright (c) 2004 Henning Brauer <henning@openbsd.org>
@@ -162,7 +162,10 @@ char			*pretty_print_option(unsigned int, struct option_data *,
 char			*pretty_print_string(unsigned char *, size_t, int);
 char			*code_to_name(int);
 char			*code_to_format(int);
+int			 code_to_action(int, int);
 int			 name_to_code(char *);
+void			 merge_option_data(struct option_data *,
+    struct option_data *, struct option_data *);
 
 /* conflex.c */
 extern int	 lexline, lexchar;
diff --git a/sbin/dhclient/options.c b/sbin/dhclient/options.c
index 572cc02415e..1797a0390cd 100644
--- a/sbin/dhclient/options.c
+++ b/sbin/dhclient/options.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: options.c,v 1.114 2019/03/18 22:26:56 krw Exp $	*/
+/*	$OpenBSD: options.c,v 1.115 2019/03/20 20:10:00 krw Exp $	*/
 
 /* DHCP options parsing and reassembly. */
 
@@ -83,6 +83,7 @@ int expand_search_domain_name(unsigned char *, size_t, int *, unsigned char *);
  * f - flag (true or false)
  * A - array of whatever precedes (e.g., IA means array of IP addresses)
  * C - CIDR description
+ * X - hex octets
  */
 
 static const struct {
@@ -401,6 +402,41 @@ code_to_format(int code)
 }
 
 /*
+ * Some option data types cannot be appended or prepended to. For
+ * such options change ACTION_PREPEND to ACTION_SUPERSEDE and
+ * ACTION_APPEND to ACTION_DEFAULT.
+ */
+int
+code_to_action(int code, int action)
+{
+	char	*fmt;
+
+	fmt = code_to_format(code);
+	if (fmt == NULL || strchr(fmt, 'A') != NULL || fmt[0] == 't')
+		return action;
+
+	if (fmt[0] == 'X' && code != DHO_DOMAIN_SEARCH)
+		return action;
+
+	/*
+	 * For our protection all formats which have been excluded shall be
+	 * deemed included.
+	 */
+	switch (action) {
+	case ACTION_APPEND:
+		action = ACTION_DEFAULT;
+		break;
+	case ACTION_PREPEND: 
+		action = ACTION_SUPERSEDE;
+		break;
+	default:
+		break;
+	}
+
+	return action;
+}
+
+/*
  * Parse options out of the specified buffer, storing addresses of
  * option values in options. Return 0 if errors, 1 if not.
  */
@@ -995,3 +1031,17 @@ unpack_options(struct dhcp_packet *packet)
 
 	return options;
 }
+
+void
+merge_option_data(struct option_data *first,
+    struct option_data *second, struct option_data *dest)
+{
+	free(dest->data);
+	dest->len = first->len + second->len;
+	dest->data = calloc(1, dest->len);
+	if (dest->data == NULL)
+		fatal("merged option data");
+
+	memcpy(dest->data, first->data, first->len);
+	memcpy(dest->data + first->len, second->data, second->len);
+}