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 = 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
Fixes: c1d10adb4a521 ("[NETFILTER]: Add ctnetlink port for nf_conntrack")
Signed-off-by: Will McVicker <[email protected]>
---
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..0b89609a6e9d 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_IPV4 && l3num != NFPROTO_IPV6)
+ return -EOPNOTSUPP;
tuple->src.l3num = l3num;
err = ctnetlink_parse_tuple_ip(tb[CTA_TUPLE_IP], tuple);
--
2.28.0.297.g1956fa8f8d-goog
Hi Will,
Given this is for -stable maintainers only, I'd suggest:
1) Specify what -stable kernel versions this patch applies to.
Explain that this problem is gone since what kernel version.
2) Maybe clarify that this is only for stable in the patch subject,
e.g. [PATCH -stable v3] netfilter: nat: add a range check for l3/l4
Otherwise, this -stable maintainers might not identify this patch as
something that is targetted to them.
Thanks.
On Mon, Aug 24, 2020 at 07:38:32PM +0000, Will McVicker wrote:
> 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 = 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
>
> Fixes: c1d10adb4a521 ("[NETFILTER]: Add ctnetlink port for nf_conntrack")
> Signed-off-by: Will McVicker <[email protected]>
> ---
> 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..0b89609a6e9d 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_IPV4 && l3num != NFPROTO_IPV6)
> + return -EOPNOTSUPP;
> tuple->src.l3num = l3num;
>
> err = ctnetlink_parse_tuple_ip(tb[CTA_TUPLE_IP], tuple);
> --
> 2.28.0.297.g1956fa8f8d-goog
>
Pablo Neira Ayuso <[email protected]> wrote:
> Hi Will,
>
> Given this is for -stable maintainers only, I'd suggest:
>
> 1) Specify what -stable kernel versions this patch applies to.
> Explain that this problem is gone since what kernel version.
>
> 2) Maybe clarify that this is only for stable in the patch subject,
> e.g. [PATCH -stable v3] netfilter: nat: add a range check for l3/l4
Hmm, we silently accept a tuple that we can't really deal with, no?
> > + if (l3num != NFPROTO_IPV4 && l3num != NFPROTO_IPV6)
> > + return -EOPNOTSUPP;
I vote to apply this to nf.git
On Fri, Aug 28, 2020 at 06:45:51PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <[email protected]> wrote:
> > Hi Will,
> >
> > Given this is for -stable maintainers only, I'd suggest:
> >
> > 1) Specify what -stable kernel versions this patch applies to.
> > Explain that this problem is gone since what kernel version.
> >
> > 2) Maybe clarify that this is only for stable in the patch subject,
> > e.g. [PATCH -stable v3] netfilter: nat: add a range check for l3/l4
>
> Hmm, we silently accept a tuple that we can't really deal with, no?
Oh, I overlook, existing kernels are affected. You're right.
> > > + if (l3num != NFPROTO_IPV4 && l3num != NFPROTO_IPV6)
> > > + return -EOPNOTSUPP;
>
> I vote to apply this to nf.git
I have rebased this patch on top of nf.git, attached what I'll apply
to nf.git.