Re: [PATCH nf] ebtables: arpreply: Add the standard target sanity check

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Mon, May 15, 2017 at 06:56:02PM +0200, Pablo Neira Ayuso wrote:
> On Fri, May 12, 2017 at 05:44:10PM +0800, gfree.wind@vip.163.com wrote:
> > From: Gao Feng <gfree.wind@vip.163.com>
> > 
> > The info->target is from userspace and it would be used directly.
> > So we need to add the sanity check to make sure it is a valid standard
> > target, although the ebtables tool has already checked it. Kernel need
> > to check anything from userspace.
> > 
> > If the target was set as an evil value, it would break the ebtables
> > and cause a panic. Because the non-standard target is treated as one
> > offset.
> > 
> > Signed-off-by: Gao Feng <gfree.wind@vip.163.com>
> > ---
> >  net/bridge/netfilter/ebt_arpreply.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/net/bridge/netfilter/ebt_arpreply.c b/net/bridge/netfilter/ebt_arpreply.c
> > index 5929309..c4886d9 100644
> > --- a/net/bridge/netfilter/ebt_arpreply.c
> > +++ b/net/bridge/netfilter/ebt_arpreply.c
> > @@ -68,6 +68,9 @@ static int ebt_arpreply_tg_check(const struct xt_tgchk_param *par)
> >  	if (e->ethproto != htons(ETH_P_ARP) ||
> >  	    e->invflags & EBT_IPROTO)
> >  		return -EINVAL;
> > +	if (INVALID_TARGET)
> 
> Please, add:
> 
> static inline bool ebt_invalid_target(int target)
> {
>         return (target < -NUM_STANDARD_TARGETS || target >= 0);
> }
> 
> and use it in this fix.
> 
> So we can get rid of this obscure INVALID_TARGET macro.

Once this propagates to nf-next.git, you can send a follow up patch to
use this new function from more spots, so we can kill INVALID_TARGET
for good.