From owner-freebsd-audit Fri Aug 4 14: 4:35 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 E92B737BA3B; Fri, 4 Aug 2000 14:04:27 -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 PAA75332; Fri, 4 Aug 2000 15:04:24 -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 PAA12903; Fri, 4 Aug 2000 15:04:15 -0600 (MDT) Message-Id: <200008042104.PAA12903@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 13:58:16 PDT." References: Date: Fri, 04 Aug 2000 15:04:15 -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, Warner Losh wrote: : : > : - strncpy(buf, result, resultlen); : > : + strncpy(buf, result, resultlen - 1); : > : buf[resultlen] = '\0'; : > : free(result); : > : } : > : : > : > This change is wrong. The strcpy puts upto resultlen characters into : > buf, and then null terminates it at the resultlen + 1st character : > (counting from 1). The strncpy should therefore not have the -1. Or : > the line setting the buf[] = 0 should have it as well. : : 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. No. The change isn't right. I know that strncpy doesn't NUL terminate when it copies N characters. That's not the point. The ponit is that you terminate at the resultlen+1ths character, so you should be strncopying resultlen characters, like it was doing before. In other words, the old code is correct. Looking more closely at the code, in the too long case, strncpy copies bytes to buf[0..resultlen-1]. the next line then NUL terminates at buf[resultlen]. Clearly, the last character isn't overwritten, and all the bytes are weel defined in the too long case. In the other case, strncpy is well defined. With your change, the strncpy would copy bytes to buf[0..resultlen-2], leaving buf[resultlen - 1] undefined, and buf[reultlen] = NUL at the end. That's why your change is wrong. Or at least inconsistant. It introduces the potential for a garbage byte. Warner To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message