[libdvdnav-devel] [PATCH] sprintf -> strcpy

[Petri Hintukainen]

On ke, 2015-09-09 at 21:45 +1200, Lawrence D'Oliveiro wrote:
> On Wed, 09 Sep 2015 12:23:09 +0300, Petri Hintukainen wrote:
> 
> > On ke, 2015-09-09 at 19:57 +1200, Lawrence D'Oliveiro wrote:
> >
> >> On Wed,  9 Sep 2015 10:11:16 +0300, Petri Hintukainen wrote:
> >> 
> >>> -    sprintf( filename, "/VIDEO_TS/VIDEO_TS.VOB" );
> >>> +    strcpy( filename, "/VIDEO_TS/VIDEO_TS.VOB" );
> >> [etc]
> >> 
> >> I would not use either. I would use strncpy instead.
> > 
> > Why ?
> > 
> > Constant string is copied to constant-size buffer. Buffer overflow
> > checks should be done at compile time, not runtime.
> 
>     ldo at theon:c_try> cat overflow.c
>     #include <string.h>
> 
>     int main(void)
>       {
>         char buf[5];
>         strcpy(buf, "THIS STRING IS MUCH TOO LARGE");
>         return
>             0;
>       } /*main*/
>     ldo at theon:c_try> gcc -c overflow.c 
>     ldo at theon:c_try> 
> 
> As you can see, there is no error reported.
>
> What compile-time option should I use to ask for one?

Depends on compiler ? Using -O did the trick here:

$ gcc --version
gcc (Ubuntu 4.9.2-10ubuntu13) 4.9.2

$ gcc -c overflow.c -O
In file included from /usr/include/string.h:639:0,
                 from overflow.c:1:
In function ‘strcpy’,
    inlined from ‘main’ at overflow.c:6:10:
/usr/include/x86_64-linux-gnu/bits/string3.h:110:3: warning: call to
__builtin___strcpy_chk will always overflow destination buffer
   return __builtin___strcpy_chk (__dest, __src, __bos (__dest));
   ^


Anyway, using strncpy or strlcpy would not fix the program. It would
just hide the problem (no warning) and fail at runtime. Not because of
stack overflow, but because of truncated file name.