Skip to contentSkip to conversation

iptables: iptables-xml: Fix various parsing bugs

PATCH In review Phil Oester 2 participants 2 messages Jun 21, 2013 – Jul 26, 2013 netfilter

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 iptables: iptables-xml: Fix various parsing bugs

There are two bugs in iptables-xml do_rule_part parsing corrected by this patch:

1) Ignore "-A <chain>" instead of just "-A"
2) When checking to see if we need a <match> tag, inversion needs to be taken into account

This closes netfilter bugzilla #679.

Phil

iptables/iptables-xml.c +10 -4
index 4b12bd4..99d7527 100644
@@ -367,7 +367,8 @@ static void
 do_rule_part(char *leveltag1, char *leveltag2, int part, int argc,
 	     char *argv[], int argvattr[])
 {
-	int arg = 1;		// ignore leading -A
+	int i;
+	int arg = 2;		// ignore leading -A <chain>
 	char invert_next = 0;
 	char *spacer = "";	// space when needed to assemble arguments
 	char *level1 = NULL;
@@ -401,9 +402,14 @@ do_rule_part(char *leveltag1, char *leveltag2, int part, int argc,
 
 	/* Before we start, if the first arg is -[^-] and not -m or -j or -g 
 	   then start a dummy <match> tag for old style built-in matches.  
-	   We would do this in any case, but no need if it would be empty */
-	if (arg < argc && argv[arg][0] == '-' && !isTarget(argv[arg])
-	    && strcmp(argv[arg], "-m") != 0) {
+	   We would do this in any case, but no need if it would be empty 
+	   In the case of negation, we need to look at arg+1 */
+	if (arg < argc && strcmp(argv[arg], "!") == 0)
+		i = arg + 1;
+	else
+		i = arg;
+	if (i < argc && argv[i][0] == '-' && !isTarget(argv[i])
+	    && strcmp(argv[i], "-m") != 0) {
 		OPEN_LEVEL(1, "match");
 		printf(">\n");
 	}
#1 lore raw
Quoted 7 lines · Phil Oester

There are two bugs in iptables-xml do_rule_part parsing corrected by this patch:

1) Ignore "-A <chain>" instead of just "-A"
2) When checking to see if we need a <match> tag, inversion needs to be taken into account

This closes netfilter bugzilla #679.

Applied with comestic change, thanks Phil.

Quoted 21 lines

Phil

iptables/iptables-xml.c +2 -1
index 4b12bd4..99d7527 100644
@@ -367,7 +367,8 @@ static void
 do_rule_part(char *leveltag1, char *leveltag2, int part, int argc,
 	     char *argv[], int argvattr[])
 {
-	int arg = 1;		// ignore leading -A
+	int i;
+	int arg = 2;		// ignore leading -A <chain>
 	char invert_next = 0;
 	char *spacer = "";	// space when needed to assemble arguments
 	char *level1 = NULL;

iptables/iptables-xml.c do_rule_part(char *leveltag1, char *leveltag2, int part, int argc, · line 404

Quoted 4 lines
iptables/iptables-xml.c +0 -1
@@ -401,9 +402,14 @@ do_rule_part(char *leveltag1, char *leveltag2, int part, int argc,
 
 	/* Before we start, if the first arg is -[^-] and not -m or -j or -g 
 	   then start a dummy <match> tag for old style built-in matches.  
-	   We would do this in any case, but no need if it would be empty */

We prefer this comment style (similar to kernel coding style):

/* This is a long comment ...
* ...
*/

/* This is a short comment */

*Not your fault*, of course, that was already there, including some trailing whitespace.

In general, I don't like patches to address comestic stuff only, I think they generate too much noise, so I prefer that comestic stuff gets fixed while fixing/enhancing some real thing, like in this case.