Make parse_string() toe the new line by returning

0 for failure, 1 for success, emitting a
single error message ("expecting string.") and
handling ';' better.

Don't leak memory when encountering pathological
config or lease files containing repeated instances
of an option or command with string data.

3 files changed, 60 insertions, 54 deletions

diff --git a/sbin/dhclient/clparse.c b/sbin/dhclient/clparse.c
index 36f2d8bb64b..2bf99581d32 100644
--- a/sbin/dhclient/clparse.c
+++ b/sbin/dhclient/clparse.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: clparse.c,v 1.134 2017/10/11 18:29:30 krw Exp $	*/
+/*	$OpenBSD: clparse.c,v 1.135 2017/10/12 13:10:13 krw Exp $	*/
 
 /* Parser for dhclient config and lease files. */
 
@@ -221,7 +221,7 @@ void
 parse_client_statement(FILE *cfile, char *name)
 {
 	uint8_t		 optlist[DHO_COUNT];
-	char		*string;
+	char		*val;
 	int		 code, count, token;
 
 	token = next_token(NULL, cfile);
@@ -313,16 +313,18 @@ parse_client_statement(FILE *cfile, char *name)
 		parse_reject_statement(cfile);
 		break;
 	case TOK_FILENAME:
-		string = parse_string(cfile, NULL);
-		free(config->filename);
-		config->filename = string;
-		parse_semi(cfile);
+		if (parse_string(cfile, NULL, &val) == 1) {
+			free(config->filename);
+			config->filename = val;
+			parse_semi(cfile);
+		}
 		break;
 	case TOK_SERVER_NAME:
-		string = parse_string(cfile, NULL);
-		free(config->server_name);
-		config->server_name = string;
-		parse_semi(cfile);
+		if (parse_string(cfile, NULL, &val) == 1) {
+			free(config->server_name);
+			config->server_name = val;
+			parse_semi(cfile);
+		}
 		break;
 	case TOK_FIXED_ADDR:
 		if (parse_ip_addr(cfile, &config->address) == 1)
@@ -550,7 +552,7 @@ parse_client_lease_declaration(FILE *cfile, struct client_lease *lease,
 {
 	char		*val;
 	unsigned int	 len;
-	int		 token;
+	int		 rslt, token;
 
 	token = next_token(&val, cfile);
 
@@ -559,18 +561,15 @@ parse_client_lease_declaration(FILE *cfile, struct client_lease *lease,
 		/* 'bootp' is just a comment. See BOOTP_LEASE(). */
 		break;
 	case TOK_INTERFACE:
-		token = next_token(&val, cfile);
-		if (token != TOK_STRING) {
-			parse_warn("expecting string.");
-			if (token != ';')
-				skip_to_semi(cfile);
+		if (parse_string(cfile, NULL, &val) == 0)
 			return;
-		}
-		if (strcmp(name, val) != 0) {
+		rslt = strcmp(name, val);
+		free(val);
+		if (rslt != 0) {
 			if (lease->is_static == 0)
 				parse_warn("wrong interface name.");
-			skip_to_semi(cfile);
-			return;
+			    skip_to_semi(cfile);
+			    return;
 		}
 		break;
 	case TOK_FIXED_ADDR:
@@ -582,19 +581,28 @@ parse_client_lease_declaration(FILE *cfile, struct client_lease *lease,
 			return;
 		break;
 	case TOK_FILENAME:
-		lease->filename = parse_string(cfile, NULL);
+		if (parse_string(cfile, NULL, &val) == 0)
+			return;
+		free(lease->filename);
+		lease->filename = val;
 		break;
 	case TOK_SERVER_NAME:
-		lease->server_name = parse_string(cfile, NULL);
+		if (parse_string(cfile, NULL, &val) == 0)
+			return;
+		free(lease->server_name);
+		lease->server_name = val;
 		break;
 	case TOK_SSID:
-		val = parse_string(cfile, &len);
-		if (val && len <= sizeof(lease->ssid)) {
-			memset(lease->ssid, 0, sizeof(lease->ssid));
-			memcpy(lease->ssid, val, len);
-			lease->ssid_len = len;
+		if (parse_string(cfile, &len, &val) == 0)
+			return;
+		if (len > sizeof(lease->ssid)) {
+			parse_warn("ssid > 32 bytes");
+			skip_to_semi(cfile);
+			return;
 		}
-		free(val);
+		memset(lease->ssid, 0, sizeof(lease->ssid));
+		memcpy(lease->ssid, val, len);
+		lease->ssid_len = len;
 		break;
 	case TOK_RENEW:
 		if (parse_date(cfile, &lease->renewal) == 0)
@@ -667,8 +675,7 @@ parse_option_decl(FILE *cfile, struct option_data *options)
 				dp = NULL;
 				break;
 			case 't': /* Text string. */
-				val = parse_string(cfile, &len);
-				if (val == NULL)
+				if (parse_string(cfile, &len, &val) == 0)
 					return -1;
 				if (hunkix + len + 1 > sizeof(hunkbuf)) {
 					parse_warn("option data buffer "
diff --git a/sbin/dhclient/dhcpd.h b/sbin/dhclient/dhcpd.h
index b6dfc022ac2..971b0fd540f 100644
--- a/sbin/dhclient/dhcpd.h
+++ b/sbin/dhclient/dhcpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: dhcpd.h,v 1.230 2017/10/11 15:06:27 krw Exp $	*/
+/*	$OpenBSD: dhcpd.h,v 1.231 2017/10/12 13:10:13 krw Exp $	*/
 
 /*
  * Copyright (c) 2004 Henning Brauer <henning@openbsd.org>
@@ -179,7 +179,7 @@ int		 peek_token(char **, FILE *);
 /* parse.c */
 void		 skip_to_semi(FILE *);
 int		 parse_semi(FILE *);
-char		*parse_string(FILE *, unsigned int *);
+int		 parse_string(FILE *, unsigned int *, char **);
 int		 parse_ip_addr(FILE *, struct in_addr *);
 int		 parse_cidr(FILE *, unsigned char *);
 int		 parse_lease_time(FILE *, time_t *);
diff --git a/sbin/dhclient/parse.c b/sbin/dhclient/parse.c
index 91b8e55eed9..44e72335bf9 100644
--- a/sbin/dhclient/parse.c
+++ b/sbin/dhclient/parse.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.c,v 1.71 2017/10/11 22:57:00 krw Exp $	*/
+/*	$OpenBSD: parse.c,v 1.72 2017/10/12 13:10:13 krw Exp $	*/
 
 /* Common parser code for dhcpd and dhclient. */
 
@@ -128,34 +128,33 @@ parse_semi(FILE *cfile)
 	return 1;
 }
 
-char *
-parse_string(FILE *cfile, unsigned int *len)
+int
+parse_string(FILE *cfile, unsigned int *len, char **string)
 {
 	static char	 unvisbuf[1500];
-	char		*val, *s;
+	char		*val;
 	int		 i, token;
 
 	token = next_token(&val, cfile);
-	if (token != TOK_STRING) {
-		parse_warn("expecting string.");
-		if (token != ';')
-			skip_to_semi(cfile);
-		return NULL;
+	if (token == TOK_STRING) {
+		i = strnunvis(unvisbuf, val, sizeof(unvisbuf));
+		if (i >= 0) {
+			*string = malloc(i+1);
+			if (*string == NULL)
+				fatal("unvis string %s", val);
+			memcpy(*string, unvisbuf, i+1);	/* Copy the NUL. */
+			if (len != NULL)
+				*len = i;
+			return 1;
+		}
 	}
 
-	i = strnunvis(unvisbuf, val, sizeof(unvisbuf));
-	if (i == -1) {
-		parse_warn("could not unvis string");
-		return NULL;
-	}
-	s = malloc(i+1);
-	if (s == NULL)
-		fatal("unvis string %s", val);
-	memcpy(s, unvisbuf, i+1);	/* Copy the terminating NUL. */
-	if (len != NULL)
-		*len = i;
-
-	return s;
+	parse_warn("expecting string.");
+
+	if (token != ';')
+		skip_to_semi(cfile);
+
+	return 0;
 }
 
 /*