[commit] printcmd.c, print_scalar_formatted, use strncpy.

[commit] printcmd.c, print_scalar_formatted, use strncpy.

[Michael Snyder]

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

checked in.


[-- Attachment #2: strncpy2.txt --]
[-- Type: text/plain, Size: 607 bytes --]

2011-03-02  Michael Snyder  <msnyder@vmware.com>

	* printcmd.c (print_scalar_formatted): Use strncpy for safety.

Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.192
diff -u -p -u -p -r1.192 printcmd.c
--- printcmd.c	26 Feb 2011 02:07:08 -0000	1.192
+++ printcmd.c	2 Mar 2011 22:55:38 -0000
@@ -533,7 +533,7 @@ print_scalar_formatted (const void *vala
 	    if (*cp == '\0')
 	      cp--;
 	  }
-	strcpy (buf, cp);
+	strncpy (buf, cp, sizeof (bits));
 	fputs_filtered (buf, stream);
       }
       break;

Re: [commit] printcmd.c, print_scalar_formatted, use strncpy.

[Pedro Alves]

On Wednesday 02 March 2011 23:00:23, Michael Snyder wrote:
> strncpy2.txt
>   2011-03-02  Michael Snyder  <msnyder@vmware.com>
> 
>         * printcmd.c (print_scalar_formatted): Use strncpy for safety.
> 
> Index: printcmd.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/printcmd.c,v
> retrieving revision 1.192
> diff -u -p -u -p -r1.192 printcmd.c
> --- printcmd.c  26 Feb 2011 02:07:08 -0000      1.192
> +++ printcmd.c  2 Mar 2011 22:55:38 -0000
> @@ -533,7 +533,7 @@ print_scalar_formatted (const void *vala
>             if (*cp == '\0')
>               cp--;
>           }
> -       strcpy (buf, cp);
> +       strncpy (buf, cp, sizeof (bits));
>         fputs_filtered (buf, stream);
>        }
>        break;

We've been through this recently...  This is not safe.

-- 
Pedro Alves

Re: [commit] printcmd.c, print_scalar_formatted, use strncpy.

[Michael Snyder]

Pedro Alves wrote:
> On Wednesday 02 March 2011 23:00:23, Michael Snyder wrote:
>> strncpy2.txt
>>   2011-03-02  Michael Snyder  <msnyder@vmware.com>
>>
>>         * printcmd.c (print_scalar_formatted): Use strncpy for safety.
>>
>> Index: printcmd.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/printcmd.c,v
>> retrieving revision 1.192
>> diff -u -p -u -p -r1.192 printcmd.c
>> --- printcmd.c  26 Feb 2011 02:07:08 -0000      1.192
>> +++ printcmd.c  2 Mar 2011 22:55:38 -0000
>> @@ -533,7 +533,7 @@ print_scalar_formatted (const void *vala
>>             if (*cp == '\0')
>>               cp--;
>>           }
>> -       strcpy (buf, cp);
>> +       strncpy (buf, cp, sizeof (bits));
>>         fputs_filtered (buf, stream);
>>        }
>>        break;
> 
> We've been through this recently...  This is not safe.
> 

I'm slow today -- how is it not safe?

(note that sizeof (bits) is smaller than sizeof (buff)).

Re: [commit] printcmd.c, print_scalar_formatted, use strncpy.

[Pedro Alves]

On Wednesday 02 March 2011 23:56:16, Michael Snyder wrote:
> Pedro Alves wrote:
> > On Wednesday 02 March 2011 23:00:23, Michael Snyder wrote:
> >>   2011-03-02  Michael Snyder  <msnyder@vmware.com>
> >>
> >>         * printcmd.c (print_scalar_formatted): Use strncpy for safety.

> >> -       strcpy (buf, cp);
> >> +       strncpy (buf, cp, sizeof (bits));
> >>         fputs_filtered (buf, stream);

> > We've been through this recently...  This is not safe.
> > 
> 
> I'm slow today -- how is it not safe?
> 
> (note that sizeof (bits) is smaller than sizeof (buff)).

In that case the change is useless.

In the case you're thinking strncpy is safer
(to prevent overflow), it does _not_ add the terminating
null byte to the destination.  See its linux man page, please.

If the change hadn't been useless for the reason above,
you'd've shifted the problem elsewhere, not made things safer,
because if the safety net had been hit, the fputs_filtered
in the next line would do undefined things, like for example
crash, when trying to print a not null-terminated BUF.

strncpy was _not_ designed as a safe version of strcpy.
It was designed to be used on fixed length fields
in things like databases, where if you don't null terminate
the destination, it's okay, because _users_ of the data
in the buffer know how to handle that.

Here <http://www.lysator.liu.se/c/rat/d11.html>:

"4.11.2.4  The strncpy function

 strncpy was initially introduced into the C library to deal with
 fixed-length name fields in structures such as directory entries.
 Such fields are not used in the same way as strings: the trailing
 null is unnecessary for a maximum-length field, and setting
 trailing bytes for shorter names to null assures efficient
 field-wise comparisons.  strncpy is not by origin a ``bounded
 strcpy,'' and the Committee has preferred to recognize existing
 practice rather than alter the function to better suit it to such use."

So, to recap, simply s/strcpy/strncpy/ is not any safer.

-- 
Pedro Alves