[lxc-devel] [lxc/master] Fix errors in configuration file parsing

[flx42 on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 377 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20171128/1b99947f/attachment.bin>
-------------- next part --------------
From ffc87df2c77abc1bcdbd27f068b56a537936f8d1 Mon Sep 17 00:00:00 2001
From: Felix Abecassis <fabecassis at nvidia.com>
Date: Mon, 27 Nov 2017 22:53:20 -0800
Subject: [PATCH 1/3] confile: fix memory leak

Signed-off-by: Felix Abecassis <fabecassis at nvidia.com>
---
 src/lxc/confile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 17f48762c..c6a0aeef9 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -2060,7 +2060,7 @@ static int parse_line(char *buffer, void *data)
 	}
 
 	if (empty_line)
-		return 0;
+		goto on_error;
 
 	line += lxc_char_left_gc(line, strlen(line));
 

From ce21ca88e9b5ec1720031af59604029da66ec48f Mon Sep 17 00:00:00 2001
From: Felix Abecassis <fabecassis at nvidia.com>
Date: Mon, 27 Nov 2017 22:54:51 -0800
Subject: [PATCH 2/3] confile: error out if a network configuration key has no
 subkey

This prevent an infinite recursion in the case of "lxc.net.0. = a"

Signed-off-by: Felix Abecassis <fabecassis at nvidia.com>
---
 src/lxc/confile.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index c6a0aeef9..a2e5ba7c1 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -3801,6 +3801,10 @@ static struct lxc_config_t *get_network_config_ops(const char *key,
 	/* lxc.net.<idx>.<subkey> */
 	if (idx_end) {
 		*idx_end = '.';
+		if (strlen(idx_end + 1) == 0) {
+			ERROR("No subkey in network configuration key \"%s\"", key);
+			goto on_error;
+		}
 
 		memmove(copy + 8, idx_end + 1, strlen(idx_end + 1));
 		copy[strlen(key) - numstrlen + 1] = '\0';

From 7bf3ebf03815c4b83538fa861a76f866c8569095 Mon Sep 17 00:00:00 2001
From: Felix Abecassis <fabecassis at nvidia.com>
Date: Mon, 27 Nov 2017 23:07:23 -0800
Subject: [PATCH 3/3] confile_utils: simplify lxc_config_net_hwaddr

In addition to the memory corruption fixed in ee3e84df78424d26fc6c90862fbe0fa92a686b0d,
this function was also performing invalid memory accesses for the following inputs:
- `lxc.net`
- `lxc.net.`
- `lxc.net.0.`
- `lxc.network`
- `lxc.network.0.`

Signed-off-by: Felix Abecassis <fabecassis at nvidia.com>
---
 src/lxc/confile_utils.c | 57 ++++++-------------------------------------------
 1 file changed, 6 insertions(+), 51 deletions(-)

diff --git a/src/lxc/confile_utils.c b/src/lxc/confile_utils.c
index 50f42ef8c..c2901116c 100644
--- a/src/lxc/confile_utils.c
+++ b/src/lxc/confile_utils.c
@@ -545,63 +545,18 @@ int rand_complete_hwaddr(char *hwaddr)
 
 bool lxc_config_net_hwaddr(const char *line)
 {
-	char *copy, *p;
+	unsigned index;
+	char tmp[7];
 
 	if (strncmp(line, "lxc.net", 7) != 0)
 		return false;
-	if (strncmp(line, "lxc.network.hwaddr", 18) == 0)
-		return true;
-
-	/* We have to dup the line, if line is something like
-	 * "lxc.net.[i].xxx = xxxxx ", we need to remove
-	 * '[i]' and compare its key with 'lxc.net.hwaddr'*/
-	copy = strdup(line);
-	if (!copy) {
-		SYSERROR("failed to allocate memory");
-		return false;
-	}
-	if (*(copy + 8) >= '0' && *(copy + 8) <= '9') {
-		p = strchr(copy + 8, '.');
-		if (!p) {
-			free(copy);
-			return false;
-		}
-		/* strlen("hwaddr") = 6 */
-		if (strlen(p + 1) >= 6)
-			 memmove(copy + 8, p + 1, 6);
-		copy[8 + 6] = '\0';
-	}
-	if (strncmp(copy, "lxc.net.hwaddr", 14) == 0) {
-		free(copy);
+	if (strncmp(line, "lxc.net.hwaddr", 14) == 0)
 		return true;
-	}
-	free(copy);
-
-	/* We have to dup the line second time, if line is something like
-	 * "lxc.network.[i].xxx = xxxxx ", we need to remove
-	 * '[i]' and compare its key with 'lxc.network.hwaddr'*/
-	copy = strdup(line);
-	if (!copy) {
-		SYSERROR("failed to allocate memory");
-		return false;
-	}
-	if (*(copy + 12) >= '0' && *(copy + 12) <= '9') {
-		p = strchr(copy + 12, '.');
-		if (!p) {
-			free(copy);
-			return false;
-		}
-		/* strlen("hwaddr") = 6 */
-		if (strlen(p + 1) >= 6)
-			memmove(copy + 12, p + 1, 6);
-		copy[12 + 6] = '\0';
-	}
-	if (strncmp(copy, "lxc.network.hwaddr", 18) == 0) {
-		free(copy);
+	if (strncmp(line, "lxc.network.hwaddr", 18) == 0)
 		return true;
-	}
+	if (sscanf(line, "lxc.net.%u.%6s", &index, tmp) == 2 || sscanf(line, "lxc.network.%u.%6s", &index, tmp) == 2)
+		return strncmp(tmp, "hwaddr", 6) == 0;
 
-	free(copy);
 	return false;
 }