klibc - Apr 2011 - [PATCH] Escape DHCP options written to /tmp/net-$DEVCICE.conf

DHCP options like domain-name or hostname are written to
/tmp/net-$DEVICE.conf which is typically later used by other scripts to
determine the network configuration. This is done by sourcing the
/tmp/net-$DEVICE.conf file to get all defined variables.

This patch escapes the DHCP options written to /tmp/net-$DEVICE.conf
to prevent arbitrary code execution.

Signed-off-by: Ulrich Dangel <uli at spamt.net>
---
 usr/kinit/ipconfig/main.c |   55 +++++++++++++++++++++++++++++++-------------
 1 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/usr/kinit/ipconfig/main.c b/usr/kinit/ipconfig/main.c
index 76708a9..e53e22c 100644
--- a/usr/kinit/ipconfig/main.c
+++ b/usr/kinit/ipconfig/main.c
@@ -95,6 +95,25 @@ static void configure_device(struct netdev *dev)
 			dev->hostname, dev->name);
 }
 
+static void write_option(FILE* f, const char* name, const char* value)
+{
+	int i=0;
+
+	fprintf(f, "%s=\"", name);
+	while (value[i]) {
+		switch (value[i]) {
+			case '"':
+			case '$':
+			case '`':
+			case '\\':
+				fprintf(f, "\\");
+		}
+		fprintf(f, "%c", value[i]);
+		++i;
+	}
+	fprintf(f, "\"\n");
+}
+
 static void dump_device_config(struct netdev *dev)
 {
 	char fn[40];
@@ -103,22 +122,26 @@ static void dump_device_config(struct netdev *dev)
 	snprintf(fn, sizeof(fn), "/tmp/net-%s.conf", dev->name);
 	f = fopen(fn, "w");
 	if (f) {
-		fprintf(f, "DEVICE=%s\n", dev->name);
-		fprintf(f, "IPV4ADDR=%s\n", my_inet_ntoa(dev->ip_addr));
-		fprintf(f, "IPV4BROADCAST=%s\n",
-			my_inet_ntoa(dev->ip_broadcast));
-		fprintf(f, "IPV4NETMASK=%s\n", my_inet_ntoa(dev->ip_netmask));
-		fprintf(f, "IPV4GATEWAY=%s\n", my_inet_ntoa(dev->ip_gateway));
-		fprintf(f, "IPV4DNS0=%s\n",
-			my_inet_ntoa(dev->ip_nameserver[0]));
-		fprintf(f, "IPV4DNS1=%s\n",
-			my_inet_ntoa(dev->ip_nameserver[1]));
-		fprintf(f, "HOSTNAME=%s\n", dev->hostname);
-		fprintf(f, "DNSDOMAIN=\"%s\"\n", dev->dnsdomainname);
-		fprintf(f, "NISDOMAIN=%s\n", dev->nisdomainname);
-		fprintf(f, "ROOTSERVER=%s\n", my_inet_ntoa(dev->ip_server));
-		fprintf(f, "ROOTPATH=%s\n", dev->bootpath);
-		fprintf(f, "filename=\"%s\"\n", dev->filename);
+		write_option(f, "DEVICE", dev->name);
+		write_option(f, "IPV4ADDR",
+				my_inet_ntoa(dev->ip_addr));
+		write_option(f, "IPV4BROADCAST",
+				my_inet_ntoa(dev->ip_broadcast));
+		write_option(f, "IPV4NETMASK",
+				my_inet_ntoa(dev->ip_netmask));
+		write_option(f, "IPV4GATEWAY",
+				my_inet_ntoa(dev->ip_gateway));
+		write_option(f, "IPV4DNS0",
+				my_inet_ntoa(dev->ip_nameserver[0]));
+		write_option(f, "IPV4DNS1",
+				my_inet_ntoa(dev->ip_nameserver[1]));
+		write_option(f, "HOSTNAME",  dev->hostname);
+		write_option(f, "DNSDOMAIN", dev->dnsdomainname);
+		write_option(f, "NISDOMAIN", dev->nisdomainname);
+		write_option(f, "ROOTSERVER",
+				my_inet_ntoa(dev->ip_server));
+		write_option(f, "ROOTPATH", dev->bootpath);
+		write_option(f, "filename", dev->filename);
 		fclose(f);
 	}
 }
-- 
1.7.1

Thanks for the patch.

On Fri, 15 Apr 2011, Ulrich Dangel wrote:
> DHCP options like domain-name or hostname are written to
> /tmp/net-$DEVICE.conf which is typically later used by other scripts to
> determine the network configuration. This is done by sourcing the
> /tmp/net-$DEVICE.conf file to get all defined variables.
> 
> This patch escapes the DHCP options written to /tmp/net-$DEVICE.conf
> to prevent arbitrary code execution.
"There is actually a much better way to escape shell variables, a trick I
learned from git: Always start with a single quote ('), then leave all
characters except ' and ! unchanged.  For those two characters, emit the
sequence '\'' or '\!' Including the single quotes" -hpa

 
> Signed-off-by: Ulrich Dangel <uli at spamt.net>
> ---
>  usr/kinit/ipconfig/main.c |   55
+++++++++++++++++++++++++++++++-------------
>  1 files changed, 39 insertions(+), 16 deletions(-)
do you care to respin?

-- 
maks

DHCP options like domain-name or hostname are written to
/tmp/net-$DEVICE.conf which is typically later used by other scripts to
determine the network configuration. This is done by sourcing the
/tmp/net-$DEVICE.conf file to get all defined variables.

This patch escapes the DHCP options written to /tmp/net-$DEVICE.conf
to prevent arbitrary code execution.

Signed-off-by: Ulrich Dangel <uli at spamt.net>
---
 usr/kinit/ipconfig/main.c |   56 ++++++++++++++++++++++++++++++++-------------
 1 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/usr/kinit/ipconfig/main.c b/usr/kinit/ipconfig/main.c
index 76708a9..ec78dad 100644
--- a/usr/kinit/ipconfig/main.c
+++ b/usr/kinit/ipconfig/main.c
@@ -95,6 +95,26 @@ static void configure_device(struct netdev *dev)
 			dev->hostname, dev->name);
 }
 
+static void write_option(FILE* f, const char* name, const char* value)
+{
+	int i=0;
+
+	fprintf(f, "%s='", name);
+	while (value[i]) {
+		switch (value[i]) {
+			case '!':
+			case '\'':
+				fprintf(f, "'\\%c'", value[i]);
+				break;
+			default:
+				fprintf(f, "%c", value[i]);
+				break;
+		}
+		++i;
+	}
+	fprintf(f, "'\n");
+}
+
 static void dump_device_config(struct netdev *dev)
 {
 	char fn[40];
@@ -103,22 +123,26 @@ static void dump_device_config(struct netdev *dev)
 	snprintf(fn, sizeof(fn), "/tmp/net-%s.conf", dev->name);
 	f = fopen(fn, "w");
 	if (f) {
-		fprintf(f, "DEVICE=%s\n", dev->name);
-		fprintf(f, "IPV4ADDR=%s\n", my_inet_ntoa(dev->ip_addr));
-		fprintf(f, "IPV4BROADCAST=%s\n",
-			my_inet_ntoa(dev->ip_broadcast));
-		fprintf(f, "IPV4NETMASK=%s\n", my_inet_ntoa(dev->ip_netmask));
-		fprintf(f, "IPV4GATEWAY=%s\n", my_inet_ntoa(dev->ip_gateway));
-		fprintf(f, "IPV4DNS0=%s\n",
-			my_inet_ntoa(dev->ip_nameserver[0]));
-		fprintf(f, "IPV4DNS1=%s\n",
-			my_inet_ntoa(dev->ip_nameserver[1]));
-		fprintf(f, "HOSTNAME=%s\n", dev->hostname);
-		fprintf(f, "DNSDOMAIN=\"%s\"\n", dev->dnsdomainname);
-		fprintf(f, "NISDOMAIN=%s\n", dev->nisdomainname);
-		fprintf(f, "ROOTSERVER=%s\n", my_inet_ntoa(dev->ip_server));
-		fprintf(f, "ROOTPATH=%s\n", dev->bootpath);
-		fprintf(f, "filename=\"%s\"\n", dev->filename);
+		write_option(f, "DEVICE", dev->name);
+		write_option(f, "IPV4ADDR",
+				my_inet_ntoa(dev->ip_addr));
+		write_option(f, "IPV4BROADCAST",
+				my_inet_ntoa(dev->ip_broadcast));
+		write_option(f, "IPV4NETMASK",
+				my_inet_ntoa(dev->ip_netmask));
+		write_option(f, "IPV4GATEWAY",
+				my_inet_ntoa(dev->ip_gateway));
+		write_option(f, "IPV4DNS0",
+				my_inet_ntoa(dev->ip_nameserver[0]));
+		write_option(f, "IPV4DNS1",
+				my_inet_ntoa(dev->ip_nameserver[1]));
+		write_option(f, "HOSTNAME",  dev->hostname);
+		write_option(f, "DNSDOMAIN", dev->dnsdomainname);
+		write_option(f, "NISDOMAIN", dev->nisdomainname);
+		write_option(f, "ROOTSERVER",
+				my_inet_ntoa(dev->ip_server));
+		write_option(f, "ROOTPATH", dev->bootpath);
+		write_option(f, "filename", dev->filename);
 		fclose(f);
 	}
 }
-- 
1.7.1