[syslog-ng]filter() call bug fix

[Jon Marks]

--IJpNTDwzlM2Ie8A6
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,

I've uncovered a small bug in the filter() call within filter definitions.
Attached is a patch to filters.c against the current stable release
(1.4.11) to fix the bug.

The following is a description of the problem. Take the following bit of
configuration code for illustration:

############################
source local { unix-dgram("/dev/log"); udp(); internal(); };

filter f_test  { host("myhost");       };
filter f_test2 { program("something"); };
filter f_test3 { filter("f_test");     }; # <-- ERROR occurs because of this

destination d_test { file("/what/ever");

log { source(local); filter(f_test3); destination(d_test); };
############################

When filters are defined in the configuration file, they're added to
a list.  If they contain a call to another filter (as in f_test3), the
actual filter to which they refer is resolved the first time the former's
evaluated against a log message. (So you can refer to a filter in the
config file before you define it). The filter call resolution procedure
traverses the list, comparing the name of each filter to the name its
looking for. The problem is just a little mixup in the logic used to
detect when the right filter is found. Here's the code responsible,
from filters.c/do_filter_call():

       if (!self->call_rule) {
                struct log_filter *p = rule->prev_filter;
                while (p &&
                       p->name->length != self->name->length &&
                       memcmp(p->name->data, self->name->data, self->name->lengt
h)) {
                        p = p->prev_filter;
                }
		...
	}

In the while() loop, the previous filter is meant to be chosen until
the right one is found. But the condition logic says, "Try the next
filter while 1) there IS a next filter AND 2) the filter's name length
is different AND 3) the filter's name's contents don't match what we're
looking for." But what happens when the length of the present filter
doesn't match, but the contents *DO*? This can happen since the code only
compares the as many bytes as the target name has. This condition would
stop the loop, and choose the wrong filter (as p would be left pointing
to the unmatching filter). This (and the other lookup phase, not shown)
is fixed by the attached patch, by changing the logic as follows:


	if (!self->call_rule) {
                struct log_filter *p = rule->prev_filter;
                while (p &&
                       (p->name->length != self->name->length ||
                        memcmp(p->name->data, self->name->data,
				self->name->length)) ) {
                        p = p->prev_filter;
                }
                ...
        }

This should guarantee that the loop ends on p only when p is either NULL,
or when it points to a filter with a name of matching length AND
matching contents (thank DeMorgan for this explanation of that
code).

Otherwise, in the configuration snippet above, the filter() call in f_test3
resolves to "f_test2" in the lookup procedure, when "f_test" is the desired
referent. That's because, as it traverses the list backwards from itself,
it encounters f_test2 first (which is defined AFTER f_test). And f_test2
has unmatching length (good for advancing p) but matching contents up the
the length of the string "f_test" which is what's being sought. 

Thanks.
--
Jonathan Marks

Systems Administrator, Production Systems Group
Computing and Communication Services Office
University of Illinois at Urbana-Champaign



--IJpNTDwzlM2Ie8A6
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="syslog-ng-1.4.11-filter_call_fix.diff"

--- syslog-ng-1.4.11-orig/src/filters.c	Sun Feb 25 06:30:22 2001
+++ syslog-ng-1.4.11/src/filters.c	Fri May  4 16:03:41 2001
@@ -257,27 +257,40 @@
 {
 	CAST(filter_expr_call, self, c);
 
+	/* If I haven't found the filter to which I refer, look for it.
+	   First search forward through the global filter list... */
 	if (!self->call_rule) {
 		struct log_filter *p = rule->prev_filter;
 		while (p && 
-		       p->name->length != self->name->length &&
-		       memcmp(p->name->data, self->name->data, self->name->length)) {
+		       (p->name->length != self->name->length ||
+		       memcmp(p->name->data, self->name->data,
+				self->name->length))) {
 			p = p->prev_filter;
 		}
+		/* And if I don't find it, search backward. */
 		if (!p) {
 			p = rule->next_filter;
 			while (p && 
-			       p->name->length != self->name->length &&
-			       memcmp(p->name->data, self->name->data, self->name->length)) {
+			       (p->name->length != self->name->length ||
+			       memcmp(p->name->data, self->name->data,
+				 	self->name->length))) {
 				p = p->next_filter;
 			}
 			
 		}
-		self->call_rule = p;
+		self->call_rule = p; /* I found it or I found NULL */
 	}
+
 	if (self->call_rule)
+		/* If I found the referent filter, evaluate it (subject to MY
+		   negation state) */
 		return (FILTER_RULE_EVAL(self->call_rule->root, self->call_rule, log)) ^ c->comp;
 	else {
+		/* I didn't find it at all. BAD CONFIG FILE! Better say
+		   something. 
+                   PROBLEM: Now self->call_rule isn't set, so I'll end up
+		   doing the full list search EVERY TIME I'm evaluated, and 
+		   I'll only fail each time. */
 		werror("Error evaluating filter(%S), filter not found\n", self->name);
 		return 0;
 	}

--IJpNTDwzlM2Ie8A6--