search

kravietz / pam_tacplus

TACACS+ protocol client library and PAM module in C. This PAM module support authentication, authorization (account management) and accounting (session management)performed using TACACS+ protocol designed by Cisco.
GNU Lesser General Public License v3.0
132 stars 100 forks source link

compile issue in gcc 8 and above due to Werror=stringop-truncation #131

Closed paulo-erichsen closed 4 years ago

paulo-erichsen commented 4 years ago

This does not seem to compile in Arch Linux

$ uname -r
5.3.7-arch1-1-ARCH

$ git rev-parse HEAD
bedf7f93e5b96cd60354b870bfab05e8b975561c

$ gcc --version
gcc (GCC) 9.2.0

autoreconf

$ autoreconf -i
libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'config'.
libtoolize: copying file 'config/ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'config'.
libtoolize: copying file 'config/libtool.m4'
libtoolize: copying file 'config/ltoptions.m4'
libtoolize: copying file 'config/ltsugar.m4'
libtoolize: copying file 'config/ltversion.m4'
libtoolize: copying file 'config/lt~obsolete.m4'
configure.ac:25: installing 'config/compile'
configure.ac:33: installing 'config/config.guess'
configure.ac:33: installing 'config/config.sub'
configure.ac:20: installing 'config/install-sh'
configure.ac:20: installing 'config/missing'
Makefile.am: installing 'config/depcomp'

configure

$ ./configure 
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /usr/bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether make supports the include directive... yes (GNU style)
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking whether gcc understands -c and -o together... yes
checking dependency style of gcc... gcc3
checking how to run the C preprocessor... gcc -E
checking for grep that handles long lines and -e... /usr/bin/grep
checking for egrep... /usr/bin/grep -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking minix/config.h usability... no
checking minix/config.h presence... no
checking for minix/config.h... no
checking whether it is safe to define __EXTENSIONS__... yes
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking whether gcc understands -c and -o together... (cached) yes
checking dependency style of gcc... (cached) gcc3
checking whether ln -s works... yes
checking whether make sets $(MAKE)... (cached) yes
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking how to print strings... printf
checking for a sed that does not truncate output... /usr/bin/sed
checking for fgrep... /usr/bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking the maximum length of command line arguments... 1572864
checking how to convert x86_64-pc-linux-gnu file names to x86_64-pc-linux-gnu format... func_convert_file_noop
checking how to convert x86_64-pc-linux-gnu file names to toolchain format... func_convert_file_noop
checking for /usr/bin/ld option to reload object files... -r
checking for objdump... objdump
checking how to recognize dependent libraries... pass_all
checking for dlltool... no
checking how to associate runtime and link libraries... printf %s\n
checking for ar... ar
checking for archiver @FILE support... @
checking for strip... strip
checking for ranlib... ranlib
checking command to parse /usr/bin/nm -B output from gcc object... ok
checking for sysroot... no
checking for a working dd... /usr/bin/dd
checking how to truncate binary pipes... /usr/bin/dd bs=4096 count=1
checking for mt... no
checking if : is a manifest tool... no
checking for dlfcn.h... yes
checking for objdir... .libs
checking if gcc supports -fno-rtti -fno-exceptions... no
checking for gcc option to produce PIC... -fPIC -DPIC
checking if gcc PIC flag -fPIC -DPIC works... yes
checking if gcc static flag -static works... yes
checking if gcc supports -c -o file.o... yes
checking if gcc supports -c -o file.o... (cached) yes
checking whether the gcc linker (/usr/bin/ld -m elf_x86_64) supports shared libraries... yes
checking whether -lc should be explicitly linked in... no
checking dynamic linker characteristics... GNU/Linux ld.so
checking how to hardcode library paths into programs... immediate
checking whether stripping libraries is possible... yes
checking if libtool supports shared libraries... yes
checking whether to build shared libraries... yes
checking whether to build static libraries... no
checking for pam_start in -lpam... yes
checking for tac_connect in -ltac... no
checking for MD5_Init in -lcrypto... yes
checking for RAND_pseudo_bytes in -lcrypto... yes
checking for RAND_bytes in -lcrypto... yes
checking for pututxline in -lc... yes
checking for ANSI C header files... (cached) yes
checking arpa/inet.h usability... yes
checking arpa/inet.h presence... yes
checking for arpa/inet.h... yes
checking fcntl.h usability... yes
checking fcntl.h presence... yes
checking for fcntl.h... yes
checking netdb.h usability... yes
checking netdb.h presence... yes
checking for netdb.h... yes
checking netinet/in.h usability... yes
checking netinet/in.h presence... yes
checking for netinet/in.h... yes
checking for stdlib.h... (cached) yes
checking for string.h... (cached) yes
checking for strings.h... (cached) yes
checking sys/socket.h usability... yes
checking sys/socket.h presence... yes
checking for sys/socket.h... yes
checking sys/time.h usability... yes
checking sys/time.h presence... yes
checking for sys/time.h... yes
checking limits.h usability... yes
checking limits.h presence... yes
checking for limits.h... yes
checking syslog.h usability... yes
checking syslog.h presence... yes
checking for syslog.h... yes
checking for unistd.h... (cached) yes
checking openssl/md5.h usability... yes
checking openssl/md5.h presence... yes
checking for openssl/md5.h... yes
checking openssl/rand.h usability... yes
checking openssl/rand.h presence... yes
checking for openssl/rand.h... yes
checking sys/random.h usability... yes
checking sys/random.h presence... yes
checking for sys/random.h... yes
checking bsd/string.h usability... yes
checking bsd/string.h presence... yes
checking for bsd/string.h... yes
checking security/pam_appl.h usability... yes
checking security/pam_appl.h presence... yes
checking for security/pam_appl.h... yes
checking for an ANSI C-conforming const... yes
checking for size_t... yes
checking whether time.h and sys/time.h may both be included... yes
checking whether CC supports -fstack-protector-all... yes
no
checking whether CC supports -Wl,-z,relro... yes
no
checking whether CC supports -Wl,-z,now... yes
no
checking whether CC supports -fPIE... yes
no
checking whether CC supports -pie... yes
no
checking whether CC supports -U_FORTIFY_SOURCE... yes
no
checking whether CC supports -D_FORTIFY_SOURCE=2... yes
no
checking for stdlib.h... (cached) yes
checking for GNU libc compatible realloc... yes
checking sys/select.h usability... yes
checking sys/select.h presence... yes
checking for sys/select.h... yes
checking for sys/socket.h... (cached) yes
checking types of arguments for select... int,fd_set *,struct timeval *
checking return type of signal handlers... void
checking for bzero... yes
checking for gethostbyname... yes
checking for gettimeofday... yes
checking for inet_ntoa... yes
checking for select... yes
checking for socket... yes
checking for logwtmp... no
checking for getrandom... yes
checking for strtol... yes
checking for strlcpy... no
checking for abort... yes
checking that generated files are newer than configure... done
configure: creating ./config.status
config.status: creating Makefile
config.status: creating libtac.pc
config.status: creating pam_tacplus.spec
config.status: creating config.h
config.status: executing depfiles commands
config.status: executing libtool commands

make

$ make
make  all-am
make[1]: Entering directory '/home/user/git/pam_tacplus'
gcc -DHAVE_CONFIG_H -I.    -Wall -Wextra -Werror -I ./libtac/include  -g -O2 -MT tacc-tacc.o -MD -MP -MF .deps/tacc-tacc.Tpo -c -o tacc-tacc.o `test -f 'tacc.c' || echo './'`tacc.c
tacc.c: In function ‘main’:
tacc.c:391:9: error: ‘strncpy’ specified bound 4 equals destination size [-Werror=stringop-truncation]
  391 |         strncpy(utmpx.ut_id, tty + C_STRLEN("tty"), sizeof(utmpx.ut_id));
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:901: tacc-tacc.o] Error 1
make[1]: Leaving directory '/home/user/git/pam_tacplus'
make: *** [Makefile:472: all] Error 2
paulo-erichsen commented 4 years ago

FYI changing the strncpy to memcpy seems to allow me to compile, but I'm not 100% sure this is the right fix

diff --git a/tacc.c b/tacc.c
index 510d399..2293629 100644
--- a/tacc.c
+++ b/tacc.c
@@ -388,7 +388,7 @@ int main(int argc, char **argv) {
         utmpx.ut_type = USER_PROCESS;
         utmpx.ut_pid = getpid();
         xstrcpy(utmpx.ut_line, tty, sizeof(utmpx.ut_line));
-        strncpy(utmpx.ut_id, tty + C_STRLEN("tty"), sizeof(utmpx.ut_id));
+        memcpy(utmpx.ut_id, tty + C_STRLEN("tty"), sizeof(utmpx.ut_id));
         xstrcpy(utmpx.ut_host, "dialup", sizeof(utmpx.ut_host));
         utmpx.ut_tv.tv_sec = tv.tv_sec;
         utmpx.ut_tv.tv_usec = tv.tv_usec;
kravietz commented 4 years ago

This is easily fixed by decreasing the string length as in sizeof(utmpx.ut_id) - 1 to account the the trailing zero.