Re: [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum

2020-09-01 Thread William Mcvicker
Hi Will,

Pablo is going to add the latest patch to the nf.git tree. Once that
happens, I'm going to propose the patch in nf.git get cherry-picked to
the -stable branches.

Thanks,
Will


On Tue, Sep 1, 2020 at 8:36 AM Will Deacon  wrote:
>
> Hi Will, Pablo,
>
> On Tue, Aug 04, 2020 at 01:37:11PM +0200, Pablo Neira Ayuso wrote:
> > This patch is much smaller and if you confirm this is address the
> > issue, then this is awesome.
>
> Did that ever get confirmed? AFAICT, nothing ended up landing in the stable
> trees for this.
>
> Cheers,
>
> Will
>
>
> > On Mon, Aug 03, 2020 at 06:31:56PM +, William Mcvicker wrote:
> > [...]
> > > diff --git a/net/netfilter/nf_conntrack_netlink.c 
> > > b/net/netfilter/nf_conntrack_netlink.c
> > > index 31fa94064a62..56d310f8b29a 100644
> > > --- a/net/netfilter/nf_conntrack_netlink.c
> > > +++ b/net/netfilter/nf_conntrack_netlink.c
> > > @@ -1129,6 +1129,8 @@ ctnetlink_parse_tuple(const struct nlattr * const 
> > > cda[],
> > > if (!tb[CTA_TUPLE_IP])
> > > return -EINVAL;
> > >
> > > +   if (l3num >= NFPROTO_NUMPROTO)
> > > +   return -EINVAL;
> >
> > l3num can only be either NFPROTO_IPV4 or NFPROTO_IPV6.
> >
> > Other than that, bail out with EOPNOTSUPP.
> >
> > Thank you.


Re: [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum

2020-09-01 Thread Will Deacon
Hi Will, Pablo,

On Tue, Aug 04, 2020 at 01:37:11PM +0200, Pablo Neira Ayuso wrote:
> This patch is much smaller and if you confirm this is address the
> issue, then this is awesome.

Did that ever get confirmed? AFAICT, nothing ended up landing in the stable
trees for this.

Cheers,

Will


> On Mon, Aug 03, 2020 at 06:31:56PM +, William Mcvicker wrote:
> [...]
> > diff --git a/net/netfilter/nf_conntrack_netlink.c 
> > b/net/netfilter/nf_conntrack_netlink.c
> > index 31fa94064a62..56d310f8b29a 100644
> > --- a/net/netfilter/nf_conntrack_netlink.c
> > +++ b/net/netfilter/nf_conntrack_netlink.c
> > @@ -1129,6 +1129,8 @@ ctnetlink_parse_tuple(const struct nlattr * const 
> > cda[],
> > if (!tb[CTA_TUPLE_IP])
> > return -EINVAL;
> >  
> > +   if (l3num >= NFPROTO_NUMPROTO)
> > +   return -EINVAL;
> 
> l3num can only be either NFPROTO_IPV4 or NFPROTO_IPV6.
> 
> Other than that, bail out with EOPNOTSUPP.
> 
> Thank you.


Re: [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum

2020-08-04 Thread Pablo Neira Ayuso
Hi,

This patch is much smaller and if you confirm this is address the
issue, then this is awesome.

On Mon, Aug 03, 2020 at 06:31:56PM +, William Mcvicker wrote:
[...]
> diff --git a/net/netfilter/nf_conntrack_netlink.c 
> b/net/netfilter/nf_conntrack_netlink.c
> index 31fa94064a62..56d310f8b29a 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -1129,6 +1129,8 @@ ctnetlink_parse_tuple(const struct nlattr * const cda[],
>   if (!tb[CTA_TUPLE_IP])
>   return -EINVAL;
>  
> + if (l3num >= NFPROTO_NUMPROTO)
> + return -EINVAL;

l3num can only be either NFPROTO_IPV4 or NFPROTO_IPV6.

Other than that, bail out with EOPNOTSUPP.

Thank you.


Re: [PATCH v2 1/1] netfilter: nat: add a range check for l3/l4 protonum

2020-08-03 Thread William Mcvicker
Hi,

I have attached the follow up fix that checks for the proto index during
conntrack creation.

Thanks,
Will

On 07/31/2020, William Mcvicker wrote:
> Hi Pablo,
> 
> > Note that this code does not exist in the tree anymore. I'm not sure
> > if this problem still exists upstream, this patch does not apply to
> > nf.git. This fix should only go for -stable maintainers.
> 
> Right, the vulnerability has been fixed by the refactor commit fe2d0020994cd
> ("netfilter: nat: remove l4proto->in_range"), but this patch is a part of
> a full re-work of the code and doesn't backport very cleanly to the LTS
> branches. So this fix is only applicable to the 4.19, 4.14, 4.9, and 4.4 LTS
> branches. I missed the -stable email, but will re-add it to this thread with
> the re-worked patch.
> 
> Thanks,
> Will
> 
> On 07/31/2020, Pablo Neira Ayuso wrote:
> > Hi William,
> > 
> > On Fri, Jul 31, 2020 at 12:26:11AM +, William Mcvicker wrote:
> > > Hi Pablo,
> > > 
> > > Yes, I believe this oops is only triggered by userspace when the user
> > > specifically passes in an invalid nf_nat_l3protos index. I'm happy to 
> > > re-work
> > > the patch to check for this in ctnetlink_create_conntrack().
> > 
> > Great.
> > 
> > Note that this code does not exist in the tree anymore. I'm not sure
> > if this problem still exists upstream, this patch does not apply to
> > nf.git. This fix should only go for -stable maintainers.
> > 
> > > > BTW, do you have a Fixes: tag for this? This will be useful for
> > > > -stable maintainer to pick up this fix.
> > > 
> > > Regarding the Fixes: tag, I don't have one offhand since this bug was 
> > > reported
> > > to me, but I can search through the code history to find the commit that
> > > exposed this vulnerability.
> > 
> > That would be great.
> > 
> > Thank you.
>From 2a9d621fa5c225e6aece6b4622a9a816c6fcfa0d Mon Sep 17 00:00:00 2001
From: Will McVicker 
Date: Fri, 31 Jul 2020 13:10:43 -0700
Subject: [PATCH] netfilter: nat: add a range check for l3/l4 protonum

The indexes to the nf_nat_l[34]protos arrays come from userspace. So
check the tuple's family, e.g. l3num, when creating the conntrack in
order to prevent an OOB memory access during setup.  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 = 0xa3933121)
CPU: 4 PID: 5614 Comm: poc Tainted: G S  W  O4.14.180-g051355490483
Hardware name: Qualcomm Technologies, Inc. SM8150 V2 PM8150 Google Inc. MSM
task: 2a3dfffe task.stack: a3933121
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

Fixes: c1d10adb4a521 ("[NETFILTER]: Add ctnetlink port for nf_conntrack")
Signed-off-by: Will McVicker 
---
 net/netfilter/nf_conntrack_netlink.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 31fa94064a62..56d310f8b29a 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1129,6 +1129,8 @@ ctnetlink_parse_tuple(const struct nlattr * const cda[],
 	if (!tb[CTA_TUPLE_IP])
 		return -EINVAL;
 
+	if (l3num >= NFPROTO_NUMPROTO)
+		return -EINVAL;
 	tuple->src.l3num = l3num;
 
 	err = ctnetlink_parse_tuple_ip(tb[CTA_TUPLE_IP], tuple);
-- 
2.28.0.163.g6104cc2f0b6-goog