diff mbox

ip(6)tables-restore segfault + patch

Message ID CAMNgthVfcqfaDNzo9NzJGzDNazUss7KkyYvvE_ykaD1gEMBd+w@mail.gmail.com
State Deferred
Delegated to: Pablo Neira
Headers show

Commit Message

Felix Bolte July 22, 2015, 9:21 a.m. UTC 
hi,

while fuzzing iptables-restore input with afl [0],
i found a very old and known crash to be still existent,
there was even a mailing list discussion [1][2] about it

instead of fixing the real cause,
the restore input was parsed for "-t" and "--table",
however this was not enough and the error could
still be triggered by e.g. "-vtnew"

please consider/review my two attached patches
the first patch is fixing the segfault less intrusively
and the second one removes the insufficient "-t" check


[0] http://lcamtuf.coredump.cx/afl/
[1] http://lists.netfilter.org/pipermail/netfilter-devel/2001-September/005638.html
[2] http://lists.netfilter.org/pipermail/netfilter-devel/2001-October/005840.html



best regards

felix
diff mbox

Patch

From: Felix Bolte <bolte.felix@gmail.com>
Date: Tue, 21 Jul 2015 15:36:18 +0200
Subject: [PATCH 2/2] ip(6)tables-restore: remove old "-t" check

This check became obsolete as the follow up segfault has been fixed.
Furthermore the check was insufficient as "-vt" was still triggering an error.
We should let iptables decide whether the passed arguments are valid or not.
However, multiple "-t" arguments should not harm the outcome as the handle dictates the current table.

Signed-off-by: Felix Bolte <bolte.felix@gmail.com>
---
 iptables/ip6tables-restore.c |    9 ---------
 iptables/iptables-restore.c  |    9 ---------
 2 files changed, 18 deletions(-)

diff --git a/iptables/ip6tables-restore.c b/iptables/ip6tables-restore.c
index 9393924..cc07b7c 100644
--- a/iptables/ip6tables-restore.c
+++ b/iptables/ip6tables-restore.c
@@ -154,15 +154,6 @@  static void add_param_to_argv(char *parsestart)
 
 			param_buffer[param_len] = '\0';
 
-			/* check if table name specified */
-			if (!strncmp(param_buffer, "-t", 2)
-                            || !strncmp(param_buffer, "--table", 8)) {
-				xtables_error(PARAMETER_PROBLEM,
-				"The -t option (seen in line %u) cannot be "
-				"used in ip6tables-restore.\n", line);
-				exit(1);
-			}
-
 			add_argv(param_buffer);
 			param_len = 0;
 		} else {
diff --git a/iptables/iptables-restore.c b/iptables/iptables-restore.c
index 638b171..488edb9 100644
--- a/iptables/iptables-restore.c
+++ b/iptables/iptables-restore.c
@@ -153,15 +153,6 @@  static void add_param_to_argv(char *parsestart)
 
 			param_buffer[param_len] = '\0';
 
-			/* check if table name specified */
-			if (!strncmp(param_buffer, "-t", 2)
-			    || !strncmp(param_buffer, "--table", 8)) {
-				xtables_error(PARAMETER_PROBLEM,
-				"The -t option (seen in line %u) cannot be "
-				"used in iptables-restore.\n", line);
-				exit(1);
-			}
-
 			add_argv(param_buffer);
 			param_len = 0;
 		} else {
-- 
1.7.9.5