[tor-commits] [torsocks/master] Overflow when parsing config lines with many arguments

[dgoulet@xxxxxxxxxxxxxx]

commit 3e9490366350114ee48a365f0fdd742614485e6f
Author: junglefowl <junglefowl@xxxxxxxxxx>
Date:   Mon Jun 13 16:42:02 2016 -0400

    Overflow when parsing config lines with many arguments
    
    It is possible to overflow tokens with a configuration that contains many
    arguments in one line.
    
    At first, the upper limit is specified as sizeof(tokens), which is wrong. It
    has to be DEFAULT_MAX_CONF_TOKEN or sizeof(tokens) / sizeof(tokens[0]). The
    former is shorter, therefor I took that one.
    
    The next issue is in utils_tokenize_ignore_comments, which verifies that
    enough space is available only with the ' ' separator. Later in the code, '\t'
    is also allowed as a separator, which means that more arguments could show up
    than previously taken into account during size checks.
    
    This is an unlikely case, so the check will be done while parsing.  When the
    limit is reached, previously allocated memory is released again and error
    value is returned.
    
    Signed-off-by: David Goulet <dgoulet@xxxxxxxxx>
---
 src/common/config-file.c |  5 +++--
 src/common/utils.c       | 29 ++++++++++++-----------------
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/common/config-file.c b/src/common/config-file.c
index da89ef7..750bfa8 100644
--- a/src/common/config-file.c
+++ b/src/common/config-file.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2000-2008 - Shaun Clowes <delius@xxxxxxxxxxx> 
+ * Copyright (C) 2000-2008 - Shaun Clowes <delius@xxxxxxxxxxx>
  * 				 2008-2011 - Robert Hogan <robert@xxxxxxxxxxxxxxx>
  * 				 	  2013 - David Goulet <dgoulet@xxxxxxxxx>
  *
@@ -200,7 +200,8 @@ static int parse_config_line(const char *line, struct configuration *config)
 	/*
 	 * The line is tokenized and each token is NULL terminated.
 	 */
-	nb_token = utils_tokenize_ignore_comments(line, sizeof(tokens), tokens);
+	nb_token = utils_tokenize_ignore_comments(line, DEFAULT_MAX_CONF_TOKEN,
+			tokens);
 	if (nb_token <= 0) {
 		/* Nothing on this line that is useful to parse. */
 		ret = 0;
diff --git a/src/common/utils.c b/src/common/utils.c
index 95ec803..e477f11 100644
--- a/src/common/utils.c
+++ b/src/common/utils.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2000-2008 - Shaun Clowes <delius@xxxxxxxxxxx> 
+ * Copyright (C) 2000-2008 - Shaun Clowes <delius@xxxxxxxxxxx>
  * 				 2008-2011 - Robert Hogan <robert@xxxxxxxxxxxxxxx>
  * 				 	  2013 - David Goulet <dgoulet@xxxxxxxxx>
  *
@@ -119,7 +119,7 @@ int utils_is_address_ipv6(const char *ip)
 ATTR_HIDDEN
 int utils_tokenize_ignore_comments(const char *_line, size_t size, char **tokens)
 {
-	int ret, i = 0, argc = 0;
+	int ret, i = 0;
 	char *c, *line = NULL, *saveptr;
 
 	assert(_line);
@@ -137,23 +137,12 @@ int utils_tokenize_ignore_comments(const char *_line, size_t size, char **tokens
 		goto end;
 	}
 
-	/* Count number of token. If larger than size, we return an error. */
-	c = line;
-	while ((c = strchr(c + 1, ' '))) {
-		/* Skip consecutive spaces. */
-		if (*(c + 1) == ' ') {
-			continue;
-		}
-		argc++;
-	}
-
-	if (argc > (int)size) {
-		ret = -ENOMEM;
-		goto error;
-	}
-
 	c = strtok_r(line, " \t", &saveptr);
 	while (c != NULL) {
+		if ((size_t) i >= size) {
+			ret = -ENOMEM;
+			goto error;
+		}
 		tokens[i] = strdup(c);
 		if (!tokens[i]) {
 			ret = -ENOMEM;
@@ -165,8 +154,14 @@ int utils_tokenize_ignore_comments(const char *_line, size_t size, char **tokens
 
 end:
 	ret = i;
+	free(line);
+	return ret;
+
 error:
 	free(line);
+	while (i-- > 0) {
+		free(tokens[i]);
+	}
 	return ret;
 }
 



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits