In dhclient check the data for some string options for

reasonableness before passing it along to the script that
interfaces with the OS. [ISC-Bugs #23722] CVE: CVE-2011-0997

3 files changed, 173 insertions, 17 deletions

diff --git a/RELNOTES b/RELNOTES
index be6f2edd..bffc04c4 100644
--- a/RELNOTES
+++ b/RELNOTES
@@ -49,6 +49,12 @@ the README file.
 - The 'domain-list' atom documentation in common/dhcp-options.5 was
   corrected.  [ISC-Bugs #21217]
 
+! In dhclient check the data for some string options for
+  reasonableness before passing it along to the script that
+  interfaces with the OS.
+  [ISC-Bugs #23722]
+  CVE: CVE-2011-0997
+
 			Changes since 3.1.3
 
 - Modified the handling of a connection to avoid releasing the omapi io
diff --git a/client/dhclient.c b/client/dhclient.c
index a083a31f..3f2698f0 100644
--- a/client/dhclient.c
+++ b/client/dhclient.c
@@ -3,7 +3,7 @@
    DHCP Client. */
 
 /*
- * Copyright (c) 2004-2010 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004-2011 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 1995-2003 by Internet Software Consortium
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -32,7 +32,7 @@
 
 #ifndef lint
 static char ocopyright[] =
-"$Id: dhclient.c,v 1.143.2.15 2010/03/17 19:32:12 sar Exp $ Copyright (c) 2004-2009 Internet Systems Consortium.  All rights reserved.\n";
+"$Id: dhclient.c,v 1.143.2.16 2011/03/24 22:43:32 sar Exp $ Copyright (c) 2004-2009 Internet Systems Consortium.  All rights reserved.\n";
 #endif /* not lint */
 
 #include "dhcpd.h"
@@ -79,6 +79,11 @@ int nowait=0;
 
 static void usage PROTO ((void));
 
+static int check_domain_name(const char *ptr, size_t len, int dots);
+static int check_domain_name_list(const char *ptr, size_t len, int dots);
+static int check_option_values(struct universe *universe, unsigned int opt,
+			       const char *ptr, size_t len);
+
 int main (argc, argv, envp)
 	int argc;
 	char **argv, **envp;
@@ -2504,13 +2509,27 @@ void client_option_envadd (struct option_cache *oc,
 		if (data.len) {
 			char name [256];
 			if (dhcp_option_ev_name (name, sizeof name,
-						 oc -> option)) {
-				client_envadd (es -> client, es -> prefix,
-					       name, "%s",
-					       (pretty_print_option
-						(oc -> option,
-						 data.data, data.len,
-						 0, 0)));
+						 oc->option)) {
+				const char *value;
+				value = pretty_print_option(oc->option,
+							    data.data,
+							    data.len, 0, 0);
+				size_t length = strlen(value);
+
+				if (check_option_values(oc->option->universe,
+							oc->option->code,
+							value, length) == 0) {
+					client_envadd(es->client, es->prefix,
+						      name, "%s", value);
+				} else {
+					log_error("suspect value in %s "
+						  "option - discarded",
+						  name);
+				}
+
+
+
+
 				data_string_forget (&data, MDL);
 			}
 		}
@@ -2590,12 +2609,31 @@ void script_write_params (client, prefix, lease)
 		data_string_forget (&data, MDL);
 	}
 
-	if (lease -> filename)
-		client_envadd (client,
-			       prefix, "filename", "%s", lease -> filename);
-	if (lease -> server_name)
-		client_envadd (client, prefix, "server_name",
-			       "%s", lease -> server_name);
+	if (lease->filename) {
+		if (check_option_values(NULL, DHO_ROOT_PATH,
+					lease->filename,
+					strlen(lease->filename)) == 0) {
+			client_envadd(client, prefix, "filename",
+				      "%s", lease->filename);
+		} else {
+			log_error("suspect value in %s "
+				  "option - discarded",
+				  lease->filename);
+		}
+	}
+
+	if (lease->server_name) {
+		if (check_option_values(NULL, DHO_HOST_NAME,
+					lease->server_name,
+					strlen(lease->server_name)) == 0 ) {
+			client_envadd (client, prefix, "server_name",
+				       "%s", lease->server_name);
+		} else {
+			log_error("suspect value in %s "
+				  "option - discarded",
+				  lease->server_name);
+		}
+	}
 
 	for (i = 0; i < lease -> options -> universe_count; i++) {
 		option_space_foreach ((struct packet *)0, (struct lease *)0,
@@ -3219,3 +3257,113 @@ isc_result_t client_dns_update (struct client_state *client, int addp, int ttl)
 	data_string_forget (&ddns_dhcid, MDL);
 	return rcode;
 }
+
+/*
+ * The following routines are used to check that certain
+ * strings are reasonable before we pass them to the scripts.
+ * This avoids some problems with scripts treating the strings
+ * as commands - see ticket 23722
+ * The domain checking code should be done as part of assembling
+ * the string but we are doing it here for now due to time
+ * constraints.
+ */
+
+static int check_domain_name(const char *ptr, size_t len, int dots)
+{
+	const char *p;
+
+	/* not empty or complete length not over 255 characters   */
+	if ((len == 0) || (len > 256))
+		return(-1);
+
+	/* consists of [[:alnum:]-]+ labels separated by [.]      */
+	/* a [_] is against RFC but seems to be "widely used"...  */
+	for (p=ptr; (*p != 0) && (len-- > 0); p++) {
+		if ((*p == '-') || (*p == '_')) {
+			/* not allowed at begin or end of a label */
+			if (((p - ptr) == 0) || (len == 0) || (p[1] == '.'))
+				return(-1);
+		} else if (*p == '.') {
+			/* each label has to be 1-63 characters;
+			   we allow [.] at the end ('foo.bar.')   */
+			size_t d = p - ptr;
+			if ((d <= 0) || (d >= 64))
+				return(-1);
+			ptr = p + 1; /* jump to the next label    */
+			if ((dots > 0) && (len > 0))
+				dots--;
+		} else if (isalnum((unsigned char)*p) == 0) {
+			/* also numbers at the begin are fine     */
+			return(-1);
+		}
+	}
+	return(dots ? -1 : 0);
+}
+
+static int check_domain_name_list(const char *ptr, size_t len, int dots)
+{
+	const char *p;
+	int ret = -1; /* at least one needed */
+
+	if ((ptr == NULL) || (len == 0))
+		return(-1);
+
+	for (p=ptr; (*p != 0) && (len > 0); p++, len--) {
+		if (*p != ' ')
+			continue;
+		if (p > ptr) {
+			if (check_domain_name(ptr, p - ptr, dots) != 0)
+				return(-1);
+			ret = 0;
+		}
+		ptr = p + 1;
+	}
+	if (p > ptr)
+		return(check_domain_name(ptr, p - ptr, dots));
+	else
+		return(ret);
+}
+
+static int check_option_values(struct universe *universe,
+			       unsigned int opt,
+			       const char *ptr,
+			       size_t len)
+{
+	if (ptr == NULL)
+		return(-1);
+
+	/* just reject options we want to protect, will be escaped anyway */
+	if ((universe == NULL) || (universe == &dhcp_universe)) {
+		switch(opt) {
+		      case DHO_HOST_NAME:
+		      case DHO_DOMAIN_NAME:
+		      case DHO_NIS_DOMAIN:
+		      case DHO_NETBIOS_SCOPE:
+			return check_domain_name(ptr, len, 0);
+			break;
+		      case DHO_DOMAIN_SEARCH:
+			return check_domain_name_list(ptr, len, 0);
+			break;
+		      case DHO_ROOT_PATH:
+			if (len == 0)
+				return(-1);
+			for (; (*ptr != 0) && (len-- > 0); ptr++) {
+				if(!(isalnum((unsigned char)*ptr) ||
+				     *ptr == '#'  || *ptr == '%' ||
+				     *ptr == '+'  || *ptr == '-' ||
+				     *ptr == '_'  || *ptr == ':' ||
+				     *ptr == '.'  || *ptr == ',' ||
+				     *ptr == '@'  || *ptr == '~' ||
+				     *ptr == '\\' || *ptr == '/' ||
+				     *ptr == '['  || *ptr == ']' ||
+				     *ptr == '='  || *ptr == ' '))
+					return(-1);
+			}
+			return(0);
+			break;
+		}
+	}
+
+	return(0);
+}
+
diff --git a/common/options.c b/common/options.c
index fde38018..43fce2cd 100644
--- a/common/options.c
+++ b/common/options.c
@@ -34,7 +34,7 @@
 
 #ifndef lint
 static char copyright[] =
-"$Id: options.c,v 1.98.2.21 2009/09/01 20:32:27 dhankins Exp $ Copyright (c) 2004-2009 Internet Systems Consortium.  All rights reserved.\n";
+"$Id: options.c,v 1.98.2.22 2011/03/24 22:43:32 sar Exp $ Copyright (c) 2004-2009 Internet Systems Consortium.  All rights reserved.\n";
 #endif /* not lint */
 
 #define DHCP_OPTION_DATA
@@ -2947,7 +2947,9 @@ pretty_escape(char **dst, char *dend, const unsigned char **src,
 				count += 4;
 			}
 		} else if (**src == '"' || **src == '\'' || **src == '$' ||
-			   **src == '`' || **src == '\\') {
+			   **src == '`' || **src == '\\' || **src == '|' ||
+			   **src == '&') {
+
 			if (*dst + 2 > dend)
 				return -1;