iptables-restore segfaults

Andreas Ferber aferber@techfak.uni-bielefeld.de
Fri, 19 Oct 2001 03:56:11 +0200


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

On Tue, Oct 16, 2001 at 09:46:35AM +0200, Harald Welte wrote:
> 
> ok. I'll consider your patch, though I'm not sure if there is a more clean
> way of solving the problem.  Maybe the iptables core should refuse taking
> two "-t " options at all.

The version in CVS is totally broken. It now refuses any option
containing "-t" as a substring, like "--to-destination".

Bens original patch wasn't that broken, as it checks for whitespace
after the "-t", but it's still broken. Guess what happens if I happen
to have a network interface named "-t" ("foo-t" will also trigger with
Bens patch) and try to match it...

Attached is a patch that fixes all the issues mentioned above.

It adds a new parameter restore_lineno to do_command, which takes the
input line number from iptables-restore.c, iptables-standalone.c sets
this to zero. If this parameter is >0, do_command refuses to accept a
"-t" parameter and instead uses the initial value of *table (which is
initialized correctly by iptables-restore.c). An additional side
effect is that it is now possible to include the line number into
error messages generated by do_command (currently only used in the new
error message for "--table" with restore_lineno>0).

The changes for ip6tables are similar.

Andreas
-- 
       Andreas Ferber - dev/consulting GmbH - Bielefeld, FRG
     ---------------------------------------------------------
         +49 521 1365800 - af@devcon.net - www.devcon.net

--5I6of5zJg18YgZEa
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="iptables-restore-fix.patch"

Index: userspace/ip6tables-restore.c
===================================================================
RCS file: /var/cvs/netfilter/userspace/ip6tables-restore.c,v
retrieving revision 1.5
diff -u -r1.5 ip6tables-restore.c
--- userspace/ip6tables-restore.c	2001/10/16 07:53:34	1.5
+++ userspace/ip6tables-restore.c	2001/10/19 01:43:28
@@ -108,6 +108,7 @@
 	unsigned int line = 0;
 	int c;
 	char curtable[IP6T_TABLE_MAXNAMELEN + 1];
+        char *table = curtable;
 	FILE *in;
 	const char *modprobe = 0;
 
@@ -294,15 +295,6 @@
                                 parsestart = buffer;
                         }
 
-			/* prevent iptables-restore from crashing in do_command
-			 * when someone passes a "-t" on the line.
-			 *  - Ben Reser <ben@reser.org> */
-			if (strstr(buffer, "-t")) {
-				exit_error(PARAMETER_PROBLEM, 
-					   "Line %u seems to have a "
-					   " -t table option.\n", line);
-				exit(1);
-			}
 			if (!strlen((char *) &curtable)) {
 				exit_error(PARAMETER_PROBLEM,
 					   "Line %u seems to to have a "
@@ -365,14 +357,14 @@
                                 }
                         }
 
-			DEBUGP("calling do_command6(%u, argv, &%s, handle):\n",
+			DEBUGP("calling do_command6(%u, argv, &%s, line, handle):\n",
 					newargc, curtable);
 
 			for (a = 0; a <= newargc; a++)
 				DEBUGP("argv[%u]: %s\n", a, newargv[a]);
 
 			ret = do_command6(newargc, newargv, 
-                                          &newargv[2], &handle);
+                                          &table, line, &handle);
 
                         free_argv();
 		}
Index: userspace/ip6tables-standalone.c
===================================================================
RCS file: /var/cvs/netfilter/userspace/ip6tables-standalone.c,v
retrieving revision 1.5
diff -u -r1.5 ip6tables-standalone.c
--- userspace/ip6tables-standalone.c	2001/08/06 18:50:22	1.5
+++ userspace/ip6tables-standalone.c	2001/10/19 01:27:22
@@ -43,7 +43,7 @@
 	init_extensions();
 #endif
 
-	ret = do_command6(argc, argv, &table, &handle);
+	ret = do_command6(argc, argv, &table, 0, &handle);
 	if (ret)
 		ret = ip6tc_commit(&handle);
 
Index: userspace/ip6tables.c
===================================================================
RCS file: /var/cvs/netfilter/userspace/ip6tables.c,v
retrieving revision 1.17
diff -u -r1.17 ip6tables.c
--- userspace/ip6tables.c	2001/10/04 08:11:44	1.17
+++ userspace/ip6tables.c	2001/10/19 01:27:01
@@ -1649,7 +1649,7 @@
 	return e;
 }
 
-int do_command6(int argc, char *argv[], char **table, ip6tc_handle_t *handle)
+int do_command6(int argc, char *argv[], char **table, unsigned int restore_lineno, ip6tc_handle_t *handle)
 {
 	struct ip6t_entry fw, *e = NULL;
 	int invert = 0;
@@ -1945,6 +1945,11 @@
 			break;
 
 		case 't':
+                        if (restore_lineno)
+                                exit_error(PARAMETER_PROBLEM,
+                                           "Bad line %u: "
+                                           "--table not allowed here",
+                                           restore_lineno);
 			if (invert)
 				exit_error(PARAMETER_PROBLEM,
 					   "unexpected ! flag before --table");
Index: userspace/iptables-restore.c
===================================================================
RCS file: /var/cvs/netfilter/userspace/iptables-restore.c,v
retrieving revision 1.16
diff -u -r1.16 iptables-restore.c
--- userspace/iptables-restore.c	2001/10/16 09:51:33	1.16
+++ userspace/iptables-restore.c	2001/10/19 01:43:14
@@ -103,6 +103,7 @@
 	unsigned int line = 0;
 	int c;
 	char curtable[IPT_TABLE_MAXNAMELEN + 1];
+        char *table = curtable;
 	FILE *in;
 	const char *modprobe = 0;
 
@@ -289,15 +290,6 @@
 				parsestart = buffer;
 			}
 
-			/* prevent iptables-restore from crashing in do_command
-			 * when someone passes a "-t" on the line.
-			 *  - Ben Reser <ben@reser.org> */
-			if (strstr(buffer, "-t")) {
-				exit_error(PARAMETER_PROBLEM, 
-					   "Line %u seems to have a "
-					   " -t table option.\n", line);
-				exit(1);
-			}
 			if (!strlen((char *) &curtable)) {
 				exit_error(PARAMETER_PROBLEM,
 					   "Line %u seems to to have a "
@@ -306,8 +298,6 @@
 			} 
 			
 			add_argv(argv[0]);
-			add_argv("-t");
-			add_argv((char *) &curtable);
 			
 			if (counters && pcnt && bcnt) {
 				add_argv("--set-counters");
@@ -358,14 +348,14 @@
 				}
 			}
 
-			DEBUGP("calling do_command(%u, argv, &%s, handle):\n",
+			DEBUGP("calling do_command(%u, argv, &%s, line, handle):\n",
 				newargc, curtable);
 
 			for (a = 0; a < newargc; a++)
 				DEBUGP("argv[%u]: %s\n", a, newargv[a]);
 
 			ret = do_command(newargc, newargv, 
-					 &newargv[2], &handle);
+					 &table, line, &handle);
 
 			free_argv();
 		}
Index: userspace/iptables-standalone.c
===================================================================
RCS file: /var/cvs/netfilter/userspace/iptables-standalone.c,v
retrieving revision 1.6
diff -u -r1.6 iptables-standalone.c
--- userspace/iptables-standalone.c	2001/08/06 18:50:22	1.6
+++ userspace/iptables-standalone.c	2001/10/19 01:23:50
@@ -44,7 +44,7 @@
 	init_extensions();
 #endif
 
-	ret = do_command(argc, argv, &table, &handle);
+	ret = do_command(argc, argv, &table, 0, &handle);
 	if (ret)
 		ret = iptc_commit(&handle);
 
Index: userspace/iptables.c
===================================================================
RCS file: /var/cvs/netfilter/userspace/iptables.c,v
retrieving revision 1.40
diff -u -r1.40 iptables.c
--- userspace/iptables.c	2001/08/15 11:21:59	1.40
+++ userspace/iptables.c	2001/10/19 01:21:55
@@ -1644,7 +1644,7 @@
 	return e;
 }
 
-int do_command(int argc, char *argv[], char **table, iptc_handle_t *handle)
+int do_command(int argc, char *argv[], char **table, unsigned int restore_lineno, iptc_handle_t *handle)
 {
 	struct ipt_entry fw, *e = NULL;
 	int invert = 0;
@@ -1945,6 +1945,11 @@
 			break;
 
 		case 't':
+                        if (restore_lineno)
+                                exit_error(PARAMETER_PROBLEM,
+                                           "Bad line %u: "
+                                           "--table not allowed here",
+                                           restore_lineno);
 			if (invert)
 				exit_error(PARAMETER_PROBLEM,
 					   "unexpected ! flag before --table");
Index: userspace/include/ip6tables.h
===================================================================
RCS file: /var/cvs/netfilter/userspace/include/ip6tables.h,v
retrieving revision 1.7
diff -u -r1.7 ip6tables.h
--- userspace/include/ip6tables.h	2001/08/06 18:50:22	1.7
+++ userspace/include/ip6tables.h	2001/10/19 01:27:56
@@ -111,7 +111,7 @@
 extern void register_target6(struct ip6tables_target *me);
 
 extern int do_command6(int argc, char *argv[], char **table,
-		       ip6tc_handle_t *handle);
+		       unsigned int restore_lineno, ip6tc_handle_t *handle);
 /* Keeping track of external matches and targets: linked lists. */
 extern struct ip6tables_match *ip6tables_matches;
 extern struct ip6tables_target *ip6tables_targets;
Index: userspace/include/iptables.h
===================================================================
RCS file: /var/cvs/netfilter/userspace/include/iptables.h,v
retrieving revision 1.6
diff -u -r1.6 iptables.h
--- userspace/include/iptables.h	2001/08/06 18:50:22	1.6
+++ userspace/include/iptables.h	2001/10/19 01:28:12
@@ -114,7 +114,7 @@
 extern char *addr_to_dotted(const struct in_addr *addrp);
 
 extern int do_command(int argc, char *argv[], char **table,
-		      iptc_handle_t *handle);
+		      unsigned int restore_lineno, iptc_handle_t *handle);
 /* Keeping track of external matches and targets: linked lists.  */
 extern struct iptables_match *iptables_matches;
 extern struct iptables_target *iptables_targets;

--5I6of5zJg18YgZEa--