From owner-freebsd-audit Fri Aug 4 14:22:43 2000 Delivered-To: freebsd-audit@freebsd.org Received: from rover.village.org (rover.village.org [204.144.255.49]) by hub.freebsd.org (Postfix) with ESMTP id EB0B637B75B; Fri, 4 Aug 2000 14:22:37 -0700 (PDT) (envelope-from imp@harmony.village.org) Received: from harmony.village.org (harmony.village.org [10.0.0.6]) by rover.village.org (8.9.3/8.9.3) with ESMTP id PAA75387; Fri, 4 Aug 2000 15:22:36 -0600 (MDT) (envelope-from imp@harmony.village.org) Received: from harmony.village.org (localhost.village.org [127.0.0.1]) by harmony.village.org (8.9.3/8.8.3) with ESMTP id PAA13032; Fri, 4 Aug 2000 15:22:28 -0600 (MDT) Message-Id: <200008042122.PAA13032@harmony.village.org> To: Kris Kennaway Subject: Re: ether_line() patch Cc: audit@FreeBSD.ORG In-reply-to: Your message of "Fri, 04 Aug 2000 14:10:03 PDT." References: Date: Fri, 04 Aug 2000 15:22:28 -0600 From: Warner Losh Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG In message Kris Kennaway writes: : On Fri, 4 Aug 2000, Kris Kennaway wrote: : : > strncpy does not null-terminate if strlen(result) == resultlen. In that : > case the buf[resultlen] character will be stomped by the NULL - it's a : > trivial change, but I think it's correct. : : Actually we were both wrong - this strncpy was just bogus and did no : bounds checking. This patch hunk should be better. : : @@ -156,8 +178,8 @@ : strlen(ether_a), &result, &resultlen)) { : continue; : } : - strncpy(buf, result, resultlen); : - buf[resultlen] = '\0'; : + strncpy(buf, result, sizeof(buf) - 1); : + buf[sizeof(buf)] = '\0'; : free(result); : } : #endif Well, you weren't listening to me when I was wrong :-) At least about the parts I was right about :-). This is incorrect too. It should be buf[sizeof(buf) - 1] = '\0'; because the valid range of buf is [0..sizeof(buf) - 1]. You don't need the -1 on strncpy, but that's a style issue. The post conditions are identical with it or without it: strncpy(buf, result, sizeof(buf)); buf[sizeof(buf) - 1] = '\0'; The -1 might optimize the copying of one byte, but often it can cause more code to execute if the expression to the left of it isn't a compile time constant. Warner To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message