From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oliver Ford Subject: Re: [1/1] iptables: Fix crash on malformed iptables-restore Date: Thu, 18 May 2017 10:33:30 +0100 Message-ID: References: <1495039661-9178-1-git-send-email-ojford@gmail.com> <20170517221939.GA1748@breakpoint.cc> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: netfilter-devel@vger.kernel.org To: Florian Westphal Return-path: Received: from mail-vk0-f51.google.com ([209.85.213.51]:33195 "EHLO mail-vk0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754998AbdERJdb (ORCPT ); Thu, 18 May 2017 05:33:31 -0400 Received: by mail-vk0-f51.google.com with SMTP id x71so20204517vkd.0 for ; Thu, 18 May 2017 02:33:31 -0700 (PDT) In-Reply-To: <20170517221939.GA1748@breakpoint.cc> Sender: netfilter-devel-owner@vger.kernel.org List-ID: On Wed, May 17, 2017 at 11:19 PM, Florian Westphal wrote: > Oliver Ford 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.