[musl] [PATCH] src: string: Replace unsafe wcscpy with wcsncat in wcscat()

[musl] [PATCH] src: string: Replace unsafe wcscpy with wcsncat in wcscat()

[Anton Moryakov]

Static analyzer reported:
PROC_USE.VULNERABLE: Use of vulnerable function 'wcscpy' at wcscat.c:5. This function is unsafe, use wcsncpy instead.

Corrections explained:
Replaced the vulnerable function wcscpy with wcsncat in wcscat()
to prevent potential buffer overflows. 

wcscpy(dest + wcslen(dest), src); was unsafe because it could overwrite
memory beyond the allocated buffer.

Now using:
    wcsncat(dest, src, wcslen(src));

This change improves security but does not guarantee buffer overflow protection.
To fully ensure safety, the function should also receive the destination buffer
size as a parameter.

Triggers found by static analyzer Svace.

Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>

---
 src/string/wcscat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/string/wcscat.c b/src/string/wcscat.c
index d4f00ebd..7599a6eb 100644
--- a/src/string/wcscat.c
+++ b/src/string/wcscat.c
@@ -2,6 +2,6 @@
 
 wchar_t *wcscat(wchar_t *restrict dest, const wchar_t *restrict src)
 {
-	wcscpy(dest + wcslen(dest), src);
+	wcsncat(dest, src, wcslen(src));
 	return dest;
 }
-- 
2.30.2

Re: [musl] [PATCH] src: string: Replace unsafe wcscpy with wcsncat in wcscat()

[Yao Zi]

On Sun, Feb 16, 2025 at 08:25:53PM +0300, Anton Moryakov wrote:
> Static analyzer reported:
> PROC_USE.VULNERABLE: Use of vulnerable function 'wcscpy' at wcscat.c:5. This function is unsafe, use wcsncpy instead.
> 
> Corrections explained:
> Replaced the vulnerable function wcscpy with wcsncat in wcscat()
> to prevent potential buffer overflows. 
> 
> wcscpy(dest + wcslen(dest), src); was unsafe because it could overwrite
> memory beyond the allocated buffer.
> 
> Now using:
>     wcsncat(dest, src, wcslen(src));
> 
> This change improves security but does not guarantee buffer overflow protection.
> To fully ensure safety, the function should also receive the destination buffer
> size as a parameter.

wcscat() itself isn't a safe function. I don't see any improvements with
this patch.

Cheers,
Yao Zi

> 
> Triggers found by static analyzer Svace.
> 
> Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
> 
> ---
>  src/string/wcscat.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/string/wcscat.c b/src/string/wcscat.c
> index d4f00ebd..7599a6eb 100644
> --- a/src/string/wcscat.c
> +++ b/src/string/wcscat.c
> @@ -2,6 +2,6 @@
>  
>  wchar_t *wcscat(wchar_t *restrict dest, const wchar_t *restrict src)
>  {
> -	wcscpy(dest + wcslen(dest), src);
> +	wcsncat(dest, src, wcslen(src));
>  	return dest;
>  }
> -- 
> 2.30.2
> 

Re: [musl] [PATCH] src: string: Replace unsafe wcscpy with wcsncat in wcscat()

[Anton Moryakov]

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

Thanks!

вс, 16 февр. 2025 г. в 20:40, Yao Zi <ziyao@disroot.org>:

> On Sun, Feb 16, 2025 at 08:25:53PM +0300, Anton Moryakov wrote:
> > Static analyzer reported:
> > PROC_USE.VULNERABLE: Use of vulnerable function 'wcscpy' at wcscat.c:5.
> This function is unsafe, use wcsncpy instead.
> >
> > Corrections explained:
> > Replaced the vulnerable function wcscpy with wcsncat in wcscat()
> > to prevent potential buffer overflows.
> >
> > wcscpy(dest + wcslen(dest), src); was unsafe because it could overwrite
> > memory beyond the allocated buffer.
> >
> > Now using:
> >     wcsncat(dest, src, wcslen(src));
> >
> > This change improves security but does not guarantee buffer overflow
> protection.
> > To fully ensure safety, the function should also receive the destination
> buffer
> > size as a parameter.
>
> wcscat() itself isn't a safe function. I don't see any improvements with
> this patch.
>
> Cheers,
> Yao Zi
>
> >
> > Triggers found by static analyzer Svace.
> >
> > Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
> >
> > ---
> >  src/string/wcscat.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/string/wcscat.c b/src/string/wcscat.c
> > index d4f00ebd..7599a6eb 100644
> > --- a/src/string/wcscat.c
> > +++ b/src/string/wcscat.c
> > @@ -2,6 +2,6 @@
> >
> >  wchar_t *wcscat(wchar_t *restrict dest, const wchar_t *restrict src)
> >  {
> > -     wcscpy(dest + wcslen(dest), src);
> > +     wcsncat(dest, src, wcslen(src));
> >       return dest;
> >  }
> > --
> > 2.30.2
> >
>

[-- Attachment #2: Type: text/html, Size: 2230 bytes --]