wcsncpy bug

wcsncpy bug

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 102 bytes --]

wcsncpy(d,s,n) did not decrease n while copying the '\0'
so when s[0]=0 and n=1 it wrote 2 zeros to d

[-- Attachment #2: wcsncpy.diff --]
[-- Type: text/x-diff, Size: 337 bytes --]

diff --git a/src/string/wcsncpy.c b/src/string/wcsncpy.c
index 0164208..fbd0631 100644
--- a/src/string/wcsncpy.c
+++ b/src/string/wcsncpy.c
@@ -3,7 +3,7 @@
 wchar_t *wcsncpy(wchar_t *d, const wchar_t *s, size_t n)
 {
 	wchar_t *a = d;
-	while (n && (*d++ = *s++)) n--;
+	while (n-- && (*d++ = *s++));
 	wmemset(d, 0, n);
 	return a;
 }

Re: wcsncpy bug

[Rich Felker]

On Mon, May 23, 2011 at 03:25:47AM +0200, Szabolcs Nagy wrote:
> wcsncpy(d,s,n) did not decrease n while copying the '\0'
> so when s[0]=0 and n=1 it wrote 2 zeros to d

> diff --git a/src/string/wcsncpy.c b/src/string/wcsncpy.c
> index 0164208..fbd0631 100644
> --- a/src/string/wcsncpy.c
> +++ b/src/string/wcsncpy.c
> @@ -3,7 +3,7 @@
>  wchar_t *wcsncpy(wchar_t *d, const wchar_t *s, size_t n)
>  {
>  	wchar_t *a = d;
> -	while (n && (*d++ = *s++)) n--;
> +	while (n-- && (*d++ = *s++));
>  	wmemset(d, 0, n);

Yes it was broken but this patch is too. It will now clobber all
memory if the source string does not contain a null terminator, since
the final value of n after the while loop will be (size_t)-1.

Thanks for catching this bug tho. I'll fix it.

Rich

Re: wcsncpy bug

[Rich Felker]

On Mon, May 23, 2011 at 03:25:47AM +0200, Szabolcs Nagy wrote:
> wcsncpy(d,s,n) did not decrease n while copying the '\0'
> so when s[0]=0 and n=1 it wrote 2 zeros to d

I believe I have fixed this bug, and similar bugs in strncat and
wcsncat. Please let me know if they still have problems.

Rich

Re: wcsncpy bug

[Szabolcs Nagy]

* Rich Felker <dalias@aerifal.cx> [2011-05-22 22:00:55 -0400]:
> I believe I have fixed this bug, and similar bugs in strncat and
> wcsncat. Please let me know if they still have problems.

yes, these look better

Re: wcsncpy bug

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2011-05-23 03:25:47 +0200]:
>  wchar_t *wcsncpy(wchar_t *d, const wchar_t *s, size_t n)
>  {
>  	wchar_t *a = d;
> -	while (n && (*d++ = *s++)) n--;
> +	while (n-- && (*d++ = *s++));

hm this code is not ok if n underflows here..

>  	wmemset(d, 0, n);
>  	return a;
>  }


let me try again..
maybe you can figure out a nicer one

wchar_t *wcsncpy(wchar_t *d, const wchar_t *s, size_t n)
{
	wchar_t *a = d;
	while (n && *s) {
		*d++ = *s++;
		n--;
	}
	wmemset(d, 0, n);
	return a;
}