Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

strncpy and snprintf bounds checks #150

Closed
dkg opened this issue Feb 26, 2018 · 1 comment
Closed

strncpy and snprintf bounds checks #150

dkg opened this issue Feb 26, 2018 · 1 comment
Assignees

Comments

@dkg
Copy link
Collaborator

dkg commented Feb 26, 2018

over in https://bugs.debian.org/891551, James Cowgill writes:

  I recently performed an (unofficial) archive rebuild with GCC 8 on
  mips64el. The main purpose of the rebuild was to discover mips toolchain
  regressions, however I noticed this error in the logs which might be
  interesting to you:

  > faketime.c: In function 'main':
  > faketime.c:289:45: error: '%s' directive output may be truncated writing up to 4095 bytes into a region of size between 0 and 4095 [-Werror=format-truncation=]
  [ 47 more citation lines. Click/Enter to hide. ]
  >      snprintf(shared_objs, PATH_BUFSIZE, "%s %s", sem_name, shm_name);
  >                                              ^~             ~~~~~~~~
  > faketime.c:289:5: note: 'snprintf' output between 2 and 8192 bytes into a destination of size 4096
  >      snprintf(shared_objs, PATH_BUFSIZE, "%s %s", sem_name, shm_name);
  >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  > In file included from libfaketime.c:49:
  > libfaketime.c: In function 'fake_clock_gettime':
  > libfaketime.c:1986:24: error: cast between incompatible function types from 'int (*)(pthread_mutex_t *)' {aka 'int (*)(union <anonymous> *)'} to 'void (*)(void *)'
  > [-Werror=cast-function-type]
  >    pthread_cleanup_push((void (*)(void *))pthread_mutex_unlock, (void *)&time_mutex);
  >                         ^
  > cc1: all warnings being treated as errors
  > Makefile:98: recipe for target 'faketime' failed
  > make[2]: *** [faketime] Error 1
  > make[2]: *** Waiting for unfinished jobs....
  > libfaketime.c: In function 'fake_clock_gettime.part.4':
  > libfaketime.c:2081:7: error: 'strncpy' specified bound 256 equals destination size [-Werror=stringop-truncation]
  >        strncpy(user_faked_time, tmp_env, BUFFERLEN);
  >        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  > libfaketime.c:2081:7: error: 'strncpy' specified bound 256 equals destination size [-Werror=stringop-truncation]
  >        strncpy(user_faked_time, tmp_env, BUFFERLEN);
  >        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  > libfaketime.c: In function 'ftpl_init':
  > libfaketime.c:1831:12: error: 'strncpy' specified bound 1024 equals destination size [-Werror=stringop-truncation]
  >      (void) strncpy(ft_spawn_target, getenv("FAKETIME_SPAWN_TARGET"), 1024);
  >             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  > libfaketime.c:1892:5: error: 'strncpy' specified bound 8192 equals destination size [-Werror=stringop-truncation]
  >      strncpy(user_faked_time_fmt, tmp_env, BUFSIZ);
  >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  > libfaketime.c: In function 'ftpl_init':
  > libfaketime.c:1831:12: error: 'strncpy' specified bound 1024 equals destination size [-Werror=stringop-truncation]
  >      (void) strncpy(ft_spawn_target, getenv("FAKETIME_SPAWN_TARGET"), 1024);
  >             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  > libfaketime.c:1892:5: error: 'strncpy' specified bound 8192 equals destination size [-Werror=stringop-truncation]
  >      strncpy(user_faked_time_fmt, tmp_env, BUFSIZ);
  >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  > cc1: all warnings being treated as errors
  > Makefile:92: recipe for target 'libfaketime.o' failed
  > make[2]: *** [libfaketime.o] Error 1
  > cc1: all warnings being treated as errors
  > Makefile:92: recipe for target 'libfaketimeMT.o' failed
  > make[2]: *** [libfaketimeMT.o] Error 1
  > make[2]: Leaving directory '/<<PKGBUILDDIR>>/src'
  > Makefile:7: recipe for target 'all' failed
  > make[1]: *** [all] Error 2
  > make[1]: Leaving directory '/<<PKGBUILDDIR>>'
  > dh_auto_build: make -j3 "INSTALL=install --strip-program=true" returned exit code 2
  > debian/rules:22: recipe for target 'build-arch' failed
  > make: *** [build-arch] Error 2
  > dpkg-buildpackage: error: debian/rules build-arch subprocess returned exit status 2

  As you probably know, you need to be careful using strncpy.

  Instead of:
   char out[SIZE];
   strncpy(out, in, SIZE);

  You need to do:
   char out[SIZE]
   strncpy(out, in, SIZE - 1);
   out[SIZE - 1] = '\0';

  See strcpy(3)
@wolfcw wolfcw self-assigned this Mar 1, 2018
tpetazzoni added a commit to tpetazzoni/libfaketime that referenced this issue May 17, 2018
gcc 8.x introduced stricter checking on strncpy(), and causes the
following build failures:

libfaketime.c: In function 'fake_clock_gettime.part.4':
libfaketime.c:2134:7: error: 'strncpy' specified bound 256 equals destination size [-Werror=stringop-truncation]
       strncpy(user_faked_time, tmp_env, BUFFERLEN);
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libfaketime.c:2134:7: error: 'strncpy' specified bound 256 equals destination size [-Werror=stringop-truncation]
       strncpy(user_faked_time, tmp_env, BUFFERLEN);
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libfaketime.c: In function 'ftpl_init':
libfaketime.c:1884:12: error: 'strncpy' specified bound 1024 equals destination size [-Werror=stringop-truncation]
     (void) strncpy(ft_spawn_target, getenv("FAKETIME_SPAWN_TARGET"), 1024);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libfaketime.c:1945:5: error: 'strncpy' specified bound 8192 equals destination size [-Werror=stringop-truncation]
     strncpy(user_faked_time_fmt, tmp_env, BUFSIZ);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libfaketime.c: In function 'ftpl_init':
libfaketime.c:1884:12: error: 'strncpy' specified bound 1024 equals destination size [-Werror=stringop-truncation]
     (void) strncpy(ft_spawn_target, getenv("FAKETIME_SPAWN_TARGET"), 1024);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
libfaketime.c:1945:5: error: 'strncpy' specified bound 8192 equals destination size [-Werror=stringop-truncation]
     strncpy(user_faked_time_fmt, tmp_env, BUFSIZ);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This commit fixes that by making sure we keep one final byte for the
nul terminator, as suggested by
wolfcw#150.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
@wolfcw
Copy link
Owner

wolfcw commented Jun 3, 2018

fixed by 0d96436 , thanks

@wolfcw wolfcw closed this as completed Jun 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants