* [PATCH 1/8] iptables: Coverity: DEADCODE
2011-06-10 13:25 [PATCH 0/8] Possible problems found by static analysis of code Jiri Popelka
@ 2011-06-10 13:25 ` Jiri Popelka
2011-06-22 13:49 ` Jan Engelhardt
2011-06-10 13:25 ` [PATCH 2/8] iptables: Coverity: FORWARD_NULL Jiri Popelka
` (7 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Jiri Popelka @ 2011-06-10 13:25 UTC (permalink / raw)
To: netfilter-devel; +Cc: Jiri Popelka
libiptc/libiptc.c:407: dead_error_condition: On this path, the condition "res > 0" cannot be false.
libiptc/libiptc.c:396: at_least: After this line, the value of "res" is at least 1.
libiptc/libiptc.c:393: equality_cond: Condition "res == 0" is evaluated as false.
libiptc/libiptc.c:396: new_values: Noticing condition "res < 0".
libiptc/libiptc.c:425: new_values: Noticing condition "res < 0".
libiptc/libiptc.c:407: new_values: Noticing condition "res > 0".
libiptc/libiptc.c:435: dead_error_line: Execution cannot reach this statement "return list_pos;".
---
libiptc/libiptc.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/libiptc/libiptc.c b/libiptc/libiptc.c
index 0b6d5e3..c2cb0bc 100644
--- a/libiptc/libiptc.c
+++ b/libiptc/libiptc.c
@@ -403,7 +403,7 @@ __iptcc_bsearch_chain_index(const char *name, unsigned int offset,
}
debug("jump back to pos:%d (end:%d)\n", pos, end);
goto loop;
- } else if (res > 0 ){ /* Not far enough, jump forward */
+ } else { /* Not far enough, jump forward */
/* Exit case: Last element of array */
if (pos == handle->chain_index_sz-1) {
@@ -430,8 +430,6 @@ __iptcc_bsearch_chain_index(const char *name, unsigned int offset,
debug("jump forward to pos:%d (end:%d)\n", pos, end);
goto loop;
}
-
- return list_pos;
}
/* Wrapper for string chain name based bsearch */
--
1.7.5.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/8] iptables: Coverity: DEADCODE
2011-06-10 13:25 ` [PATCH 1/8] iptables: Coverity: DEADCODE Jiri Popelka
@ 2011-06-22 13:49 ` Jan Engelhardt
0 siblings, 0 replies; 17+ messages in thread
From: Jan Engelhardt @ 2011-06-22 13:49 UTC (permalink / raw)
To: Jiri Popelka; +Cc: netfilter-devel
On Friday 2011-06-10 15:25, Jiri Popelka wrote:
>libiptc/libiptc.c:407: dead_error_condition: On this path, the condition "res > 0" cannot be false.
>libiptc/libiptc.c:396: at_least: After this line, the value of "res" is at least 1.
>libiptc/libiptc.c:393: equality_cond: Condition "res == 0" is evaluated as false.
>libiptc/libiptc.c:396: new_values: Noticing condition "res < 0".
>libiptc/libiptc.c:425: new_values: Noticing condition "res < 0".
>libiptc/libiptc.c:407: new_values: Noticing condition "res > 0".
>libiptc/libiptc.c:435: dead_error_line: Execution cannot reach this statement "return list_pos;".
Picked up by me.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/8] iptables: Coverity: FORWARD_NULL
2011-06-10 13:25 [PATCH 0/8] Possible problems found by static analysis of code Jiri Popelka
2011-06-10 13:25 ` [PATCH 1/8] iptables: Coverity: DEADCODE Jiri Popelka
@ 2011-06-10 13:25 ` Jiri Popelka
2011-06-10 13:25 ` [PATCH 3/8] iptables: Coverity: NEGATIVE_RETURNS Jiri Popelka
` (6 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Jiri Popelka @ 2011-06-10 13:25 UTC (permalink / raw)
To: netfilter-devel; +Cc: Jiri Popelka
ip6tables.c:1841: var_compare_op: Comparing "chain" to null implies that "chain" might be null.
ip6tables.c:1863: var_deref_model: Passing null variable "chain" to function "strcmp", which
dereferences it. (The dereference is assumed on the basis of the 'nonnull' parameter attribute.)
ip6tables.c:1946: var_deref_model: Passing null variable "chain" to function "ip6tc_delete_num_entry", which dereferences it.
libiptc/libiptc.c:2050: deref_parm_in_call: Function "iptcc_find_label" dereferences parameter "chain".
libiptc/libiptc.c:737: deref_parm_in_call: Function "strcmp" dereferences parameter "name".
(The dereference is assumed on the basis of the 'nonnull' parameter attribute.)
ip6tables.c:1967: var_deref_model: Passing null variable "chain" to function "ip6tc_zero_counter", which dereferences it.
ip6tables.c:1999: var_deref_model: Passing null variable "chain" to function "ip6tc_create_chain", which dereferences it.
ip6tables.c:2005: var_deref_model: Passing null variable "chain" to function "ip6tc_rename_chain", which dereferences it.
ip6tables.c:2008: var_deref_model: Passing null variable "chain" to function "ip6tc_set_policy", which dereferences it.
iptables.c:1879: var_compare_op: Comparing "chain" to null implies that "chain" might be null.
iptables.c:1901: var_deref_model: Passing null variable "chain" to function "strcmp", which
dereferences it. (The dereference is assumed on the basis of the 'nonnull' parameter attribute.)
iptables.c:1986: var_deref_model: Passing null variable "chain" to function "iptc_delete_num_entry", which dereferences it.
libiptc/libiptc.c:2050: deref_parm_in_call: Function "iptcc_find_label" dereferences parameter "chain".
libiptc/libiptc.c:737: deref_parm_in_call: Function "strcmp" dereferences parameter "name".
(The dereference is assumed on the basis of the 'nonnull' parameter attribute.)
iptables.c:2007: var_deref_model: Passing null variable "chain" to function "iptc_zero_counter", which dereferences it.
iptables.c:2039: var_deref_model: Passing null variable "chain" to function "iptc_create_chain", which dereferences it.
iptables.c:2045: var_deref_model: Passing null variable "chain" to function "iptc_rename_chain", which dereferences it.
iptables.c:2048: var_deref_model: Passing null variable "chain" to function "iptc_set_policy", which dereferences it.
iptables.c:1828: var_compare_op: Comparing "policy" to null implies that "policy" might be null.
iptables.c:2048: var_deref_model: Passing null variable "policy" to function "iptc_set_policy", which dereferences it.
libiptc/libiptc.c:2422: deref_parm_in_call: Function "strcmp" dereferences parameter "policy".
(The dereference is assumed on the basis of the 'nonnull' parameter attribute.)
iptables-xml.c:745: assign_zero: Assigning: "chain" = 0.
iptables-xml.c:855: var_deref_model: Passing null variable "chain" to function "needChain", which dereferences it.
iptables-xml.c:282: deref_parm_in_call: Function "strcmp" dereferences parameter "chain".
(The dereference is assumed on the basis of the 'nonnull' parameter attribute.)
---
iptables/ip6tables.c | 5 ++++-
iptables/iptables-xml.c | 3 ++-
iptables/iptables.c | 8 ++++++--
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 4037acf..b30c9b7 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -1770,7 +1770,10 @@ int do_command6(int argc, char *argv[], char **table, struct ip6tc_handle **hand
generic_opt_check(command, cs.options);
- if (chain != NULL && strlen(chain) >= XT_EXTENSION_MAXNAMELEN)
+ if (chain == NULL)
+ xtables_error(PARAMETER_PROBLEM, "no chain");
+
+ if (strlen(chain) >= XT_EXTENSION_MAXNAMELEN)
xtables_error(PARAMETER_PROBLEM,
"chain name `%s' too long (must be under %u chars)",
chain, XT_EXTENSION_MAXNAMELEN);
diff --git a/iptables/iptables-xml.c b/iptables/iptables-xml.c
index 5aa638c..e2cb809 100644
--- a/iptables/iptables-xml.c
+++ b/iptables/iptables-xml.c
@@ -847,7 +847,8 @@ main(int argc, char *argv[])
for (a = 0; a < newargc; a++)
DEBUGP("argv[%u]: %s\n", a, newargv[a]);
- needChain(chain);// Should we explicitly look for -A
+ if (chain != NULL)
+ needChain(chain);// Should we explicitly look for -A
do_rule(pcnt, bcnt, newargc, newargv, newargvattr);
save_argv();
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 4ae7541..4868e40 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -1800,7 +1800,10 @@ int do_command4(int argc, char *argv[], char **table, struct iptc_handle **handl
generic_opt_check(command, cs.options);
- if (chain != NULL && strlen(chain) >= XT_EXTENSION_MAXNAMELEN)
+ if (chain == NULL)
+ xtables_error(PARAMETER_PROBLEM, "no chain");
+
+ if (strlen(chain) >= XT_EXTENSION_MAXNAMELEN)
xtables_error(PARAMETER_PROBLEM,
"chain name `%s' too long (must be under %u chars)",
chain, XT_EXTENSION_MAXNAMELEN);
@@ -1978,7 +1981,8 @@ int do_command4(int argc, char *argv[], char **table, struct iptc_handle **handl
ret = iptc_rename_chain(chain, newname, *handle);
break;
case CMD_SET_POLICY:
- ret = iptc_set_policy(chain, policy, cs.options&OPT_COUNTERS ? &cs.fw.counters : NULL, *handle);
+ if (policy != NULL)
+ ret = iptc_set_policy(chain, policy, cs.options&OPT_COUNTERS ? &cs.fw.counters : NULL, *handle);
break;
default:
/* We should never reach this... */
--
1.7.5.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/8] iptables: Coverity: NEGATIVE_RETURNS
2011-06-10 13:25 [PATCH 0/8] Possible problems found by static analysis of code Jiri Popelka
2011-06-10 13:25 ` [PATCH 1/8] iptables: Coverity: DEADCODE Jiri Popelka
2011-06-10 13:25 ` [PATCH 2/8] iptables: Coverity: FORWARD_NULL Jiri Popelka
@ 2011-06-10 13:25 ` Jiri Popelka
2011-06-22 13:55 ` Jan Engelhardt
2011-06-10 13:25 ` [PATCH 4/8] iptables: Coverity: REVERSE_INULL Jiri Popelka
` (5 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Jiri Popelka @ 2011-06-10 13:25 UTC (permalink / raw)
To: netfilter-devel; +Cc: Jiri Popelka
libipq/libipq.c:232: var_tested_neg: Variable "h->fd" tests negative.
libipq/libipq.c:234: negative_returns: "h->fd" is passed to a parameter that cannot be negative.
---
libipq/libipq.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/libipq/libipq.c b/libipq/libipq.c
index e330487..fb65971 100644
--- a/libipq/libipq.c
+++ b/libipq/libipq.c
@@ -231,7 +231,6 @@ struct ipq_handle *ipq_create_handle(uint32_t flags, uint32_t protocol)
if (h->fd == -1) {
ipq_errno = IPQ_ERR_SOCKET;
- close(h->fd);
free(h);
return NULL;
}
--
1.7.5.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/8] iptables: Coverity: REVERSE_INULL
2011-06-10 13:25 [PATCH 0/8] Possible problems found by static analysis of code Jiri Popelka
` (2 preceding siblings ...)
2011-06-10 13:25 ` [PATCH 3/8] iptables: Coverity: NEGATIVE_RETURNS Jiri Popelka
@ 2011-06-10 13:25 ` Jiri Popelka
2011-06-22 13:58 ` Jan Engelhardt
2011-06-10 13:25 ` [PATCH 5/8] iptables: Coverity: UNINIT Jiri Popelka
` (4 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Jiri Popelka @ 2011-06-10 13:25 UTC (permalink / raw)
To: netfilter-devel; +Cc: Jiri Popelka
ip6tables-restore.c:186: deref_ptr_in_call: Dereferencing pointer "in".
ip6tables-restore.c:463: check_after_deref: Dereferencing "in" before a null check.
iptables-restore.c:192: deref_ptr_in_call: Dereferencing pointer "in".
iptables-restore.c:468: check_after_deref: Dereferencing "in" before a null check.
iptables-xml.c:671: deref_ptr_in_call: Dereferencing pointer "in".
iptables-xml.c:873: check_after_deref: Dereferencing "in" before a null check.
---
iptables/ip6tables-restore.c | 3 +--
iptables/iptables-restore.c | 3 +--
iptables/iptables-xml.c | 3 +--
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/iptables/ip6tables-restore.c b/iptables/ip6tables-restore.c
index 420bc52..ae147d5 100644
--- a/iptables/ip6tables-restore.c
+++ b/iptables/ip6tables-restore.c
@@ -460,7 +460,6 @@ int main(int argc, char *argv[])
exit(1);
}
- if (in != NULL)
- fclose(in);
+ fclose(in);
return 0;
}
diff --git a/iptables/iptables-restore.c b/iptables/iptables-restore.c
index 2624599..1cb833c 100644
--- a/iptables/iptables-restore.c
+++ b/iptables/iptables-restore.c
@@ -465,7 +465,6 @@ main(int argc, char *argv[])
exit(1);
}
- if (in != NULL)
- fclose(in);
+ fclose(in);
return 0;
}
diff --git a/iptables/iptables-xml.c b/iptables/iptables-xml.c
index e2cb809..f340889 100644
--- a/iptables/iptables-xml.c
+++ b/iptables/iptables-xml.c
@@ -866,8 +866,7 @@ main(int argc, char *argv[])
exit(1);
}
- if (in != NULL)
- fclose(in);
+ fclose(in);
printf("</iptables-rules>\n");
free_argv();
--
1.7.5.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/8] iptables: Coverity: UNINIT
2011-06-10 13:25 [PATCH 0/8] Possible problems found by static analysis of code Jiri Popelka
` (3 preceding siblings ...)
2011-06-10 13:25 ` [PATCH 4/8] iptables: Coverity: REVERSE_INULL Jiri Popelka
@ 2011-06-10 13:25 ` Jiri Popelka
2011-06-10 13:26 ` [PATCH 6/8] iptables: Coverity: VARARGS Jiri Popelka
` (3 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Jiri Popelka @ 2011-06-10 13:25 UTC (permalink / raw)
To: netfilter-devel; +Cc: Jiri Popelka
extensions/libxt_multiport.c:102: var_decl: Declaring variable "m" without initializer.
extensions/libxt_multiport.c:127: uninit_use: Using uninitialized value "m".
xtables.c:1496: var_decl: Declaring variable "n" without initializer.
xtables.c:1568: uninit_use: Using uninitialized value "n".
iptables-restore.c:128: var_decl: Declaring variable "curtable" without initializer.
iptables-restore.c:454: uninit_use_in_call: Using uninitialized element
of array "curtable" when calling "strcmp".
---
extensions/libxt_multiport.c | 2 --
iptables/iptables-restore.c | 2 +-
iptables/xtables.c | 2 +-
3 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/extensions/libxt_multiport.c b/extensions/libxt_multiport.c
index 03af5a9..63f96d9 100644
--- a/extensions/libxt_multiport.c
+++ b/extensions/libxt_multiport.c
@@ -108,7 +108,6 @@ parse_multi_ports_v1(const char *portstring,
{
char *buffer, *cp, *next, *range;
unsigned int i;
- uint16_t m;
buffer = strdup(portstring);
if (!buffer) xtables_error(OTHER_PROBLEM, "strdup failed");
@@ -133,7 +132,6 @@ parse_multi_ports_v1(const char *portstring,
if (multiinfo->ports[i-1] >= multiinfo->ports[i])
xtables_error(PARAMETER_PROBLEM,
"invalid portrange specified");
- m <<= 1;
}
}
multiinfo->count = i;
diff --git a/iptables/iptables-restore.c b/iptables/iptables-restore.c
index 1cb833c..5df0ef2 100644
--- a/iptables/iptables-restore.c
+++ b/iptables/iptables-restore.c
@@ -451,7 +451,7 @@ main(int argc, char *argv[])
free_argv();
fflush(stdout);
}
- if (tablename && (strcmp(tablename, curtable) != 0))
+ if (tablename && curtable && (strcmp(tablename, curtable) != 0))
continue;
if (!ret) {
fprintf(stderr, "%s: line %u failed\n",
diff --git a/iptables/xtables.c b/iptables/xtables.c
index acfcf8b..ee38421 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -1605,7 +1605,7 @@ xtables_ip6parse_multiple(const char *name, struct in6_addr **addrpp,
static const struct in6_addr zero_addr;
struct in6_addr *addrp;
char buf[256], *p;
- unsigned int len, i, j, n, count = 1;
+ unsigned int len, i, j, n = 0, count = 1;
const char *loop = name;
while ((loop = strchr(loop, ',')) != NULL) {
--
1.7.5.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/8] iptables: Coverity: VARARGS
2011-06-10 13:25 [PATCH 0/8] Possible problems found by static analysis of code Jiri Popelka
` (4 preceding siblings ...)
2011-06-10 13:25 ` [PATCH 5/8] iptables: Coverity: UNINIT Jiri Popelka
@ 2011-06-10 13:26 ` Jiri Popelka
2011-06-22 13:59 ` Jan Engelhardt
2011-06-10 13:26 ` [PATCH 7/8] iptables: Coverity: OVERRUN_STATIC Jiri Popelka
` (2 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Jiri Popelka @ 2011-06-10 13:26 UTC (permalink / raw)
To: netfilter-devel; +Cc: Jiri Popelka
xtables.c:931: va_init: Initializing va_list "args".
xtables.c:938: missing_va_end: va_end was not called for "args".
xtables.c:947: missing_va_end: va_end was not called for "args".
xtables.c:961: missing_va_end: va_end was not called for "args".
---
iptables/xtables.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/iptables/xtables.c b/iptables/xtables.c
index ee38421..9a43612 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -1042,8 +1042,10 @@ void xtables_param_act(unsigned int status, const char *p1, ...)
case XTF_ONLY_ONCE:
p2 = va_arg(args, const char *);
b = va_arg(args, unsigned int);
- if (!b)
+ if (!b) {
+ va_end(args);
return;
+ }
xt_params->exit_err(PARAMETER_PROBLEM,
"%s: \"%s\" option may only be specified once",
p1, p2);
@@ -1051,8 +1053,10 @@ void xtables_param_act(unsigned int status, const char *p1, ...)
case XTF_NO_INVERT:
p2 = va_arg(args, const char *);
b = va_arg(args, unsigned int);
- if (!b)
+ if (!b) {
+ va_end(args);
return;
+ }
xt_params->exit_err(PARAMETER_PROBLEM,
"%s: \"%s\" option cannot be inverted", p1, p2);
break;
@@ -1065,8 +1069,10 @@ void xtables_param_act(unsigned int status, const char *p1, ...)
break;
case XTF_ONE_ACTION:
b = va_arg(args, unsigned int);
- if (!b)
+ if (!b) {
+ va_end(args);
return;
+ }
xt_params->exit_err(PARAMETER_PROBLEM,
"%s: At most one action is possible", p1);
break;
--
1.7.5.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 7/8] iptables: Coverity: OVERRUN_STATIC
2011-06-10 13:25 [PATCH 0/8] Possible problems found by static analysis of code Jiri Popelka
` (5 preceding siblings ...)
2011-06-10 13:26 ` [PATCH 6/8] iptables: Coverity: VARARGS Jiri Popelka
@ 2011-06-10 13:26 ` Jiri Popelka
2011-06-10 21:10 ` Jan Engelhardt
2011-06-10 13:26 ` [PATCH 8/8] iptables: Coverity: RESOURCE_LEAK Jiri Popelka
2011-06-10 16:04 ` [PATCH 0/8] Possible problems found by static analysis of code Pablo Neira Ayuso
8 siblings, 1 reply; 17+ messages in thread
From: Jiri Popelka @ 2011-06-10 13:26 UTC (permalink / raw)
To: netfilter-devel; +Cc: Jiri Popelka
extensions/libip6t_REJECT.c:114: overrun-local: Overrunning static array "reject_table",
with 5 elements, at position 5 with index variable "i".
extensions/libip6t_REJECT.c:127: overrun-local: Overrunning static array "reject_table",
with 5 elements, at position 5 with index variable "i".
extensions/libipt_REJECT.c:135: overrun-local: Overrunning static array "reject_table",
with 8 elements, at position 8 with index variable "i".
extensions/libipt_REJECT.c:148: overrun-local: Overrunning static array "reject_table",
with 8 elements, at position 8 with index variable "i".
extensions/libxt_sctp.c:419: overrun-call: Overrunning callee's array of size 18 by passing
index "i" of value 255 in call to function "print_chunk(i, numeric)".
extensions/libxt_sctp.c:383: index_parm: Directly indexing parameter.
libiptc/libiptc.c:1690: overrun-buffer-arg: Overrunning static array "t->target.u.user.name"
of size 29 bytes by passing it to a function which indexes it
with argument "30UL" at byte position 29.
libiptc/libiptc.c:1127: overrun-buffer-arg: Overrunning static array "t->target.u.user.name"
of size 29 bytes by passing it to a function which indexes it
with argument "30UL" at byte position 29.
---
extensions/libip6t_REJECT.c | 13 +++++++------
extensions/libipt_REJECT.c | 11 ++++++-----
extensions/libxt_sctp.c | 2 +-
libiptc/libiptc.c | 4 ++--
4 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/extensions/libip6t_REJECT.c b/extensions/libip6t_REJECT.c
index 8085321..aeba996 100644
--- a/extensions/libip6t_REJECT.c
+++ b/extensions/libip6t_REJECT.c
@@ -27,7 +27,7 @@ static const struct reject_names reject_table[] = {
{"icmp6-adm-prohibited", "adm-prohibited",
IP6T_ICMP6_ADM_PROHIBITED, "ICMPv6 administratively prohibited"},
#if 0
- {"icmp6-not-neighbor", "not-neighbor"},
+ {"icmp6-not-neighbor", "not-neighbor",
IP6T_ICMP6_NOT_NEIGHBOR, "ICMPv6 not a neighbor"},
#endif
{"icmp6-addr-unreachable", "addr-unreach",
@@ -102,9 +102,10 @@ static void REJECT_print(const void *ip, const struct xt_entry_target *target,
unsigned int i;
for (i = 0; i < ARRAY_SIZE(reject_table); ++i)
- if (reject_table[i].with == reject->with)
+ if (reject_table[i].with == reject->with) {
+ printf(" reject-with %s", reject_table[i].name);
break;
- printf(" reject-with %s", reject_table[i].name);
+ }
}
static void REJECT_save(const void *ip, const struct xt_entry_target *target)
@@ -114,10 +115,10 @@ static void REJECT_save(const void *ip, const struct xt_entry_target *target)
unsigned int i;
for (i = 0; i < ARRAY_SIZE(reject_table); ++i)
- if (reject_table[i].with == reject->with)
+ if (reject_table[i].with == reject->with) {
+ printf(" --reject-with %s", reject_table[i].name);
break;
-
- printf(" --reject-with %s", reject_table[i].name);
+ }
}
static struct xtables_target reject_tg6_reg = {
diff --git a/extensions/libipt_REJECT.c b/extensions/libipt_REJECT.c
index 362c65e..39ddeb5 100644
--- a/extensions/libipt_REJECT.c
+++ b/extensions/libipt_REJECT.c
@@ -122,9 +122,10 @@ static void REJECT_print(const void *ip, const struct xt_entry_target *target,
unsigned int i;
for (i = 0; i < ARRAY_SIZE(reject_table); ++i)
- if (reject_table[i].with == reject->with)
+ if (reject_table[i].with == reject->with) {
+ printf(" reject-with %s", reject_table[i].name);
break;
- printf(" reject-with %s", reject_table[i].name);
+ }
}
static void REJECT_save(const void *ip, const struct xt_entry_target *target)
@@ -134,10 +135,10 @@ static void REJECT_save(const void *ip, const struct xt_entry_target *target)
unsigned int i;
for (i = 0; i < ARRAY_SIZE(reject_table); ++i)
- if (reject_table[i].with == reject->with)
+ if (reject_table[i].with == reject->with) {
+ printf(" --reject-with %s", reject_table[i].name);
break;
-
- printf(" --reject-with %s", reject_table[i].name);
+ }
}
static struct xtables_target reject_tg_reg = {
diff --git a/extensions/libxt_sctp.c b/extensions/libxt_sctp.c
index 5dbc36f..da9fb22 100644
--- a/extensions/libxt_sctp.c
+++ b/extensions/libxt_sctp.c
@@ -374,7 +374,7 @@ print_chunk(uint32_t chunknum, int numeric)
for (i = 0; i < ARRAY_SIZE(sctp_chunk_names); ++i)
if (sctp_chunk_names[i].chunk_type == chunknum)
- printf("%s", sctp_chunk_names[chunknum].name);
+ printf("%s", sctp_chunk_names[i].name);
}
}
diff --git a/libiptc/libiptc.c b/libiptc/libiptc.c
index c2cb0bc..1a99047 100644
--- a/libiptc/libiptc.c
+++ b/libiptc/libiptc.c
@@ -1121,7 +1121,7 @@ static inline int iptcc_compile_rule (struct xtc_handle *h, STRUCT_REPLACE *repl
STRUCT_STANDARD_TARGET *t;
t = (STRUCT_STANDARD_TARGET *)GET_TARGET(r->entry);
/* memset for memcmp convenience on delete/replace */
- memset(t->target.u.user.name, 0, FUNCTION_MAXNAMELEN);
+ memset(t->target.u.user.name, 0, XT_EXTENSION_MAXNAMELEN);
strcpy(t->target.u.user.name, STANDARD_TARGET);
/* Jumps can only happen to builtin chains, so we
* can safely assume that they always have a header */
@@ -1675,7 +1675,7 @@ iptcc_standard_map(struct rule_head *r, int verdict)
return 0;
}
/* memset for memcmp convenience on delete/replace */
- memset(t->target.u.user.name, 0, FUNCTION_MAXNAMELEN);
+ memset(t->target.u.user.name, 0, XT_EXTENSION_MAXNAMELEN);
strcpy(t->target.u.user.name, STANDARD_TARGET);
t->verdict = verdict;
--
1.7.5.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 7/8] iptables: Coverity: OVERRUN_STATIC
2011-06-10 13:26 ` [PATCH 7/8] iptables: Coverity: OVERRUN_STATIC Jiri Popelka
@ 2011-06-10 21:10 ` Jan Engelhardt
0 siblings, 0 replies; 17+ messages in thread
From: Jan Engelhardt @ 2011-06-10 21:10 UTC (permalink / raw)
To: Jiri Popelka; +Cc: netfilter-devel
On Friday 2011-06-10 15:26, Jiri Popelka wrote:
>
>libiptc/libiptc.c:1690: overrun-buffer-arg: Overrunning static array "t->target.u.user.name"
> of size 29 bytes by passing it to a function which indexes it
> with argument "30UL" at byte position 29.
>
>libiptc/libiptc.c:1127: overrun-buffer-arg: Overrunning static array "t->target.u.user.name"
> of size 29 bytes by passing it to a function which indexes it
> with argument "30UL" at byte position 29.
>diff --git a/libiptc/libiptc.c b/libiptc/libiptc.c
>index c2cb0bc..1a99047 100644
>--- a/libiptc/libiptc.c
>+++ b/libiptc/libiptc.c
>@@ -1121,7 +1121,7 @@ static inline int iptcc_compile_rule (struct xtc_handle *h, STRUCT_REPLACE *repl
> STRUCT_STANDARD_TARGET *t;
> t = (STRUCT_STANDARD_TARGET *)GET_TARGET(r->entry);
> /* memset for memcmp convenience on delete/replace */
>- memset(t->target.u.user.name, 0, FUNCTION_MAXNAMELEN);
>+ memset(t->target.u.user.name, 0, XT_EXTENSION_MAXNAMELEN);
> strcpy(t->target.u.user.name, STANDARD_TARGET);
> /* Jumps can only happen to builtin chains, so we
> * can safely assume that they always have a header */
I have a feeling that there was something... namely implicitly
setting t->target.u.user.revision too.
So this might need a +t->target.u.user.revision = 0;
unless you want t->target.u.user.revision to contain.. something,
probably undefined value.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 8/8] iptables: Coverity: RESOURCE_LEAK
2011-06-10 13:25 [PATCH 0/8] Possible problems found by static analysis of code Jiri Popelka
` (6 preceding siblings ...)
2011-06-10 13:26 ` [PATCH 7/8] iptables: Coverity: OVERRUN_STATIC Jiri Popelka
@ 2011-06-10 13:26 ` Jiri Popelka
2011-06-22 16:09 ` Jan Engelhardt
2011-06-10 16:04 ` [PATCH 0/8] Possible problems found by static analysis of code Pablo Neira Ayuso
8 siblings, 1 reply; 17+ messages in thread
From: Jiri Popelka @ 2011-06-10 13:26 UTC (permalink / raw)
To: netfilter-devel; +Cc: Jiri Popelka
xtables.c:320: alloc_fn: Calling allocation function "get_modprobe".
xtables.c:294: alloc_fn: Storage is returned from allocation function "malloc".
xtables.c:294: var_assign: Assigning: "ret" = "malloc(1024UL)".
xtables.c:304: return_alloc: Returning allocated memory "ret".
xtables.c:320: var_assign: Assigning: "buf" = storage returned from "get_modprobe()".
xtables.c:323: var_assign: Assigning: "modprobe" = "buf".
xtables.c:348: leaked_storage: Variable "buf" going out of scope
leaks the storage it points to.
xtables.c:348: leaked_storage: Returning without freeing "modprobe"
leaks the storage that it points to.
---
iptables/xtables.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 9a43612..951fca1 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -362,6 +362,7 @@ int xtables_insmod(const char *modname, const char *modprobe, bool quiet)
/* not usually reached */
exit(1);
case -1:
+ free(buf);
return -1;
default: /* parent */
--
1.7.5.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 0/8] Possible problems found by static analysis of code
2011-06-10 13:25 [PATCH 0/8] Possible problems found by static analysis of code Jiri Popelka
` (7 preceding siblings ...)
2011-06-10 13:26 ` [PATCH 8/8] iptables: Coverity: RESOURCE_LEAK Jiri Popelka
@ 2011-06-10 16:04 ` Pablo Neira Ayuso
2011-06-11 13:40 ` Jozsef Kadlecsik
8 siblings, 1 reply; 17+ messages in thread
From: Pablo Neira Ayuso @ 2011-06-10 16:04 UTC (permalink / raw)
To: Jiri Popelka; +Cc: netfilter-devel
On 10/06/11 15:25, Jiri Popelka wrote:
> We had analyzed the iptables-1.4.10 code with Coverity.
> Coverity is commercial enterprise level tool for
> static analysis (analysis based only on compiling of sources,
> not based on running of binary) of the code.
it would be cool if you pass that tool to the conntrack-tools and ipset.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/8] Possible problems found by static analysis of code
2011-06-10 16:04 ` [PATCH 0/8] Possible problems found by static analysis of code Pablo Neira Ayuso
@ 2011-06-11 13:40 ` Jozsef Kadlecsik
0 siblings, 0 replies; 17+ messages in thread
From: Jozsef Kadlecsik @ 2011-06-11 13:40 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Jiri Popelka, netfilter-devel
On Fri, 10 Jun 2011, Pablo Neira Ayuso wrote:
> On 10/06/11 15:25, Jiri Popelka wrote:
> > We had analyzed the iptables-1.4.10 code with Coverity.
> > Coverity is commercial enterprise level tool for
> > static analysis (analysis based only on compiling of sources,
> > not based on running of binary) of the code.
>
> it would be cool if you pass that tool to the conntrack-tools and ipset.
Yes, I second that.
Best regards,
Jozsef
-
E-mail : kadlec@blackhole.kfki.hu, kadlec@mail.kfki.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics
H-1525 Budapest 114, POB. 49, Hungary
^ permalink raw reply [flat|nested] 17+ messages in thread