From owner-freebsd-ipfw@FreeBSD.ORG Mon Nov 29 20:26:16 2004 Return-Path: Delivered-To: freebsd-ipfw@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8461F16A4CE for ; Mon, 29 Nov 2004 20:26:16 +0000 (GMT) Received: from smtpout.mac.com (smtpout.mac.com [17.250.248.86]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6915E43D66 for ; Mon, 29 Nov 2004 20:26:16 +0000 (GMT) (envelope-from cswiger@mac.com) Received: from mac.com (smtpin08-en2 [10.13.10.153]) by smtpout.mac.com (Xserve/MantshX 2.0) with ESMTP id iATKQFYp014787; Mon, 29 Nov 2004 12:26:15 -0800 (PST) Received: from [10.1.1.245] (nfw1.codefab.com [199.103.21.225]) (authenticated bits=0) by mac.com (Xserve/smtpin08/MantshX 4.0) with ESMTP id iATKQDn6008764; Mon, 29 Nov 2004 12:26:15 -0800 (PST) In-Reply-To: <20041129192514.GA7331@odin.ac.hmc.edu> References: <20041129192514.GA7331@odin.ac.hmc.edu> Mime-Version: 1.0 (Apple Message framework v619) Content-Type: text/plain; charset=US-ASCII; format=flowed Message-Id: Content-Transfer-Encoding: 7bit From: Charles Swiger Date: Mon, 29 Nov 2004 15:26:12 -0500 To: Brooks Davis X-Mailer: Apple Mail (2.619) cc: ipfw@freebsd.org Subject: Re: strncmp usage in ipfw X-BeenThere: freebsd-ipfw@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: IPFW Technical Discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 Nov 2004 20:26:16 -0000 On Nov 29, 2004, at 2:25 PM, Brooks Davis wrote: > char *var; > if (!strncmp(var, "str", strlen(var))) > ... > [ ... ] > Was use of this idiom deliberate or accidental? I can't speak for the author, but using the "n"-for-length variant of the string and printf() family of functions is considered an important saftey practice, especially for network/firewall/IDS software which may be exposed to externally generated data which contains deliberately malicious string lengths. Since the topic came up, it's also potentially dangerous to write code like: char errstr[1024]; /* ...intervening code... */ snprintf(errstr, 1024, "..."); ...because people making changes to the code may change the size of errstr without changing the 1024 in the snprintf(). Using a macro for the size is better practice: #define ERRLEN (1024) char errstr[ERRLEN]; /* ...intervening code... */ snprintf(errstr, ERRLEN, "..."); ...but the strong recommendation I've seen is to always use sizeof(): snprintf(errstr, sizeof(errstr), ...) This brings me back to your point with regard to partial matches; it might be the case that the IPFW code could use char arrays and sizeof(var) rather than char *'s and strlen(var) for some cases? The former approach would not only address your concerns, Brooks, but also be faster. Otherwise, I suspect that: char *var; if (!strncmp(var, "str", strlen(var))) ... ...should become: #define STR "str" char *var; if (!strncmp(var, STR, sizeof(STR))) ... -- -Chuck