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 :
3633arg = 0;
3634args[arg] = line;
3635
3636while (*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 */
3703while (++arg <= MAX_LINE_ARGS) {
3704args[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, 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
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
