[iptables PATCH v2 1/2] xtables: Fix for crash when comparing rules with standard target

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



When parsing an nftnl_rule with a standard verdict,
nft_rule_to_iptables_command_state() initialized cs->target but didn't
care about cs->target->t. When later comparing that rule to another,
compare_targets() crashed due to unconditional access to t's fields.

Signed-off-by: Phil Sutter <phil@xxxxxx>
---
Changes since v1:
- Fix for double free caused by not setting cs->target->t to NULL after
  freeing it.
---
 iptables/nft-shared.c                         | 23 +++++++++++++++----
 .../testcases/iptables/0005-delete-rules_0    |  7 ++++++
 iptables/xtables.c                            |  4 +++-
 3 files changed, 29 insertions(+), 5 deletions(-)
 create mode 100755 iptables/tests/shell/testcases/iptables/0005-delete-rules_0

diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index ca4e593656562..2d4b8d557eb73 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -660,19 +660,34 @@ void nft_rule_to_iptables_command_state(const struct nftnl_rule *r,
 		match->m = m;
 	}
 
-	if (cs->target != NULL)
+	if (cs->target != NULL) {
 		cs->jumpto = cs->target->name;
-	else if (cs->jumpto != NULL)
+	} else if (cs->jumpto != NULL) {
+		struct xt_entry_target *t;
+		uint32_t size;
+
 		cs->target = xtables_find_target(cs->jumpto, XTF_TRY_LOAD);
-	else
+		if (!cs->target)
+			return;
+
+		size = XT_ALIGN(sizeof(struct xt_entry_target)) + cs->target->size;
+		t = xtables_calloc(1, size);
+		t->u.target_size = size;
+		t->u.user.revision = cs->target->revision;
+		strcpy(t->u.user.name, cs->jumpto);
+		cs->target->t = t;
+	} else {
 		cs->jumpto = "";
+	}
 }
 
 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);
+		cs->target->t = 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
new file mode 100755
index 0000000000000..9312fd53c3437
--- /dev/null
+++ b/iptables/tests/shell/testcases/iptables/0005-delete-rules_0
@@ -0,0 +1,7 @@
+#!/bin/bash
+
+# test for crash when comparing rules with standard target
+
+$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
diff --git a/iptables/xtables.c b/iptables/xtables.c
index d0167e6396975..eaa9fedeb03bb 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -1185,8 +1185,10 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table,
 	*table = p.table;
 
 	xtables_rule_matches_free(&cs.matches);
-	if (cs.target)
+	if (cs.target) {
 		free(cs.target->t);
+		cs.target->t = NULL;
+	}
 
 	if (h->family == AF_INET) {
 		free(args.s.addr.v4);
-- 
2.20.1




[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux