[PATCH nf] netfilter: x_tables: require IP6T_F_PROTO when matching protocol

Pablo Neira Ayuso <[email protected]>
Newsgroups gmane.comp.security.firewalls.netfilter.devel
Message-ID <[email protected]>
Several check() functions validate the protocol field value but do not
require IP6T_F_PROTO. A crafted ip6tables rule can set the field to TCP
or UDP while leaving the protocol matching flag clear.

Reject rules without IP6T_F_PROTO so the match/target is invoked only
for the protocols it supports.

In the TPROXY target, this allows to reach a WARN in packet path.

Fixes: 6ad7889327a5e ("tproxy: added IPv6 support to the TPROXY target")
Link: https://patch.msgid.link/[email protected]/
Reported-by: Vega <[email protected]>
Signed-off-by: Pablo Neira Ayuso <[email protected]>
---
Florian mentioned xt_l2tp is also missing this, but xt_multiport also
needs it. I could not find any other user of .proto which does not
validate IP6T_F_PROTO in IPv6.

 net/netfilter/xt_TPROXY.c    |  3 ++-
 net/netfilter/xt_ecn.c       |  1 +
 net/netfilter/xt_l2tp.c      | 20 ++++++++++++--------
 net/netfilter/xt_multiport.c |  3 +++
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/xt_TPROXY.c b/net/netfilter/xt_TPROXY.c
index 5f60e7298a1e..13a94c9d06c0 100644
--- a/net/netfilter/xt_TPROXY.c
+++ b/net/netfilter/xt_TPROXY.c
@@ -179,7 +179,8 @@ static int tproxy_tg6_check(const struct xt_tgchk_param *par)
 	if (err)
 		return err;
 
-	if ((i->proto == IPPROTO_TCP || i->proto == IPPROTO_UDP) &&
+	if ((i->flags & IP6T_F_PROTO) &&
+	    (i->proto == IPPROTO_TCP || i->proto == IPPROTO_UDP) &&
 	    !(i->invflags & IP6T_INV_PROTO))
 		return 0;
 
diff --git a/net/netfilter/xt_ecn.c b/net/netfilter/xt_ecn.c
index a8503f5d26bf..a5c99f00c201 100644
--- a/net/netfilter/xt_ecn.c
+++ b/net/netfilter/xt_ecn.c
@@ -139,6 +139,7 @@ static int ecn_mt_check6(const struct xt_mtchk_param *par)
 		return -EINVAL;
 
 	if (info->operation & (XT_ECN_OP_MATCH_ECE | XT_ECN_OP_MATCH_CWR) &&
+	    ip->flags & IP6T_F_PROTO &&
 	    (ip->proto != IPPROTO_TCP || ip->invflags & IP6T_INV_PROTO)) {
 		pr_info_ratelimited("cannot match TCP bits for non-tcp packets\n");
 		return -EINVAL;
diff --git a/net/netfilter/xt_l2tp.c b/net/netfilter/xt_l2tp.c
index a61eb81e9f49..346cbc168009 100644
--- a/net/netfilter/xt_l2tp.c
+++ b/net/netfilter/xt_l2tp.c
@@ -267,14 +267,16 @@ static int l2tp_mt_check4(const struct xt_mtchk_param *par)
 	if (ret != 0)
 		return ret;
 
-	if ((ip->proto != IPPROTO_UDP) &&
-	    (ip->proto != IPPROTO_L2TP)) {
+	if (!(ip->flags & IP6T_F_PROTO) ||
+	    (ip->proto != IPPROTO_UDP &&
+	     ip->proto != IPPROTO_L2TP)) {
 		pr_info_ratelimited("missing protocol rule (udp|l2tpip)\n");
 		return -EINVAL;
 	}
 
-	if ((ip->proto == IPPROTO_L2TP) &&
-	    (info->version == 2)) {
+	if (!(ip->flags & IP6T_F_PROTO) ||
+	    (ip->proto == IPPROTO_L2TP &&
+	     info->version == 2)) {
 		pr_info_ratelimited("v2 doesn't support IP mode\n");
 		return -EINVAL;
 	}
@@ -294,14 +296,16 @@ static int l2tp_mt_check6(const struct xt_mtchk_param *par)
 	if (ret != 0)
 		return ret;
 
-	if ((ip->proto != IPPROTO_UDP) &&
-	    (ip->proto != IPPROTO_L2TP)) {
+	if (!(ip->flags & IP6T_F_PROTO) ||
+	    (ip->proto != IPPROTO_UDP &&
+	     ip->proto != IPPROTO_L2TP)) {
 		pr_info_ratelimited("missing protocol rule (udp|l2tpip)\n");
 		return -EINVAL;
 	}
 
-	if ((ip->proto == IPPROTO_L2TP) &&
-	    (info->version == 2)) {
+	if (!(ip->flags & IP6T_F_PROTO) ||
+	    (ip->proto == IPPROTO_L2TP &&
+	     info->version == 2)) {
 		pr_info_ratelimited("v2 doesn't support IP mode\n");
 		return -EINVAL;
 	}
diff --git a/net/netfilter/xt_multiport.c b/net/netfilter/xt_multiport.c
index bff5f53a9bef..b48c2cbbee62 100644
--- a/net/netfilter/xt_multiport.c
+++ b/net/netfilter/xt_multiport.c
@@ -156,6 +156,9 @@ static int multiport_mt6_check(const struct xt_mtchk_param *par)
 	const struct ip6t_ip6 *ip = par->entryinfo;
 	const struct xt_multiport_v1 *multiinfo = par->matchinfo;
 
+	if (!(ip->flags & IP6T_F_PROTO))
+		return -EINVAL;
+
 	if (!check(ip->proto, ip->invflags, multiinfo->flags, multiinfo->count))
 		return -EINVAL;
 
-- 
2.47.3
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.