From: Roy Marples <roy@marples.name>
Date: Wed, 6 Apr 2011 08:16:32 +0000 (+0000)
Subject: Escape | and & characters when passing to the shell.
X-Git-Tag: v5.2.12~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=66fedad6625f9cb6add84ac459f38fdc6397c52d;p=thirdparty%2Fdhcpcd.git

Escape | and & characters when passing to the shell.
Add functions for hooks to check validity of domain names and paths.
Ensure we set a valid hostname, DNS domain and NIS domain.
Document the need for input validation in dhcpcd-run-hooks(8).

Fixes CVE-2011-996.
Based on patches to dhcpcd-3 by Marius Tomaschewski <mt@suse.de>.
---

diff --git a/dhcp.c b/dhcp.c
index d0a9e129..2def2489 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -1143,7 +1143,9 @@ print_string(char *s, ssize_t len, int dl, const uint8_t *data)
 		case '\'': /* FALLTHROUGH */
 		case '$':  /* FALLTHROUGH */
 		case '`':  /* FALLTHROUGH */
- 		case '\\':
+ 		case '\\': /* FALLTHROUGH */
+		case '|':  /* FALLTHROUGH */
+		case '&':
 			if (s) {
 				if (len < 3) {
 					errno = ENOBUFS;
diff --git a/dhcpcd-hooks/20-resolv.conf b/dhcpcd-hooks/20-resolv.conf
index ccc5f21e..984dae69 100644
--- a/dhcpcd-hooks/20-resolv.conf
+++ b/dhcpcd-hooks/20-resolv.conf
@@ -81,14 +81,22 @@ add_resolv_conf()
 	if [ -n "$new_domain_name" ]; then
 		set -- $new_domain_name
 		new_domain_name="$1"
-		conf="${conf}domain $new_domain_name\n"
+		if valid_domainname "$new_domain_name"; then
+			conf="${conf}domain $new_domain_name\n"
+		else
+			syslog err "Invalid domain name: $new_domain_name"
+		fi
 		# Support RFC violating search in domain
 		if [ -z "$new_domain_search" -a -n "$2" ]; then
 			new_domain_search="$@"
 		fi
 	fi
 	if [ -n "$new_domain_search" ]; then
-		conf="${conf}search $new_domain_search\n"
+		if valid_domainname_list; then
+			conf="${conf}search $new_domain_search\n"
+		else
+			syslog err "Invalid domain name in list: $new_domain_search"
+		fi
 	fi
 	for x in ${new_domain_name_servers}; do
 		conf="${conf}nameserver $x\n"
diff --git a/dhcpcd-hooks/30-hostname b/dhcpcd-hooks/30-hostname
index 87446fbe..65d1a13b 100644
--- a/dhcpcd-hooks/30-hostname
+++ b/dhcpcd-hooks/30-hostname
@@ -18,13 +18,22 @@ need_hostname()
 	esac
 }
 
+try_hostname()
+{
+	if valid_domainname "$1"; then
+		hostname "$1"
+	else
+		syslog err "Invalid hostname: $1"
+	fi
+}
+
 set_hostname()
 {
 	if need_hostname; then
 		if [ -n "$new_host_name" ]; then
-			hostname "$new_host_name"
+			try_hostname "$new_host_name"
 		elif [ -n "$new_fqdn_name" ]; then
-			hostname "$new_fqdn_name"
+			try_hostname "$new_fqdn_name"
 		fi
 	fi
 }
diff --git a/dhcpcd-hooks/50-yp.conf b/dhcpcd-hooks/50-yp.conf
index a1f57988..1a2bf4fa 100644
--- a/dhcpcd-hooks/50-yp.conf
+++ b/dhcpcd-hooks/50-yp.conf
@@ -13,6 +13,11 @@ make_yp_conf()
 	rm -f "$cf"
 	echo "$signature" > "$cf"
 	if [ -n "$new_nis_domain" ]; then
+		if ! valid_domainname "$new_nis_domain"; then
+			syslog err "Invalid NIS domain name: $new_nis_domain"
+			rm -f "$cf"
+			return 1
+		fi
 		domainname "$new_nis_domain"
 		if [ -n "$new_nis_servers" ]; then
 			prefix="domain $new_nis_domain server "
diff --git a/dhcpcd-hooks/50-ypbind b/dhcpcd-hooks/50-ypbind
index 9a1feccd..f63660e5 100644
--- a/dhcpcd-hooks/50-ypbind
+++ b/dhcpcd-hooks/50-ypbind
@@ -67,7 +67,11 @@ if [ "$reason" = PREINIT ]; then
 	rm -f "$ypbind_dir/$interface"
 elif $if_up || $if_down; then
 	if [ -n "$new_nis_domain" ]; then
-		make_yp_binding
+		if valid_domainname "$new_nis_domain"; then
+			make_yp_binding
+		else
+			syslog err "Invalid NIS domain name: $new_nis_domain"
+		fi
 	elif [ -n "$old_nis_domain" ]; then
 		restore_yp_binding
 	fi
diff --git a/dhcpcd-run-hooks.8.in b/dhcpcd-run-hooks.8.in
index bcfc81da..1a1f7022 100644
--- a/dhcpcd-run-hooks.8.in
+++ b/dhcpcd-run-hooks.8.in
@@ -1,4 +1,4 @@
-.\" Copyright (c) 2006-2010 Roy Marples
+.\" Copyright (c) 2006-2011 Roy Marples
 .\" All rights reserved
 .\"
 .\" Redistribution and use in source and binary forms, with or without
@@ -22,7 +22,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd August 24, 2010
+.Dd March 23, 2011
 .Dt DHCPCD-RUN-HOOKS 8 SMM
 .Os
 .Sh NAME
@@ -135,3 +135,11 @@ in a lexical order and then finally
 .An Roy Marples Aq roy@marples.name
 .Sh BUGS
 Please report them to http://roy.marples.name/projects/dhcpcd
+.Sh SECURITY CONSIDERATIONS
+Little validation of DHCP options is done in dhcpcd itself.
+Instead, it is up to the hooks to handle any validation needed.
+To this end, some helper functions are provided, such as valid_domainname as
+used by the
+.Pa 20-resolv.conf
+hook to ensure that the hostname is not set to an invalid value.
+valid_path is also provided, but is currently unused by a stock hook script.
diff --git a/dhcpcd-run-hooks.in b/dhcpcd-run-hooks.in
index 7a72041c..11d29fb7 100644
--- a/dhcpcd-run-hooks.in
+++ b/dhcpcd-run-hooks.in
@@ -147,6 +147,46 @@ syslog()
 	fi
 }
 
+# Check for a valid domain name as per RFC1123 with the exception of
+# allowing - and _ as they seem to be widely used.
+valid_domainname()
+{
+	local name="$1" label
+
+	[ -z "$name" -o ${#name} -gt 255 ] && return 1
+	
+	while [ -n "$name" ]; do
+		label="${name%%.*}"
+		[ -z "$label" -o ${#label} -gt 63 ] && return 1
+		case "$label" in
+		-*|_*|*-|*_)		return 1;;
+		*[![:alnum:]-_]*)	return 1;;
+		esac
+		[ "$name" = "${name#*.}" ] && break
+		name="${name#*.}"
+	done
+	return 0	
+}
+
+valid_domainname_list()
+{
+	local name
+
+	for name in $@; do
+		valid_domainname "$name" || return $?
+	done
+	return 0
+}
+
+# Check for a valid path
+valid_path()
+{
+	case "$@" in
+	*[![:alnum:]#%+-_:\.,@~\\/\[\]=\ ]*) return 1;;
+	esac
+	return 0
+}
+
 # Check a system service exists 
 service_exists()
 {