Date: Fri, 04 Aug 2000 15:04:15 -0600 From: Warner Losh <imp@village.org> To: Kris Kennaway <kris@hub.freebsd.org> Cc: audit@FreeBSD.ORG Subject: Re: ether_line() patch Message-ID: <200008042104.PAA12903@harmony.village.org> In-Reply-To: Your message of "Fri, 04 Aug 2000 13:58:16 PDT." <Pine.BSF.4.21.0008041355250.64303-100000@hub.freebsd.org> References: <Pine.BSF.4.21.0008041355250.64303-100000@hub.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <Pine.BSF.4.21.0008041355250.64303-100000@hub.freebsd.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200008042104.PAA12903>