[PATCH v3] kconfig/lxdialog: replace strcpy() with strncpy() in inputbox.c

[PATCH v3] kconfig/lxdialog: replace strcpy() with strncpy() in inputbox.c

[Suchit Karunakaran]

strcpy() performs no bounds checking and can lead to buffer overflows if
the input string exceeds the destination buffer size. This patch replaces
it with strncpy(), and null terminates the input string.

Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
---
 scripts/kconfig/lxdialog/inputbox.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/lxdialog/inputbox.c b/scripts/kconfig/lxdialog/inputbox.c
index 3c6e24b20f5b..5e4a131724f2 100644
--- a/scripts/kconfig/lxdialog/inputbox.c
+++ b/scripts/kconfig/lxdialog/inputbox.c
@@ -39,8 +39,10 @@ int dialog_inputbox(const char *title, const char *prompt, int height, int width
 
 	if (!init)
 		instr[0] = '\0';
-	else
-		strcpy(instr, init);
+	else {
+		strncpy(instr, init, sizeof(dialog_input_result) - 1);
+		instr[sizeof(dialog_input_result) - 1] = '\0';
+	}
 
 do_resize:
 	if (getmaxy(stdscr) <= (height - INPUTBOX_HEIGHT_MIN))
-- 
2.50.1

Re: [PATCH v3] kconfig/lxdialog: replace strcpy() with strncpy() in inputbox.c

[Nicolas Schier]

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

On Sun, Jul 27, 2025 at 10:14:33PM +0530 Suchit Karunakaran wrote:
> strcpy() performs no bounds checking and can lead to buffer overflows if
> the input string exceeds the destination buffer size. This patch replaces
> it with strncpy(), and null terminates the input string.
> 
> Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> ---
>  scripts/kconfig/lxdialog/inputbox.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Nicolas Schier <nicolas.schier@linux.dev>

thanks for your contribution!

If you want to continue contributing, you might want to check-out tools
like b4 which simplifies sending and tracking patch-sets.

Kind regards,
Nicolas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3] kconfig/lxdialog: replace strcpy() with strncpy() in inputbox.c

[Suchit K]

On Mon, 28 Jul 2025 at 00:49, Nicolas Schier <nicolas.schier@linux.dev> wrote:
>
> On Sun, Jul 27, 2025 at 10:14:33PM +0530 Suchit Karunakaran wrote:
> > strcpy() performs no bounds checking and can lead to buffer overflows if
> > the input string exceeds the destination buffer size. This patch replaces
> > it with strncpy(), and null terminates the input string.
> >
> > Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> > ---
> >  scripts/kconfig/lxdialog/inputbox.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
>
> Reviewed-by: Nicolas Schier <nicolas.schier@linux.dev>
>
> thanks for your contribution!
>
> If you want to continue contributing, you might want to check-out tools
> like b4 which simplifies sending and tracking patch-sets.
>

Thank you very much! Please let me know if there's anything else I
should do. Also, I'd appreciate it if you could take a look at my
other patch that replaces strcpy with snprintf. Thanks again!

Re: [PATCH v3] kconfig/lxdialog: replace strcpy() with strncpy() in inputbox.c

[Greg KH]

On Sun, Jul 27, 2025 at 10:14:33PM +0530, Suchit Karunakaran wrote:
> strcpy() performs no bounds checking and can lead to buffer overflows if
> the input string exceeds the destination buffer size. This patch replaces
> it with strncpy(), and null terminates the input string.
> 
> Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> ---
>  scripts/kconfig/lxdialog/inputbox.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/kconfig/lxdialog/inputbox.c b/scripts/kconfig/lxdialog/inputbox.c
> index 3c6e24b20f5b..5e4a131724f2 100644
> --- a/scripts/kconfig/lxdialog/inputbox.c
> +++ b/scripts/kconfig/lxdialog/inputbox.c
> @@ -39,8 +39,10 @@ int dialog_inputbox(const char *title, const char *prompt, int height, int width
>  
>  	if (!init)
>  		instr[0] = '\0';
> -	else
> -		strcpy(instr, init);
> +	else {
> +		strncpy(instr, init, sizeof(dialog_input_result) - 1);
> +		instr[sizeof(dialog_input_result) - 1] = '\0';

As this is a userspace tool, why is this change needed at all?  How can
this overflow and if it does, what happens?

thanks,

greg k-h

Re: [PATCH v3] kconfig/lxdialog: replace strcpy() with strncpy() in inputbox.c

[Suchit K]

On Mon, 28 Jul 2025 at 09:59, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sun, Jul 27, 2025 at 10:14:33PM +0530, Suchit Karunakaran wrote:
> > strcpy() performs no bounds checking and can lead to buffer overflows if
> > the input string exceeds the destination buffer size. This patch replaces
> > it with strncpy(), and null terminates the input string.
> >
> > Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> > ---
> >  scripts/kconfig/lxdialog/inputbox.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/kconfig/lxdialog/inputbox.c b/scripts/kconfig/lxdialog/inputbox.c
> > index 3c6e24b20f5b..5e4a131724f2 100644
> > --- a/scripts/kconfig/lxdialog/inputbox.c
> > +++ b/scripts/kconfig/lxdialog/inputbox.c
> > @@ -39,8 +39,10 @@ int dialog_inputbox(const char *title, const char *prompt, int height, int width
> >
> >       if (!init)
> >               instr[0] = '\0';
> > -     else
> > -             strcpy(instr, init);
> > +     else {
> > +             strncpy(instr, init, sizeof(dialog_input_result) - 1);
> > +             instr[sizeof(dialog_input_result) - 1] = '\0';
>
> As this is a userspace tool, why is this change needed at all?  How can
> this overflow and if it does, what happens?
>

Hi Greg. The primary motivation for this patch was the deprecation of
strcpy(). Additionally, I believed there was a possibility of a buffer
overflow if the initial string accidentally exceeded the length of
instr, although the chances might be low.

Re: [PATCH v3] kconfig/lxdialog: replace strcpy() with strncpy() in inputbox.c

[Greg KH]

On Mon, Jul 28, 2025 at 10:18:35AM +0530, Suchit K wrote:
> On Mon, 28 Jul 2025 at 09:59, Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Sun, Jul 27, 2025 at 10:14:33PM +0530, Suchit Karunakaran wrote:
> > > strcpy() performs no bounds checking and can lead to buffer overflows if
> > > the input string exceeds the destination buffer size. This patch replaces
> > > it with strncpy(), and null terminates the input string.
> > >
> > > Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> > > ---
> > >  scripts/kconfig/lxdialog/inputbox.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/scripts/kconfig/lxdialog/inputbox.c b/scripts/kconfig/lxdialog/inputbox.c
> > > index 3c6e24b20f5b..5e4a131724f2 100644
> > > --- a/scripts/kconfig/lxdialog/inputbox.c
> > > +++ b/scripts/kconfig/lxdialog/inputbox.c
> > > @@ -39,8 +39,10 @@ int dialog_inputbox(const char *title, const char *prompt, int height, int width
> > >
> > >       if (!init)
> > >               instr[0] = '\0';
> > > -     else
> > > -             strcpy(instr, init);
> > > +     else {
> > > +             strncpy(instr, init, sizeof(dialog_input_result) - 1);
> > > +             instr[sizeof(dialog_input_result) - 1] = '\0';
> >
> > As this is a userspace tool, why is this change needed at all?  How can
> > this overflow and if it does, what happens?
> >
> 
> Hi Greg. The primary motivation for this patch was the deprecation of
> strcpy(). Additionally, I believed there was a possibility of a buffer
> overflow if the initial string accidentally exceeded the length of
> instr, although the chances might be low.

Is strcpy() being deprecated in userspace?  I think it's a core part of
the C language specification :)

Again, how can that buffer be "too large"?

thanks,

greg k-h

Re: [PATCH v3] kconfig/lxdialog: replace strcpy() with strncpy() in inputbox.c

[Suchit K]

Resending because I unknowingly disabled plain text mode. Sorry about that.

>
> Is strcpy() being deprecated in userspace?  I think it's a core part of
> the C language specification :)
>

My apologies. I was under the assumption that all folders within the
kernel repository adhered to the kernel coding guidelines, only to
realize that these guidelines primarily apply to kernel space code.
You're right, strcpy() isn't deprecated in userspace but as far as I
know some compilers emit warnings to replace it with other functions
since it's unsafe.

> Again, how can that buffer be "too large"?
>

Tbh I'm not sure. I was glancing through the code in mconf.c and there
were some calls to dialog_inputbox() with file names and a variable
length string being passed for the init argument. So, I thought there
might be some chance of overflowing.

Re: [PATCH v3] kconfig/lxdialog: replace strcpy() with strncpy() in inputbox.c

[Masahiro Yamada]

On Mon, Jul 28, 2025 at 1:29 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Sun, Jul 27, 2025 at 10:14:33PM +0530, Suchit Karunakaran wrote:
> > strcpy() performs no bounds checking and can lead to buffer overflows if
> > the input string exceeds the destination buffer size. This patch replaces
> > it with strncpy(), and null terminates the input string.
> >
> > Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> > ---
> >  scripts/kconfig/lxdialog/inputbox.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/kconfig/lxdialog/inputbox.c b/scripts/kconfig/lxdialog/inputbox.c
> > index 3c6e24b20f5b..5e4a131724f2 100644
> > --- a/scripts/kconfig/lxdialog/inputbox.c
> > +++ b/scripts/kconfig/lxdialog/inputbox.c
> > @@ -39,8 +39,10 @@ int dialog_inputbox(const char *title, const char *prompt, int height, int width
> >
> >       if (!init)
> >               instr[0] = '\0';
> > -     else
> > -             strcpy(instr, init);
> > +     else {
> > +             strncpy(instr, init, sizeof(dialog_input_result) - 1);
> > +             instr[sizeof(dialog_input_result) - 1] = '\0';
>
> As this is a userspace tool, why is this change needed at all?  How can
> this overflow and if it does, what happens?

The buffer size (2049 byte) is large enough, and
buffer overflow is unlikely to happen in practical use cases.
If it does, I think the program will crash.





-- 
Best Regards
Masahiro Yamada

Re: [PATCH v3] kconfig/lxdialog: replace strcpy() with strncpy() in inputbox.c

[Masahiro Yamada]

On Mon, Jul 28, 2025 at 1:44 AM Suchit Karunakaran
<suchitkarunakaran@gmail.com> wrote:
>
> strcpy() performs no bounds checking and can lead to buffer overflows if
> the input string exceeds the destination buffer size. This patch replaces
> it with strncpy(), and null terminates the input string.
>
> Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> ---
>  scripts/kconfig/lxdialog/inputbox.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/kconfig/lxdialog/inputbox.c b/scripts/kconfig/lxdialog/inputbox.c
> index 3c6e24b20f5b..5e4a131724f2 100644
> --- a/scripts/kconfig/lxdialog/inputbox.c
> +++ b/scripts/kconfig/lxdialog/inputbox.c
> @@ -39,8 +39,10 @@ int dialog_inputbox(const char *title, const char *prompt, int height, int width
>
>         if (!init)
>                 instr[0] = '\0';
> -       else
> -               strcpy(instr, init);
> +       else {
> +               strncpy(instr, init, sizeof(dialog_input_result) - 1);
> +               instr[sizeof(dialog_input_result) - 1] = '\0';
> +       }


Applied to linux-kbuild. Thanks.



-- 
Best Regards
Masahiro Yamada