ongrep

commit 2b405bcea731e1f15cc05ce488bf69385b7a8000
parent 39f982c34853baabeead508be94f0f23cca5bbaf
Author: Stephen Gregoratto <dev@sgregoratto.me>
Date:   Sun, 21 Jun 2020 23:02:44 +1000

Fix expressions not being matched due to e9f2f7e

In changing the bpf filter setup, it would seem I introduced a subtle
bug: nothing would be matched when a expression is provided. This was
due to me assembling the filter by putting the initial template *first*
instead of last like in the original code. I am puzzled though, since
pcap-filter(5) and tcpdump(8) both state:

> Negation ("!" or "not")
> Concatenation ("&&" or "and")
> Alternation ("||" or "or")
>
> Alternation ("||" or "or") and concatenation ("&&" or "and") have
> equal precedence and associate left to right.

Now the filter is assembled as before. Added back changing "\r\n" to ' '
and added an extra byte for the null char at the end. Also tweaked an
error message to not refer to internal variables.

Diffstat:
M
ngrep.c
 | 
36
+++++++++++++++++++++---------------

1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/ngrep.c b/ngrep.c
@@ -955,15 +955,13 @@ char *
 get_filter_from_file(void)
 {
 	char *fstr;
-	size_t size, prefix;
+	size_t size;
 	const char *template;
 	int fd;
 	struct stat st;
 	ssize_t flen;
 
-	/* filter = prefix + " and ( " + file + " )" */
 	template = include_vlan ? BPF_TEMPLATE_IP_VLAN : BPF_TEMPLATE_IP;
-	prefix = strlen(template) + 5;
 
 	if ((fd = open(filter_file, O_RDONLY)) == -1) {
 		warn("cannot open %s", filter_file);
@@ -978,15 +976,15 @@ get_filter_from_file(void)
 		clean_exit(-1);
 	}
 
-	size = prefix + 8 + st.st_size + 2;
+	/* filter = "( " + file + " ) and " + template + '\0' */
+	size = 2 + st.st_size + 7 + strlen(template) + 1;
 	if (size >= SIZE_MAX)
-		warnx("fstr too long: %s", filter_file);
+		warnx("filter too long: %s", filter_file);
 	if ((fstr = malloc(size)) == NULL)
 		err(2, "malloc");
 
-	(void)strlcpy(fstr, template, size);
-	(void)strlcat(fstr, " and  ( ", size);
-	flen = read(fd, fstr + prefix, st.st_size);
+	(void)strlcpy(fstr, "( ", size);
+	flen = read(fd, fstr + 2, st.st_size);
 	if (flen == -1) {
 		warn("read %s", filter_file);
 		clean_exit(-1);
@@ -994,7 +992,15 @@ get_filter_from_file(void)
 		warn("short read of %s: expected %lld, got %zu", filter_file, st.st_size, flen);
 		clean_exit(-1);
 	}
-	(void)strlcat(fstr, " )", size);
+	for (char *s = fstr + 2; *s; s++)
+		if (*s == '\n' || *s == '\r')
+			*s = ' ';
+	/*
+	 * XXX: looks ugly if the file doesn't end in a newline.
+	 * Then again, do we even need to pad the parens?
+	 */
+	(void)strlcat(fstr, ") and ", size);
+	(void)strlcat(fstr, template, size);
 
 	return fstr;
 }
@@ -1003,13 +1009,11 @@ char *
 get_filter_from_argv(char **argv)
 {
 	char *fstr;
-	size_t size;
+	size_t size = 0;
 	char **arg = argv;
 	const char *template;
 
-	/* filter = prefix + " and ( " + args + " )" */
 	template = include_vlan ? BPF_TEMPLATE_IP_VLAN : BPF_TEMPLATE_IP;
-	size = strlen(template) + 8 + 2;
 
 	if (arg == NULL)
 		return NULL;
@@ -1017,6 +1021,8 @@ get_filter_from_argv(char **argv)
 	for (; *arg != NULL; arg++)
 		size += strlen(*arg) + 1;
 
+	/* filter = "( " + args + ") and " + template + '\0' */
+	size = 2 + size + 6 + strlen(template) + 1;
 	if (size >= SIZE_MAX) {
 		warnx("filter too long");
 		clean_exit(-1);
@@ -1025,13 +1031,13 @@ get_filter_from_argv(char **argv)
 	if ((fstr = malloc(size)) == NULL)
 		err(2, "malloc");
 
-	(void)strlcpy(fstr, template, size);
-	(void)strlcat(fstr, " and  ( ", size);
+	(void)strlcpy(fstr, "( ", size);
 	for (arg = argv; *arg != NULL; arg++) {
 		(void)strlcat(fstr, *arg, size);
 		(void)strlcat(fstr, " ", size);
 	}
-	(void)strlcat(fstr, ")", size);
+	(void)strlcat(fstr, ") and ", size);
+	(void)strlcat(fstr, template, size);
 
 	return fstr;
 }