linux-kernel - Re: [PATCH] staging: rtl8192e: Replace strcpy with strscpy in rtl819x_translate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZsYoYe+8k0YEyS2U@embed-PC.myguest.virtualbox.org>
Date: Wed, 21 Aug 2024 23:18:17 +0530
From: Abhishek Tamboli <abhishektamboli9@...il.com>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: gregkh@...uxfoundation.org, tdavies@...kphysics.net,
	philipp.g.hortmann@...il.com, garyrookard@...tmail.org,
	linux-staging@...ts.linux.dev, skhan@...uxfoundation.org,
	rbmarliere@...il.com,
	linux-kernel-mentees@...ts.linuxfoundation.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: rtl8192e: Replace strcpy with strscpy in
 rtl819x_translate_scan

On Tue, Aug 20, 2024 at 10:29:47PM +0300, Dan Carpenter wrote:
> On Wed, Aug 21, 2024 at 12:12:16AM +0530, Abhishek Tamboli wrote:
> > Replace strcpy() with strscpy() in rtl819x_translate_scan() 
> > function to ensure buffer safety.
> > 
> > Signed-off-by: Abhishek Tamboli <abhishektamboli9@...il.com>
> > ---
> >  drivers/staging/rtl8192e/rtllib_wx.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/rtl8192e/rtllib_wx.c b/drivers/staging/rtl8192e/rtllib_wx.c
> > index fbd4ec824084..970b7fcb3f7e 100644
> > --- a/drivers/staging/rtl8192e/rtllib_wx.c
> > +++ b/drivers/staging/rtl8192e/rtllib_wx.c
> > @@ -61,7 +61,7 @@ static inline char *rtl819x_translate_scan(struct rtllib_device *ieee,
> >  	iwe.cmd = SIOCGIWNAME;
> >  	for (i = 0; i < ARRAY_SIZE(rtllib_modes); i++) {
> >  		if (network->mode & BIT(i)) {
> > -			strcpy(pname, rtllib_modes[i]);
> > +			strscpy(pname, rtllib_modes[i], sizeof(pname));
>                                                                ^^^^^
> pname is a pointer, not an array, so this doesn't work.
Thanks for pointing out the issue with strscpy.
> >  			pname += strlen(rtllib_modes[i]);
>                         ^^^^^^^^
> pname is incremented here.
> 
> What this loop is doing is that it's going through all the network modes and
> adding to the string.  You should look at the rtllib_modes[] array and ensure
> that if we printed every string it would fit into pname.  (Currently that is not
> the case.  Probably not all network modes are possible.  But I have looked at
> this code and I'm saying that we should just ensure that we could handle it if
> they were all possible).
I understand that the size of proto_name is insufficient if all network modes 
from rtllib_modes[] are copied, so I need to increase its size.

Given this, would it be better to use strcat?
This would eliminate the need for the pname pointer and align with 
the current code's method of concatenating the rtllib_modes.

Regards,
Abhishek