From mboxrd@z Thu Jan  1 00:00:00 1970
From: Florian Westphal <fw@strlen.de>
Subject: Re: [1/1] iptables: Fix crash on malformed iptables-restore
Date: Thu, 18 May 2017 12:01:13 +0200
Message-ID: <20170518100113.GB1272@breakpoint.cc>
References: <1495039661-9178-1-git-send-email-ojford@gmail.com>
 <20170517221939.GA1748@breakpoint.cc>
 <CAGMVOduDWZ8i1ZKMSjf1SU7Pm66WW48DRNModU3f77wwrj5o=g@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Florian Westphal <fw@strlen.de>, netfilter-devel@vger.kernel.org
To: Oliver Ford <ojford@gmail.com>
Return-path: <netfilter-devel-owner@vger.kernel.org>
Received: from Chamillionaire.breakpoint.cc ([146.0.238.67]:52482 "EHLO
        Chamillionaire.breakpoint.cc" rhost-flags-OK-OK-OK-OK)
        by vger.kernel.org with ESMTP id S1755497AbdERKB7 (ORCPT
        <rfc822;netfilter-devel@vger.kernel.org>);
        Thu, 18 May 2017 06:01:59 -0400
Content-Disposition: inline
In-Reply-To: <CAGMVOduDWZ8i1ZKMSjf1SU7Pm66WW48DRNModU3f77wwrj5o=g@mail.gmail.com>
Sender: netfilter-devel-owner@vger.kernel.org
List-ID: <netfilter-devel.vger.kernel.org>

Oliver Ford <ojford@gmail.com> wrote:
> On Wed, May 17, 2017 at 11:19 PM, Florian Westphal <fw@strlen.de> wrote:
> > Oliver Ford <ojford@gmail.com> wrote:
> >> Fixes the crash reported in Bugzilla #1131 where a malformed
> >> parameter that specifies the table option can create an invalid
> >> pointer.
> >>
> >> Improves the tables option check to find a beginning '-' followed by
> >> a 't' anywhere in the parameter.
> >
> >> diff --git a/iptables/iptables-restore.c b/iptables/iptables-restore.c
> >> index 876fe06..70dc7eb 100644
> >> --- a/iptables/iptables-restore.c
> >> +++ b/iptables/iptables-restore.c
> >> @@ -162,8 +162,7 @@ static void add_param_to_argv(char *parsestart)
> >>                       param_buffer[param_len] = '\0';
> >>
> >>                       /* check if table name specified */
> >> -                     if (!strncmp(param_buffer, "-t", 2)
> >> -                         || !strncmp(param_buffer, "--table", 8)) {
> >> +                     if (param_buffer[0] == '-' && strchr(param_buffer, 't')) {
> >>                               xtables_error(PARAMETER_PROBLEM,
> >>                               "The -t option (seen in line %u) cannot be "
> >>                               "used in iptables-restore.\n", line);
> >
> > strchr() can only called if param_buffer[1] != '-', else this breaks
> > something like:
> >
> > # Generated by iptables-save v1.6.0 on Thu May 18 00:00:38 2017
> > *filter
> > :INPUT ACCEPT [0:0]
> > :FORWARD ACCEPT [0:0]
> > :OUTPUT ACCEPT [0:0]
> > --delete foo
> > COMMIT
> >
> > (use '-n' option to iptables).
> 
> Good point I'll check for that and send an updated patch.

Forgot to mention: Would be nice to explain in the commit message
why this is needed, e.g. -nt, --tab, etc. are not caught by current
test.