When comparing two rules with non-standard targets, differences in
targets' payloads wasn't respected.
The cause is a rather hideous one: Unlike xtables_find_match(),
xtables_find_target() did not care whether the found target was already
in use or not, so the same target instance was assigned to both rules
and therefore payload comparison happened over the same memory location.
With legacy iptables it is not possible to reuse a target: The only case
where two rules (i.e., iptables_command_state instances) could exist at
the same time is when comparing rules, but that's handled using libiptc.
Signed-off-by: Phil Sutter <phil@xxxxxx>
---
iptables/nft-bridge.c | 9 +++++++++
iptables/nft-shared.c | 8 +++++++-
.../testcases/iptables/0005-delete-rules_0 | 7 +++++++
libxtables/xtables.c | 18 +++++++++++++++++-
4 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index cbd671c38d2e1..9b0c6932ccca5 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -45,6 +45,15 @@ void ebt_cs_clean(struct iptables_command_state *cs)
free(m);
m = nm;
}
+
+ if (cs->target) {
+ free(cs->target->t);
+
+ if (cs->target == cs->target->next) {
+ free(cs->target);
+ cs->target = NULL;
+ }
+ }
}
static void ebt_print_mac(const unsigned char *mac)
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index caed347ddb724..d24b27ec146bc 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -684,8 +684,14 @@ void nft_rule_to_iptables_command_state(const struct nftnl_rule *r,
void nft_clear_iptables_command_state(struct iptables_command_state *cs)
{
xtables_rule_matches_free(&cs->matches);
- if (cs->target)
+ if (cs->target) {
free(cs->target->t);
+
+ if (cs->target == cs->target->next) {
+ free(cs->target);
+ cs->target = NULL;
+ }
+ }
}
void print_header(unsigned int format, const char *chain, const char *pol,
diff --git a/iptables/tests/shell/testcases/iptables/0005-delete-rules_0 b/iptables/tests/shell/testcases/iptables/0005-delete-rules_0
index 9312fd53c3437..5038cbce5a5cf 100755
--- a/iptables/tests/shell/testcases/iptables/0005-delete-rules_0
+++ b/iptables/tests/shell/testcases/iptables/0005-delete-rules_0
@@ -5,3 +5,10 @@
$XT_MULTI iptables -A FORWARD -i eth23 -o eth42 -j DROP
$XT_MULTI iptables -D FORWARD -i eth23 -o eth42 -j REJECT
[[ $? -eq 1 ]] || exit 1
+
+# test incorrect deletion of rules with deviating payload
+# in non-standard target
+
+$XT_MULTI iptables -A FORWARD -i eth23 -o eth42 -j MARK --set-mark 23
+$XT_MULTI iptables -D FORWARD -i eth23 -o eth42 -j MARK --set-mark 42
+[[ $? -eq 1 ]] || exit 1
diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index ea9bb102c8eb4..895f6988eaf57 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -756,8 +756,24 @@ xtables_find_target(const char *name, enum xtables_tryload tryload)
}
for (ptr = xtables_targets; ptr; ptr = ptr->next) {
- if (extension_cmp(name, ptr->name, ptr->family))
+ if (extension_cmp(name, ptr->name, ptr->family)) {
+ struct xtables_target *clone;
+
+ /* First target of this type: */
+ if (ptr->t == NULL)
+ break;
+
+ /* Second and subsequent clones */
+ clone = xtables_malloc(sizeof(struct xtables_target));
+ memcpy(clone, ptr, sizeof(struct xtables_target));
+ clone->udata = NULL;
+ clone->tflags = 0;
+ /* This is a clone: */
+ clone->next = clone;
+
+ ptr = clone;
break;
+ }
}
#ifndef NO_SHARED_LIBS
--
2.20.1
