Re: Segfault if using long ACLs

Hi Holger,

first, thank you very much for such a detailed bug report ! At first it looked impossible to me because there are tests everywhere to ensure that the last arg points to a null string.

But after a more careful read, I think I have found the issue.

Here in cfgparse.c, we iterate over the parameters :

3633	arg = 0;
3634	args[arg] = line;
3635
3636	while (*line && arg < MAX_LINE_ARGS) {

And here we ensure that we set the last arg to point to the last char of the line :

3700	/* zero out remaining args and ensure that at least one entry
3701	 * is zeroed out.
3702	 */
3703	while (++arg <= MAX_LINE_ARGS) {
3704		args[arg] = line;
3705	}

But this is wrong for two reasons :

• if we stop at 3636 on arg == MAX_LINE_ARGS, at 3703, we don't enter the second loop because ++arg is already larger than MAX_LINE_ARGS. In fact there is a reason for this, we don't want to scratch the last word when it's not a space.
• even if we got there, <line> would not point to the last char so we would not have an empty string anyway.

In fact the test would be valid if we could ensure that *line = 0 in case of truncated line. Also it would be wise to report it because as you tell it, you don't know if your configuration is working or not. So let's reject the line and indicate where it broke.

The following patch fixes it.

Thanks!
Willy

From 3b39c1446b9bd842324e87782a836948a07c25a2 Mon Sep 17 00:00:00 2001 From: Willy Tarreau <w#1wt.eu>
Date: Mon, 9 Nov 2009 21:16:53 +0100
Subject: [BUG] config: fix wrong handling of too large argument count

Holger Just reported that running ACLs with too many args caused a segfault during config parsing. This is caused by a wrong test on argument count. In case of too many arguments on a config line, the last one was not correctly zeroed. This is now done and we report the error indicating what part had been truncated.

---
 src/cfgparse.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index f0d690b..a1d2428 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -3697,6 +3697,21 @@ int readcfgfile(const char *file)
 		if (!**args)
 			continue;
 
+		if (*line) {
+			/* we had to stop due to too many args.
+			 * Let's terminate the string, print the offending part then cut the
+			 * last arg.
+			 */
+			while (*line && *line != '#' && *line != '\n' && *line != '\r')
+				line++;
+			*line = '\0';
+
+			Alert("parsing [%s:%d]: line too long, truncating at word %d, position %d : <%s>.\n",
+			      file, linenum, arg + 1, args[arg] - thisline + 1, args[arg]);
+			err_code |= ERR_ALERT | ERR_FATAL;
+			args[arg] = line;
+		}
+
 		/* zero out remaining args and ensure that at least one entry
 		 * is zeroed out.
 		 */
-- 
1.6.4.4