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@xxxxxxx>
> > Signed-off-by: Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxxx, kadlecsik.jozsef@xxxxxxxxxxxxx
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
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
