* [PATCH] iptables-restore cleanup & input checks
@ 2005-11-05 22:13 Olaf Rempel
0 siblings, 0 replies; only message in thread
From: Olaf Rempel @ 2005-11-05 22:13 UTC (permalink / raw)
To: netfilter-devel
[-- Attachment #1: Type: text/plain, Size: 940 bytes --]
Hi list!
iptables-restore can restore wrong policy counters or even segfault when
reading invalid/incomplete data:
# Generated by iptables-save v1.3.3 on Sat Nov 5 21:12:41 2005
*mangle
# wrong data will be inserted (return value of parse_counters() is not checked)
:PREROUTING ACCEPT [ 14226472:14821988667]
# wrong data will be inserted (like PREROUTING)
:INPUT ACCEPT [587357:]
# restore will segfault, b/c parse_counters() string-parameter is a NULL-pointer
:FORWARD ACCEPT
:OUTPUT ACCEPT [11628546:19902662543]
:POSTROUTING ACCEPT [22035607:20460618489]
COMMIT
two patches attached:
- iptables-1.3.4-exit_error.patch
some cleanup work: removes all exit() calls after exit_error() calls
- iptables-1.3.4-policycounters.patch
adds NULL-pointer check in parse_counters() and a error msg if it fails
(this is similar to the "counters" patch from fedora and others, but
nobody seems to care pushing :(
cheers
Olaf Rempel
[-- Attachment #2: iptables-1.3.4-exit_error.patch --]
[-- Type: text/x-patch, Size: 4110 bytes --]
diff -uNr iptables-1.3.4.org/ip6tables-restore.c iptables-1.3.4/ip6tables-restore.c
--- iptables-1.3.4.org/ip6tables-restore.c 2005-06-24 18:34:19.000000000 +0200
+++ iptables-1.3.4/ip6tables-restore.c 2005-11-05 22:31:32.000000000 +0100
@@ -66,11 +66,10 @@
handle = ip6tc_init(tablename);
}
- if (!handle) {
+ if (!handle)
exit_error(PARAMETER_PROBLEM, "%s: unable to initialize"
"table '%s'\n", program_name, tablename);
- exit(1);
- }
+
return handle;
}
@@ -191,12 +190,11 @@
table = strtok(buffer+1, " \t\n");
DEBUGP("line %u, table '%s'\n", line, table);
- if (!table) {
+ if (!table)
exit_error(PARAMETER_PROBLEM,
"%s: line %u table name invalid\n",
program_name, line);
- exit(1);
- }
+
strncpy(curtable, table, IP6T_TABLE_MAXNAMELEN);
curtable[IP6T_TABLE_MAXNAMELEN] = '\0';
@@ -225,12 +223,10 @@
chain = strtok(buffer+1, " \t\n");
DEBUGP("line %u, chain '%s'\n", line, chain);
- if (!chain) {
+ if (!chain)
exit_error(PARAMETER_PROBLEM,
"%s: line %u chain name invalid\n",
program_name, line);
- exit(1);
- }
if (ip6tc_builtin(chain, handle) <= 0) {
if (noflush && ip6tc_is_chain(chain, handle)) {
@@ -252,12 +248,10 @@
policy = strtok(NULL, " \t\n");
DEBUGP("line %u, policy '%s'\n", line, policy);
- if (!policy) {
+ if (!policy)
exit_error(PARAMETER_PROBLEM,
"%s: line %u policy invalid\n",
program_name, line);
- exit(1);
- }
if (strcmp(policy, "-") != 0) {
struct ip6t_counters count;
@@ -381,12 +375,10 @@
/* check if table name specified */
if (!strncmp(param_buffer, "-t", 3)
- || !strncmp(param_buffer, "--table", 8)) {
+ || !strncmp(param_buffer, "--table", 8))
exit_error(PARAMETER_PROBLEM,
"Line %u seems to have a "
"-t table option.\n", line);
- exit(1);
- }
add_argv(param_buffer);
param_start += param_len + 1;
diff -uNr iptables-1.3.4.org/iptables-restore.c iptables-1.3.4/iptables-restore.c
--- iptables-1.3.4.org/iptables-restore.c 2005-06-24 18:34:19.000000000 +0200
+++ iptables-1.3.4/iptables-restore.c 2005-11-05 22:32:21.000000000 +0100
@@ -63,11 +63,10 @@
handle = iptc_init(tablename);
}
- if (!handle) {
+ if (!handle)
exit_error(PARAMETER_PROBLEM, "%s: unable to initialize"
"table '%s'\n", program_name, tablename);
- exit(1);
- }
+
return handle;
}
@@ -194,12 +193,11 @@
table = strtok(buffer+1, " \t\n");
DEBUGP("line %u, table '%s'\n", line, table);
- if (!table) {
+ if (!table)
exit_error(PARAMETER_PROBLEM,
"%s: line %u table name invalid\n",
program_name, line);
- exit(1);
- }
+
strncpy(curtable, table, IPT_TABLE_MAXNAMELEN);
curtable[IPT_TABLE_MAXNAMELEN] = '\0';
@@ -228,12 +226,10 @@
chain = strtok(buffer+1, " \t\n");
DEBUGP("line %u, chain '%s'\n", line, chain);
- if (!chain) {
+ if (!chain)
exit_error(PARAMETER_PROBLEM,
"%s: line %u chain name invalid\n",
program_name, line);
- exit(1);
- }
if (iptc_builtin(chain, handle) <= 0) {
if (noflush && iptc_is_chain(chain, handle)) {
@@ -255,12 +251,10 @@
policy = strtok(NULL, " \t\n");
DEBUGP("line %u, policy '%s'\n", line, policy);
- if (!policy) {
+ if (!policy)
exit_error(PARAMETER_PROBLEM,
"%s: line %u policy invalid\n",
program_name, line);
- exit(1);
- }
if (strcmp(policy, "-") != 0) {
struct ipt_counters count;
@@ -384,12 +378,10 @@
/* check if table name specified */
if (!strncmp(param_buffer, "-t", 3)
- || !strncmp(param_buffer, "--table", 8)) {
+ || !strncmp(param_buffer, "--table", 8))
exit_error(PARAMETER_PROBLEM,
"Line %u seems to have a "
"-t table option.\n", line);
- exit(1);
- }
add_argv(param_buffer);
param_start += param_len + 1;
[-- Attachment #3: iptables-1.3.4-policycounters.patch --]
[-- Type: text/x-patch, Size: 1965 bytes --]
diff -uNr iptables-1.3.4.org/ip6tables-restore.c iptables-1.3.4/ip6tables-restore.c
--- iptables-1.3.4.org/ip6tables-restore.c 2005-11-05 22:38:24.000000000 +0100
+++ iptables-1.3.4/ip6tables-restore.c 2005-11-05 22:39:52.000000000 +0100
@@ -75,7 +75,10 @@
int parse_counters(char *string, struct ip6t_counters *ctr)
{
- return (sscanf(string, "[%llu:%llu]", (unsigned long long *)&ctr->pcnt, (unsigned long long *)&ctr->bcnt) == 2);
+ if (string)
+ return (sscanf(string, "[%llu:%llu]", (unsigned long long *)&ctr->pcnt, (unsigned long long *)&ctr->bcnt) == 2);
+ else
+ return 0;
}
/* global new argv and argc */
@@ -260,8 +263,10 @@
char *ctrs;
ctrs = strtok(NULL, " \t\n");
- parse_counters(ctrs, &count);
-
+ if (!parse_counters(ctrs, &count))
+ exit_error(PARAMETER_PROBLEM,
+ "%s: line %u policy counters invalid\n",
+ program_name, line);
} else {
memset(&count, 0,
sizeof(struct ip6t_counters));
diff -uNr iptables-1.3.4.org/iptables-restore.c iptables-1.3.4/iptables-restore.c
--- iptables-1.3.4.org/iptables-restore.c 2005-11-05 22:38:24.000000000 +0100
+++ iptables-1.3.4/iptables-restore.c 2005-11-05 22:36:09.000000000 +0100
@@ -72,7 +72,10 @@
int parse_counters(char *string, struct ipt_counters *ctr)
{
- return (sscanf(string, "[%llu:%llu]", (unsigned long long *)&ctr->pcnt, (unsigned long long *)&ctr->bcnt) == 2);
+ if (string)
+ return (sscanf(string, "[%llu:%llu]", (unsigned long long *)&ctr->pcnt, (unsigned long long *)&ctr->bcnt) == 2);
+ else
+ return 0;
}
/* global new argv and argc */
@@ -263,8 +266,10 @@
char *ctrs;
ctrs = strtok(NULL, " \t\n");
- parse_counters(ctrs, &count);
-
+ if (!parse_counters(ctrs, &count))
+ exit_error(PARAMETER_PROBLEM,
+ "%s: line %u policy counters invalid\n",
+ program_name, line);
} else {
memset(&count, 0,
sizeof(struct ipt_counters));
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2005-11-05 22:13 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-11-05 22:13 [PATCH] iptables-restore cleanup & input checks Olaf Rempel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.