On Thu, Mar 10, 2016 at 06:00:01PM +0100, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > On Wed, Mar 09, 2016 at 12:04:21AM +0100, Florian Westphal wrote: > > > We copy accoring to ->target|matchsize, so check that the netlink attribute > > > (which can include padding and might be larger) contains enough data. > > > > > > Reported-by: Julia Lawall <Julia.Lawall@xxxxxxx> > > > Signed-off-by: Florian Westphal <fw@xxxxxxxxx> > > > > I think xt_check_match() and xt_check_target() already validate this > > for us. > > But AFAICS we copy before this: > > nft_target_init: > > size = XT_ALIGN(nla_len(tb[NFTA_TARGET_INFO])); > ... > target_compat_from_user(target, nla_data(tb[NFTA_TARGET_INFO]), info); > -> memcpy(out, in, t->targetsize); > > xt_check_target(&par, size, proto, inv); // checks size vs. targetsize > > 'target' is sized based on target->targetsize in > nft_target_select_ops(). > > So if tb[NFTA_TARGET_INFO] is != t->targetsize we might copy more > data than whats in 'in', no? Right, if the user passes less information, then we would be copying more than what we have. I'm going to place this in the nf-next, thanks Florian. -- 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
