From owner-freebsd-stable@FreeBSD.ORG Wed Nov 27 07:57:42 2013 Return-Path: Delivered-To: freebsd-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id CC6D9567 for ; Wed, 27 Nov 2013 07:57:42 +0000 (UTC) Received: from sola.nimnet.asn.au (paqi.nimnet.asn.au [115.70.110.159]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 053E62E20 for ; Wed, 27 Nov 2013 07:57:41 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by sola.nimnet.asn.au (8.14.2/8.14.2) with ESMTP id rAR7uahG034742; Wed, 27 Nov 2013 18:56:36 +1100 (EST) (envelope-from smithi@nimnet.asn.au) Date: Wed, 27 Nov 2013 18:56:36 +1100 (EST) From: Ian Smith To: Ben Morrow Subject: Re: ipfw table add problem In-Reply-To: <20131126124757.GA9974@anubis.morrow.me.uk> Message-ID: <20131127011442.P78756@sola.nimnet.asn.au> References: <52911993.8010108@ipfw.ru> <529259DE.2040701@FreeBSD.org> <20131125152238.S78756@sola.nimnet.asn.au> <1385391778.1220.4.camel@revolution.hippie.lan> <20131126001806.27951AD3DBF@rock.dv.isc.org> <20131126124757.GA9974@anubis.morrow.me.uk> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; CHARSET=US-ASCII Content-ID: <20131127181617.C78756@sola.nimnet.asn.au> Cc: "Alexander V.Chernikov" , Ian Lepore , freebsd-ipfw@freebsd.org, freebsd-stable@freebsd.org, Luigi Rizzo , =?ISO-8859-1?Q?=D6zkan_KIRIK?= X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Nov 2013 07:57:42 -0000 On Tue, 26 Nov 2013 12:48:01 +0000, Ben Morrow wrote: > To: freebsd-stable@freebsd.org Restoring cc ipfw@ and others after the inet_pton side?thread in stable@. grepping /usr/src for inet_pton suggests that a behavioural change in inet_pton at this stage seems rather unlikely :) > Quoth Michael Butler : > > > > Misinterpreting "10.2.3.01" as "0.0.0.10/32" without so much as a > > warning from either inet_pton() or ipfw is an egregious breach of POLA, > > That's not a bug in inet_pton, though, that's a bug in ipfw. It's > blindly passing the string to atoi or some such when inet_pton fails, > and ignoring the fact it doesn't consume the whole string. Indeed it is; strtol actually, which quits at the first (here decimal) non-digit. It does return a pointer to it though, and a check for that character being '.' seems like a fair indicator of a failed dotted quad? http://svnweb.freebsd.org/base/head/sbin/ipfw/ipfw2.c?revision=250759&view=co if (ishexnumber(*arg) != 0 || *arg == ':') { /* Remove / if exists */ if ((p = strchr(arg, '/')) != NULL) { *p = '\0'; mask = atoi(p + 1); } if (inet_pton(AF_INET, arg, paddr) == 1) { ... } else if (inet_pton(AF_INET6, arg, paddr) == 1) { ... } else { /* Port or any other key */ key = strtol(arg, &p, 10); /* Skip non-base 10 entries like 'fa1' */ if (p != arg) { pkey = (uint32_t *)paddr; *pkey = htonl(key); type = IPFW_TABLE_CIDR; addrlen = sizeof(uint32_t); } } } if (type == 0 && strchr(arg, '.') == NULL) { /* Assume interface name. Copy significant data only */ ... } if (type == 0) { if (lookup_host(arg, (struct in_addr *)paddr) != 0) errx(EX_NOHOST, "hostname ``%s'' unknown", arg); ... } ... } I'm mostly a pascal programmer (oh, the shame! :) so I can easily misuse C pointers, but my reading of strtol(3) leads to suggest something like: } else { /* Port or any other key */ key = strtol(arg, &p, 10); /* Skip non-base 10 entries like 'fa1' */ if (p != arg) { + /* IPv4 address that failed inet_pton */ + if (*p == '.') { + errx(EX_DATAERR, "bad IPv4 address"); + } pkey = (uint32_t *)paddr; *pkey = htonl(key); type = IPFW_TABLE_CIDR; addrlen = sizeof(uint32_t); } } cheers, Ian