dhcpcd-discuss

Hi List

dhcpcd-5.2.12 fixes a potential security issue (CVE-2011-996) where 
dhcpcd does not strip or escape some shell meta characters in DHCP 
responses, allowing a rogue sever or party with escalated privileges on 
the server to cause remote code execution on the client.

As such, I've removed prior released from my web page for download, but 
they are still available from the FTP server for reference.

Workarounds for dhcpcd-4.x and dhcpcd-5.x:
Remove host_name from the options line in /etc/dhcpcd.conf

Workarounds for older dhcpcd versions:
none, upgrade to dhcpcd-5.2.12

Attached is a patch from Marius Tomaschewski <mt@xxxxxxx> to 
dhcpcd-3.2.3 which fixes this. Feel free to adapt it to older versions 
if you still run them, but I really encourage upgrading to dhcpcd-5 :)

Thanks

Roy

From 1e664b472a2a915ad31567de729ee0a521db93d0 Mon Sep 17 00:00:00 2001
From: Marius Tomaschewski <mt@xxxxxxx>
Date: Thu, 17 Mar 2011 22:18:16 +0100
Subject: [PATCH] Discard string options with incorrect values

Discard string options such as host and domain names
containing disallowed characters or beeing too long.
This proctive patch limits root-path to the a-zA-Z0-9,
space and the #%+-_:.,@~/\[]= characters.

Signed-off-by: Marius Tomaschewski <mt@xxxxxxx>
---
 configure.c |   15 +++---
 dhcp.c      |  160 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 dhcp.h      |    8 +++
 dhcpcd.c    |    9 +++-
 4 files changed, 177 insertions(+), 15 deletions(-)

diff --git a/configure.c b/configure.c
index 0969f73..24dd6e8 100644
--- a/configure.c
+++ b/configure.c
@@ -453,7 +453,7 @@ static char *lookuphostname (char *hostname, const dhcp_t *dhcp,
 	char *addr;
 	struct addrinfo hints;
 	struct addrinfo *res = NULL;
-	int result;
+	int result, check;
 	char *p;
 
 	logger (LOG_DEBUG, "Looking up hostname via DNS");
@@ -479,9 +479,10 @@ static char *lookuphostname (char *hostname, const dhcp_t *dhcp,
 	result = getaddrinfo (addr, "0", &hints, &res);
 	if (res)
 		freeaddrinfo (res);
-	if (result == 0)
+	check = check_domain_name(addr, strlen(addr), 0);
+	if (result == 0 || check != 0)
 		logger (LOG_ERR, "malicious PTR record detected");
-	if (result == 0 || ! *addr) {
+	if (result == 0 || ! *addr || check != 0) {
 		free (addr);
 		return (NULL);
 	}
@@ -758,12 +759,12 @@ int configure (const options_t *options, interface_t *iface,
 #endif
 
 	curhostname = xmalloc (sizeof (char) * MAXHOSTNAMELEN);
-	*curhostname = '\0';
+	memset(curhostname, 0, MAXHOSTNAMELEN);
 
-	gethostname (curhostname, MAXHOSTNAMELEN);
+	gethostname (curhostname, MAXHOSTNAMELEN - 1);
+	curhostname[MAXHOSTNAMELEN - 1] = '\0';
 	if (options->dohostname ||
-	    strlen (curhostname) == 0 ||
-	    strcmp (curhostname, "(none)") == 0 ||
+	    check_domain_name(curhostname, strlen (curhostname), 0) != 0 ||
 	    strcmp (curhostname, "localhost") == 0)
 	{
 		newhostname = xmalloc (sizeof (char) * MAXHOSTNAMELEN);
diff --git a/dhcp.c b/dhcp.c
index 8ed66da..5165078 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -41,6 +41,8 @@
 #include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
+#include <stddef.h>
+#include <ctype.h>
 
 #include "config.h"
 
@@ -618,6 +620,106 @@ static struct route_head *decode_routers (const unsigned char *data, int length)
 	return (head);
 }
 
+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 && 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.')   */
+			ptrdiff_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((int)*p)) {
+			/* also numbers at the begin are fine     */
+			return -1;
+		}
+	}
+	return dots ? -1 : 0;
+}
+
+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 || !len)
+		return -1;
+
+	for (p=ptr; *p && 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;
+}
+
+int check_dhcpoption(unsigned char option, const char *ptr, size_t len)
+{
+	if( !ptr)
+		return -1;
+
+	switch (option) {
+		case DHCP_NETBIOSNODETYPE:
+			if(len == 1 && ((int)*ptr == 1 || (int)*ptr == 2 ||
+					(int)*ptr == 4 || (int)*ptr == 8))
+				return 0;
+		break;
+		case DHCP_HOSTNAME:
+		case DHCP_DNSDOMAIN:
+		case DHCP_NISDOMAIN:
+		case DHCP_NETBIOSSCOPE:
+			return check_domain_name(ptr, len, 0);
+		break;
+		case DHCP_SIPSERVER:
+		case DHCP_DNSSEARCH:
+			return check_domain_name_list(ptr, len, 0);
+		break;
+		case DHCP_ROOTPATH:
+			if( len == 0)
+				return -1;
+			for (; *ptr && len-- > 0; ptr++) {
+				if( !(isalnum((int)*ptr) ||
+					*ptr == '#'  || *ptr == '%' ||
+					*ptr == '+'  || *ptr == '-' ||
+					*ptr == '_'  || *ptr == ':' ||
+					*ptr == '.'  || *ptr == ',' ||
+					*ptr == '@'  || *ptr == '~' ||
+					*ptr == '\\' || *ptr == '/' ||
+					*ptr == '['  || *ptr == ']' ||
+					*ptr == '='  || *ptr == ' '))
+					return -1;
+			}
+			return 0;
+		break;
+	}
+	return -1;
+}
+
 int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message)
 {
 	const unsigned char *p = message->options;
@@ -646,8 +748,16 @@ int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message)
 	dhcp->leasedfrom = tv.tv_sec;
 	dhcp->frominfo = false;
 	dhcp->address.s_addr = message->yiaddr;
-	strlcpy (dhcp->servername, (char *) message->servername,
-		 sizeof (dhcp->servername));
+	if (message->servername[0] != '\0' &&
+		check_domain_name((const char *)message->servername,
+			strlen((const char *)message->servername), 0) != 0)
+	{
+		logger (LOG_ERR, "suspect value in SERVERNAME - discarded");
+		dhcp->servername[0] = '\0';
+	} else {
+		strlcpy (dhcp->servername, (char *) message->servername,
+			 sizeof (dhcp->servername));
+	}
 
 #define LEN_ERR \
 	{ \
@@ -768,10 +878,20 @@ parse_start:
 	memcpy (_var, p, (size_t) length); \
 	memset (_var + length, 0, 1); \
 }
+#define CHECKOPT(_opt,_var) \
+	if(check_dhcpoption(_opt, (const char *)p, length) != 0) { \
+		logger (LOG_ERR, "suspect value in option %s - discarded", #_opt); \
+		if (_var) free (_var); \
+		_var = NULL; \
+	}
 			case DHCP_HOSTNAME:
+				CHECKOPT (DHCP_HOSTNAME, dhcp->hostname)
+				else
 				GETSTR (dhcp->hostname);
 				break;
 			case DHCP_DNSDOMAIN:
+				CHECKOPT (DHCP_DNSDOMAIN, dhcp->dnsdomain)
+				else
 				GETSTR (dhcp->dnsdomain);
 				break;
 			case DHCP_MESSAGE:
@@ -779,11 +899,15 @@ parse_start:
 				break;
 #ifdef ENABLE_INFO
 			case DHCP_ROOTPATH:
+				CHECKOPT (DHCP_ROOTPATH, dhcp->rootpath)
+				else
 				GETSTR (dhcp->rootpath);
 				break;
 #endif
 #ifdef ENABLE_NIS
 			case DHCP_NISDOMAIN:
+				CHECKOPT (DHCP_NISDOMAIN, dhcp->nisdomain)
+				else
 				GETSTR (dhcp->nisdomain);
 				break;
 #endif
@@ -814,11 +938,21 @@ parse_start:
 			case DHCP_DNSSEARCH:
 				MIN_LENGTH (1);
 				free (dhcp->dnssearch);
+				dhcp->dnssearch = NULL;
 				len = decode_search (p, length, NULL);
 				if (len > 0) {
-					dhcp->dnssearch = xmalloc (len);
-					decode_search (p, length,
-						       dhcp->dnssearch);
+					char *str = xmalloc (len);
+					decode_search (p, length, str);
+					if(check_dhcpoption(DHCP_DNSSEARCH,
+						str, len - 1) != 0) {
+						logger (LOG_ERR,
+							"suspect value in "
+							"option %s - discarded",
+							"DHCP_DNSSEARCH");
+						free(str);
+					} else {
+						dhcp->dnssearch = str;
+					}
 				}
 				break;
 
@@ -837,7 +971,20 @@ parse_start:
 #ifdef ENABLE_INFO
 			case DHCP_SIPSERVER:
 				free (dhcp->sipservers);
-				dhcp->sipservers = decode_sipservers (p,length);
+				dhcp->sipservers = NULL;
+				{
+					char *str = decode_sipservers (p,length);
+					if(check_dhcpoption(DHCP_SIPSERVER,
+						str, strlen(str)) != 0) {
+						logger (LOG_ERR,
+							"suspect value in "
+							"option %s - discarded",
+							"DHCP_SIPSERVER");
+						free(str);
+					} else {
+						dhcp->sipservers = str;
+					}
+				}
 				break;
 #endif
 
@@ -873,6 +1020,7 @@ parse_start:
 #undef LENGTH
 #undef MIN_LENGTH
 #undef MULT_LENGTH
+#undef CHECKOPT
 
 			default:
 				logger (LOG_DEBUG,
diff --git a/dhcp.h b/dhcp.h
index cc66d13..d2cdc45 100644
--- a/dhcp.h
+++ b/dhcp.h
@@ -86,6 +86,10 @@ enum DHCP_OPTIONS
 	DHCP_NISDOMAIN              = 40,
 	DHCP_NISSERVER              = 41,
 	DHCP_NTPSERVER              = 42,
+	DHCP_NETBIOSNAMESERVER      = 44,
+	DHCP_NETBIOSDDSERVER        = 45,
+	DHCP_NETBIOSNODETYPE        = 46,
+	DHCP_NETBIOSSCOPE           = 47,
 	DHCP_ADDRESS                = 50,
 	DHCP_LEASETIME              = 51,
 	DHCP_OPTIONSOVERLOADED      = 52,
@@ -213,4 +217,8 @@ ssize_t send_message (const interface_t *iface, const dhcp_t *dhcp,
 void free_dhcp (dhcp_t *dhcp);
 int parse_dhcpmessage (dhcp_t *dhcp, const dhcpmessage_t *message);
 
+int check_dhcpoption (unsigned char option, const char *ptr, size_t len);
+int check_domain_name(const char *ptr, size_t len, int dots);
+int check_domain_name_list(const char *ptr, size_t len, int dots);
+
 #endif
diff --git a/dhcpcd.c b/dhcpcd.c
index e101847..318dd7f 100644
--- a/dhcpcd.c
+++ b/dhcpcd.c
@@ -178,8 +178,10 @@ int main(int argc, char **argv)
 	options->doduid = true;
 	options->timeout = DEFAULT_TIMEOUT;
 
-	gethostname (options->hostname, sizeof (options->hostname));
-	if (strcmp (options->hostname, "(none)") == 0 ||
+	memset (options->hostname, 0, sizeof (options->hostname));
+	gethostname (options->hostname, sizeof (options->hostname) - 1);
+	options->hostname[sizeof (options->hostname) - 1] = '\0';
+	if (check_domain_name(options->hostname, strlen(options->hostname), 0) != 0 ||
 	    strcmp (options->hostname, "localhost") == 0)
 		memset (options->hostname, 0, sizeof (options->hostname));
 
@@ -228,6 +230,9 @@ int main(int argc, char **argv)
 						"`%s' too long for HostName string, max is %d",
 						optarg, MAXHOSTNAMELEN);
 					goto abort;
+				} else if(check_domain_name(optarg, strlen(optarg), 0) != 0) {
+					logger (LOG_ERR, "suspect string in hostname argument");
+					goto abort;
 				} else
 					strlcpy (options->hostname, optarg,
 						 sizeof (options->hostname));
-- 
1.7.1