* [PATCH 0/1] ipset patch for nf
@ 2016-03-08 19:44 Jozsef Kadlecsik
2016-03-08 19:44 ` [PATCH 1/1] netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length Jozsef Kadlecsik
2016-03-10 18:28 ` [PATCH 0/1] ipset patch for nf Pablo Neira Ayuso
0 siblings, 2 replies; 5+ messages in thread
From: Jozsef Kadlecsik @ 2016-03-08 19:44 UTC (permalink / raw)
To: netfilter-devel; +Cc: Pablo Neira Ayuso
Hi Pablo,
Please apply the next patch against the nf tree:
- Julia Lawall pointed out that IPSET_ATTR_ETHER netlink attribute
length was not checked explicitly. The patch adds the missing
checkings.
The patch should be applied to the older stable kernel branches too.
Best regards,
Jozsef
The following changes since commit 45040978c8994d1401baf5cc5ac71c1495d4e120:
netfilter: ipset: Fix set:list type crash when flush/dump set in parallel (2016-02-24 20:32:21 +0100)
are available in the git repository at:
git://blackhole.kfki.hu/nf master
for you to fetch changes up to d8aacd87180141ff6b812b53de77a4336e87c91a:
netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length (2016-03-08 20:36:17 +0100)
----------------------------------------------------------------
Jozsef Kadlecsik (1):
netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length
net/netfilter/ipset/ip_set_bitmap_ipmac.c | 2 ++
net/netfilter/ipset/ip_set_hash_mac.c | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/1] netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length 2016-03-08 19:44 [PATCH 0/1] ipset patch for nf Jozsef Kadlecsik @ 2016-03-08 19:44 ` Jozsef Kadlecsik 2016-03-08 20:26 ` Daniel Borkmann 2016-03-10 18:28 ` [PATCH 0/1] ipset patch for nf Pablo Neira Ayuso 1 sibling, 1 reply; 5+ messages in thread From: Jozsef Kadlecsik @ 2016-03-08 19:44 UTC (permalink / raw) To: netfilter-devel; +Cc: Pablo Neira Ayuso Julia Lawall pointed out that IPSET_ATTR_ETHER netlink attribute length was not checked explicitly, just for the maximum possible size. Malicious netlink clients could send shorter attribute and thus resulting a kernel read after the buffer. The patch adds the explicit length checkings. Reported-by: Julia Lawall <julia.lawall@lip6.fr> Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> --- net/netfilter/ipset/ip_set_bitmap_ipmac.c | 2 ++ net/netfilter/ipset/ip_set_hash_mac.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/net/netfilter/ipset/ip_set_bitmap_ipmac.c index 29dde20..9a065f6 100644 --- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c +++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c @@ -267,6 +267,8 @@ bitmap_ipmac_uadt(struct ip_set *set, struct nlattr *tb[], e.id = ip_to_id(map, ip); if (tb[IPSET_ATTR_ETHER]) { + if (nla_len(tb[IPSET_ATTR_ETHER]) != ETH_ALEN) + return -IPSET_ERR_PROTOCOL; memcpy(e.ether, nla_data(tb[IPSET_ATTR_ETHER]), ETH_ALEN); e.add_mac = 1; } diff --git a/net/netfilter/ipset/ip_set_hash_mac.c b/net/netfilter/ipset/ip_set_hash_mac.c index f1e7d2c..8f004ed 100644 --- a/net/netfilter/ipset/ip_set_hash_mac.c +++ b/net/netfilter/ipset/ip_set_hash_mac.c @@ -110,7 +110,8 @@ hash_mac4_uadt(struct ip_set *set, struct nlattr *tb[], if (tb[IPSET_ATTR_LINENO]) *lineno = nla_get_u32(tb[IPSET_ATTR_LINENO]); - if (unlikely(!tb[IPSET_ATTR_ETHER])) + if (unlikely(!tb[IPSET_ATTR_ETHER] || + nla_len(tb[IPSET_ATTR_ETHER]) != ETH_ALEN)) return -IPSET_ERR_PROTOCOL; ret = ip_set_get_extensions(set, tb, &ext); -- 1.8.5.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length 2016-03-08 19:44 ` [PATCH 1/1] netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length Jozsef Kadlecsik @ 2016-03-08 20:26 ` Daniel Borkmann 2016-03-08 21:46 ` Jozsef Kadlecsik 0 siblings, 1 reply; 5+ messages in thread From: Daniel Borkmann @ 2016-03-08 20:26 UTC (permalink / raw) To: Jozsef Kadlecsik; +Cc: netfilter-devel, Pablo Neira Ayuso Hi Jozsef, On 03/08/2016 08:44 PM, Jozsef Kadlecsik wrote: > Julia Lawall pointed out that IPSET_ATTR_ETHER netlink attribute length > was not checked explicitly, just for the maximum possible size. Malicious > netlink clients could send shorter attribute and thus resulting a kernel > read after the buffer. > > The patch adds the explicit length checkings. > > Reported-by: Julia Lawall <julia.lawall@lip6.fr> > Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> > --- > net/netfilter/ipset/ip_set_bitmap_ipmac.c | 2 ++ > net/netfilter/ipset/ip_set_hash_mac.c | 3 ++- > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/net/netfilter/ipset/ip_set_bitmap_ipmac.c > index 29dde20..9a065f6 100644 > --- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c > +++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c > @@ -267,6 +267,8 @@ bitmap_ipmac_uadt(struct ip_set *set, struct nlattr *tb[], > > e.id = ip_to_id(map, ip); > if (tb[IPSET_ATTR_ETHER]) { > + if (nla_len(tb[IPSET_ATTR_ETHER]) != ETH_ALEN) > + return -IPSET_ERR_PROTOCOL; Any reason to not just remove the 'NLA_BINARY' from the policy? include/net/netlink.h +191: * Meaning of `len' field: [...] * NLA_BINARY Maximum length of attribute payload [...] * All other Minimum length of attribute payload validate_nla() will just do the right thing and check for minlen. Thanks, Daniel > memcpy(e.ether, nla_data(tb[IPSET_ATTR_ETHER]), ETH_ALEN); > e.add_mac = 1; > } > diff --git a/net/netfilter/ipset/ip_set_hash_mac.c b/net/netfilter/ipset/ip_set_hash_mac.c > index f1e7d2c..8f004ed 100644 > --- a/net/netfilter/ipset/ip_set_hash_mac.c > +++ b/net/netfilter/ipset/ip_set_hash_mac.c > @@ -110,7 +110,8 @@ hash_mac4_uadt(struct ip_set *set, struct nlattr *tb[], > if (tb[IPSET_ATTR_LINENO]) > *lineno = nla_get_u32(tb[IPSET_ATTR_LINENO]); > > - if (unlikely(!tb[IPSET_ATTR_ETHER])) > + if (unlikely(!tb[IPSET_ATTR_ETHER] || > + nla_len(tb[IPSET_ATTR_ETHER]) != ETH_ALEN)) > return -IPSET_ERR_PROTOCOL; > > ret = ip_set_get_extensions(set, tb, &ext); > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length 2016-03-08 20:26 ` Daniel Borkmann @ 2016-03-08 21:46 ` Jozsef Kadlecsik 0 siblings, 0 replies; 5+ messages in thread From: Jozsef Kadlecsik @ 2016-03-08 21:46 UTC (permalink / raw) To: Daniel Borkmann; +Cc: netfilter-devel, Pablo Neira Ayuso Hi Daniel, On Tue, 8 Mar 2016, Daniel Borkmann wrote: > On 03/08/2016 08:44 PM, Jozsef Kadlecsik wrote: > > Julia Lawall pointed out that IPSET_ATTR_ETHER netlink attribute length > > was not checked explicitly, just for the maximum possible size. Malicious > > netlink clients could send shorter attribute and thus resulting a kernel > > read after the buffer. > > > > The patch adds the explicit length checkings. > > > > Reported-by: Julia Lawall <julia.lawall@lip6.fr> > > Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> > > --- > > net/netfilter/ipset/ip_set_bitmap_ipmac.c | 2 ++ > > net/netfilter/ipset/ip_set_hash_mac.c | 3 ++- > > 2 files changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/net/netfilter/ipset/ip_set_bitmap_ipmac.c > > b/net/netfilter/ipset/ip_set_bitmap_ipmac.c > > index 29dde20..9a065f6 100644 > > --- a/net/netfilter/ipset/ip_set_bitmap_ipmac.c > > +++ b/net/netfilter/ipset/ip_set_bitmap_ipmac.c > > @@ -267,6 +267,8 @@ bitmap_ipmac_uadt(struct ip_set *set, struct nlattr > > *tb[], > > > > e.id = ip_to_id(map, ip); > > if (tb[IPSET_ATTR_ETHER]) { > > + if (nla_len(tb[IPSET_ATTR_ETHER]) != ETH_ALEN) > > + return -IPSET_ERR_PROTOCOL; > > Any reason to not just remove the 'NLA_BINARY' from the policy? > > include/net/netlink.h +191: > > * Meaning of `len' field: > [...] > * NLA_BINARY Maximum length of attribute payload > [...] > * All other Minimum length of attribute payload > > validate_nla() will just do the right thing and check for minlen. The exact payload length must be checked anyway, because we copy the attribute into a buffer[ETH_ALEN]. So that'd mean a bigger patch :-) Best regards, Jozsef - E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences H-1525 Budapest 114, POB. 49, Hungary ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/1] ipset patch for nf 2016-03-08 19:44 [PATCH 0/1] ipset patch for nf Jozsef Kadlecsik 2016-03-08 19:44 ` [PATCH 1/1] netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length Jozsef Kadlecsik @ 2016-03-10 18:28 ` Pablo Neira Ayuso 1 sibling, 0 replies; 5+ messages in thread From: Pablo Neira Ayuso @ 2016-03-10 18:28 UTC (permalink / raw) To: Jozsef Kadlecsik; +Cc: netfilter-devel On Tue, Mar 08, 2016 at 08:44:19PM +0100, Jozsef Kadlecsik wrote: > Hi Pablo, > > Please apply the next patch against the nf tree: > > - Julia Lawall pointed out that IPSET_ATTR_ETHER netlink attribute > length was not checked explicitly. The patch adds the missing > checkings. > > The patch should be applied to the older stable kernel branches too. > > Best regards, > Jozsef > > The following changes since commit 45040978c8994d1401baf5cc5ac71c1495d4e120: > > netfilter: ipset: Fix set:list type crash when flush/dump set in parallel (2016-02-24 20:32:21 +0100) > > are available in the git repository at: > > git://blackhole.kfki.hu/nf master This patch also came with the previous pull, so this will show up in the nf-next tree too. Thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-03-10 18:28 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-03-08 19:44 [PATCH 0/1] ipset patch for nf Jozsef Kadlecsik 2016-03-08 19:44 ` [PATCH 1/1] netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length Jozsef Kadlecsik 2016-03-08 20:26 ` Daniel Borkmann 2016-03-08 21:46 ` Jozsef Kadlecsik 2016-03-10 18:28 ` [PATCH 0/1] ipset patch for nf Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).