Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Will McVicker <willmcvicker@google.com>
To: security@kernel.org, Pablo Neira Ayuso <pablo@netfilter.org>,
	Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>,
	Florian Westphal <fw@strlen.de>
Cc: "David S. Miller" <davem@davemloft.net>,
	Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-team@android.com, Will McVicker <willmcvicker@google.com>
Subject: [PATCH 1/1] netfilter: nat: add range checks for access to nf_nat_l[34]protos[]
Date: Mon, 27 Jul 2020 17:57:20 +0000
Message-ID: <20200727175720.4022402-2-willmcvicker@google.com> (raw)
In-Reply-To: <20200727175720.4022402-1-willmcvicker@google.com>

The indexes to the nf_nat_l[34]protos arrays come from userspace. So we
need to make sure that before indexing the arrays, we verify the index
is within the array bounds in order to prevent an OOB memory access.
Here is an example kernel panic on 4.14.180 when userspace passes in an
index greater than NFPROTO_NUMPROTO.

Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
Modules linked in:...
Process poc (pid: 5614, stack limit = 0x00000000a3933121)
CPU: 4 PID: 5614 Comm: poc Tainted: G S      W  O    4.14.180-g051355490483
Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM
task: 000000002a3dfffe task.stack: 00000000a3933121
pc : __cfi_check_fail+0x1c/0x24
lr : __cfi_check_fail+0x1c/0x24
...
Call trace:
__cfi_check_fail+0x1c/0x24
name_to_dev_t+0x0/0x468
nfnetlink_parse_nat_setup+0x234/0x258
ctnetlink_parse_nat_setup+0x4c/0x228
ctnetlink_new_conntrack+0x590/0xc40
nfnetlink_rcv_msg+0x31c/0x4d4
netlink_rcv_skb+0x100/0x184
nfnetlink_rcv+0xf4/0x180
netlink_unicast+0x360/0x770
netlink_sendmsg+0x5a0/0x6a4
___sys_sendmsg+0x314/0x46c
SyS_sendmsg+0xb4/0x108
el0_svc_naked+0x34/0x38

Signed-off-by: Will McVicker <willmcvicker@google.com>
---
 net/ipv4/netfilter/nf_nat_l3proto_ipv4.c |  6 ++++--
 net/ipv6/netfilter/nf_nat_l3proto_ipv6.c |  5 +++--
 net/netfilter/nf_nat_core.c              | 27 ++++++++++++++++++++++--
 net/netfilter/nf_nat_helper.c            |  4 ++++
 4 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c b/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
index 6115bf1ff6f0..1912fc66147c 100644
--- a/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
@@ -218,7 +218,8 @@ int nf_nat_icmp_reply_translation(struct sk_buff *skb,
 		return 1;
 
 	l4proto = __nf_nat_l4proto_find(NFPROTO_IPV4, inside->ip.protocol);
-	if (!nf_nat_ipv4_manip_pkt(skb, hdrlen + sizeof(inside->icmp),
+	if (!l4proto ||
+	    !nf_nat_ipv4_manip_pkt(skb, hdrlen + sizeof(inside->icmp),
 				   l4proto, &ct->tuplehash[!dir].tuple, !manip))
 		return 0;
 
@@ -234,7 +235,8 @@ int nf_nat_icmp_reply_translation(struct sk_buff *skb,
 	/* Change outer to look like the reply to an incoming packet */
 	nf_ct_invert_tuplepr(&target, &ct->tuplehash[!dir].tuple);
 	l4proto = __nf_nat_l4proto_find(NFPROTO_IPV4, 0);
-	if (!nf_nat_ipv4_manip_pkt(skb, 0, l4proto, &target, manip))
+	if (!l4proto ||
+	    !nf_nat_ipv4_manip_pkt(skb, 0, l4proto, &target, manip))
 		return 0;
 
 	return 1;
diff --git a/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c b/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
index ca6d38698b1a..a72840baf27b 100644
--- a/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
+++ b/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
@@ -228,7 +228,8 @@ int nf_nat_icmpv6_reply_translation(struct sk_buff *skb,
 		return 1;
 
 	l4proto = __nf_nat_l4proto_find(NFPROTO_IPV6, inside->ip6.nexthdr);
-	if (!nf_nat_ipv6_manip_pkt(skb, hdrlen + sizeof(inside->icmp6),
+	if (!l4proto ||
+	    !nf_nat_ipv6_manip_pkt(skb, hdrlen + sizeof(inside->icmp6),
 				   l4proto, &ct->tuplehash[!dir].tuple, !manip))
 		return 0;
 
@@ -245,7 +246,7 @@ int nf_nat_icmpv6_reply_translation(struct sk_buff *skb,
 
 	nf_ct_invert_tuplepr(&target, &ct->tuplehash[!dir].tuple);
 	l4proto = __nf_nat_l4proto_find(NFPROTO_IPV6, IPPROTO_ICMPV6);
-	if (!nf_nat_ipv6_manip_pkt(skb, 0, l4proto, &target, manip))
+	if (!l4proto || !nf_nat_ipv6_manip_pkt(skb, 0, l4proto, &target, manip))
 		return 0;
 
 	return 1;
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 2268b10a9dcf..d28185f38955 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -64,12 +64,16 @@ struct nat_net {
 inline const struct nf_nat_l3proto *
 __nf_nat_l3proto_find(u8 family)
 {
+	if (family >= NFPROTO_NUMPROTO)
+		return NULL;
 	return rcu_dereference(nf_nat_l3protos[family]);
 }
 
 inline const struct nf_nat_l4proto *
 __nf_nat_l4proto_find(u8 family, u8 protonum)
 {
+	if (family >= NFPROTO_NUMPROTO)
+		return NULL;
 	return rcu_dereference(nf_nat_l4protos[family][protonum]);
 }
 EXPORT_SYMBOL_GPL(__nf_nat_l4proto_find);
@@ -317,7 +321,7 @@ find_best_ips_proto(const struct nf_conntrack_zone *zone,
  * range. It might not be possible to get a unique tuple, but we try.
  * At worst (or if we race), we will end up with a final duplicate in
  * __ip_conntrack_confirm and drop the packet. */
-static void
+static int
 get_unique_tuple(struct nf_conntrack_tuple *tuple,
 		 const struct nf_conntrack_tuple *orig_tuple,
 		 const struct nf_nat_range2 *range,
@@ -328,13 +332,22 @@ get_unique_tuple(struct nf_conntrack_tuple *tuple,
 	const struct nf_nat_l3proto *l3proto;
 	const struct nf_nat_l4proto *l4proto;
 	struct net *net = nf_ct_net(ct);
+	int ret = 0;
 
 	zone = nf_ct_zone(ct);
 
 	rcu_read_lock();
 	l3proto = __nf_nat_l3proto_find(orig_tuple->src.l3num);
+	if (!l3proto) {
+		ret = -EINVAL;
+		goto out;
+	}
 	l4proto = __nf_nat_l4proto_find(orig_tuple->src.l3num,
 					orig_tuple->dst.protonum);
+	if (!l4proto) {
+		ret = -EINVAL;
+		goto out;
+	}
 
 	/* 1) If this srcip/proto/src-proto-part is currently mapped,
 	 * and that same mapping gives a unique tuple within the given
@@ -387,6 +400,7 @@ get_unique_tuple(struct nf_conntrack_tuple *tuple,
 	l4proto->unique_tuple(l3proto, tuple, range, maniptype, ct);
 out:
 	rcu_read_unlock();
+	return ret;
 }
 
 struct nf_conn_nat *nf_ct_nat_ext_add(struct nf_conn *ct)
@@ -409,6 +423,7 @@ nf_nat_setup_info(struct nf_conn *ct,
 {
 	struct net *net = nf_ct_net(ct);
 	struct nf_conntrack_tuple curr_tuple, new_tuple;
+	int err;
 
 	/* Can't setup nat info for confirmed ct. */
 	if (nf_ct_is_confirmed(ct))
@@ -428,7 +443,9 @@ nf_nat_setup_info(struct nf_conn *ct,
 	nf_ct_invert_tuplepr(&curr_tuple,
 			     &ct->tuplehash[IP_CT_DIR_REPLY].tuple);
 
-	get_unique_tuple(&new_tuple, &curr_tuple, range, ct, maniptype);
+	err = get_unique_tuple(&new_tuple, &curr_tuple, range, ct, maniptype);
+	if (err < 0)
+		return NF_DROP;
 
 	if (!nf_ct_tuple_equal(&new_tuple, &curr_tuple)) {
 		struct nf_conntrack_tuple reply;
@@ -509,8 +526,12 @@ static unsigned int nf_nat_manip_pkt(struct sk_buff *skb, struct nf_conn *ct,
 	nf_ct_invert_tuplepr(&target, &ct->tuplehash[!dir].tuple);
 
 	l3proto = __nf_nat_l3proto_find(target.src.l3num);
+	if (!l3proto)
+		return NF_DROP;
 	l4proto = __nf_nat_l4proto_find(target.src.l3num,
 					target.dst.protonum);
+	if (!l4proto)
+		return NF_DROP;
 	if (!l3proto->manip_pkt(skb, 0, l4proto, &target, mtype))
 		return NF_DROP;
 
@@ -816,6 +837,8 @@ static int nfnetlink_parse_nat_proto(struct nlattr *attr,
 		return err;
 
 	l4proto = __nf_nat_l4proto_find(nf_ct_l3num(ct), nf_ct_protonum(ct));
+	if (!l4proto)
+		return -EINVAL;
 	if (l4proto->nlattr_to_range)
 		err = l4proto->nlattr_to_range(tb, range);
 
diff --git a/net/netfilter/nf_nat_helper.c b/net/netfilter/nf_nat_helper.c
index 99606baedda4..6f694444137e 100644
--- a/net/netfilter/nf_nat_helper.c
+++ b/net/netfilter/nf_nat_helper.c
@@ -121,6 +121,8 @@ bool __nf_nat_mangle_tcp_packet(struct sk_buff *skb,
 	datalen = skb->len - protoff;
 
 	l3proto = __nf_nat_l3proto_find(nf_ct_l3num(ct));
+	if (!l3proto)
+		return false;
 	l3proto->csum_recalc(skb, IPPROTO_TCP, tcph, &tcph->check,
 			     datalen, oldlen);
 
@@ -179,6 +181,8 @@ nf_nat_mangle_udp_packet(struct sk_buff *skb,
 		return true;
 
 	l3proto = __nf_nat_l3proto_find(nf_ct_l3num(ct));
+	if (!l3proto)
+		return false;
 	l3proto->csum_recalc(skb, IPPROTO_UDP, udph, &udph->check,
 			     datalen, oldlen);
 
-- 
2.28.0.rc0.142.g3c755180ce-goog


  reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-27 17:57 [PATCH 0/1] Netfilter OOB memory access security patch Will McVicker
2020-07-27 17:57 ` Will McVicker [this message]
2020-07-29 21:46   ` [PATCH 1/1] netfilter: nat: add range checks for access to nf_nat_l[34]protos[] Pablo Neira Ayuso
2020-07-31  0:26     ` William Mcvicker
2020-07-31 17:51       ` Pablo Neira Ayuso
2020-07-31 18:16         ` William Mcvicker
2020-08-03 18:31           ` [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum William Mcvicker
2020-08-04 11:37             ` Pablo Neira Ayuso
2020-08-24 19:38               ` [PATCH v3 0/1] " Will McVicker
2020-08-24 19:38                 ` [PATCH v3 1/1] " Will McVicker
2020-08-28 16:42                   ` Pablo Neira Ayuso
2020-08-28 16:45                     ` Florian Westphal
2020-08-28 17:11                       ` Pablo Neira Ayuso
2020-09-01 15:36               ` [PATCH v2 " Will Deacon
2020-09-01 17:29                 ` William Mcvicker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200727175720.4022402-2-willmcvicker@google.com \
    --to=willmcvicker@google.com \
    --cc=coreteam@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=fw@strlen.de \
    --cc=kadlec@blackhole.kfki.hu \
    --cc=kernel-team@android.com \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=security@kernel.org \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org
	public-inbox-index netfilter-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git