From 710b87dcb9b237b20c995086d9a2fd393330a006 Mon Sep 17 00:00:00 2001 From: Karsten Keil Date: Wed, 18 Jul 2018 07:27:41 +0200 Subject: [PATCH] Fix possible buffer overflows detected by newer GCC versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GCC reports problems like this: gcc -DHAVE_CONFIG_H -I. -I../include -I../include -Wall -Werror -I./include -D_FORTIFY_SOURCE=2 -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt -MT layer3/q931.lo -MD -MP -MF layer3/.deps/q931.Tpo -c layer3/q931.c -fPIC -DPIC -o layer3/.libs/q931.o In file included from /usr/include/string.h:494, from layer3/q931.c:22: In function ‘strncpy’, inlined from ‘mi_encode_redirecting_nr’ at layer3/q931.c:531:3: /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ forming offset [25, 31] is out of the bounds [0, 24] of object ‘ie’ with type ‘unsigned char[24]’ [-Werror=array-bounds] return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Thanks to Tobias Powalowski for reporting this This commit fixes issue #9 on github. --- example/testcon.c | 8 ++++---- lib/layer3/q931.c | 4 ++-- tools/logger_config_parser.l | 4 ++-- tools/rename.c | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/example/testcon.c b/example/testcon.c index 781ac24..25ddff4 100644 --- a/example/testcon.c +++ b/example/testcon.c @@ -1309,7 +1309,7 @@ static int do_setup(devinfo_t *di) { int main(int argc, char *argv[]) { - char FileName[200],FileNameOut[200], FileNameIn[200]; + char FileName[200],FileNameOut[208], FileNameIn[208]; int aidx=1,para=1, idx; char sw; devinfo_t mISDN; @@ -1373,7 +1373,7 @@ int main(int argc, char *argv[]) } else { if (para==1) { if (argc > 1) - strncpy(FileName, argv[aidx], 199); + strncpy(FileName, argv[aidx], sizeof(FileName) - 1); para++; } else { fprintf(stderr,"Undefined argument %s\n",argv[aidx]); @@ -1391,8 +1391,8 @@ int main(int argc, char *argv[]) return 1; } close(err); - sprintf(FileNameOut,"%s.out", FileName); - sprintf(FileNameIn,"%s.in", FileName); + snprintf(FileNameOut, sizeof(FileNameOut) - 1, "%s.out", FileName); + snprintf(FileNameIn, sizeof(FileNameIn) - 1, "%s.in", FileName); if (0>(mISDN.save = open(FileNameIn, O_WRONLY|O_CREAT|O_TRUNC,S_IRWXU))) { printf("TestmISDN cannot open %s due to %s\n",FileNameIn, strerror(errno)); diff --git a/lib/layer3/q931.c b/lib/layer3/q931.c index 0e23aa5..a371deb 100644 --- a/lib/layer3/q931.c +++ b/lib/layer3/q931.c @@ -506,7 +506,7 @@ mi_encode_called_nr(struct l3_msg *l3m, char *nr, unsigned int type, unsigned in int mi_encode_redirecting_nr(struct l3_msg *l3m, char *nr, int pres, unsigned int type, unsigned int plan, int reason) { - unsigned char ie[24]; + unsigned char ie[32]; int l; if (nr == NULL || *nr == 0) /* not provided */ @@ -537,7 +537,7 @@ mi_encode_redirecting_nr(struct l3_msg *l3m, char *nr, int pres, unsigned int ty int mi_encode_redirection_nr(struct l3_msg *l3m, char *nr, int pres, unsigned int type, unsigned int plan) { - unsigned char ie[24]; + unsigned char ie[32]; int l; if (nr == NULL || *nr == 0) /* not provided */ diff --git a/tools/logger_config_parser.l b/tools/logger_config_parser.l index fc3f872..72b1aff 100644 --- a/tools/logger_config_parser.l +++ b/tools/logger_config_parser.l @@ -460,9 +460,9 @@ static int setLayer3(int, int); if (defController == currentController) fprintf(stderr, "dumpfile ignored for global section\n"); else - strncpy(currentController->dumpfile, yytext, MAX_FILE_NAME); + strncpy(currentController->dumpfile, yytext, MAX_FILE_NAME - 1); } else if (filetyp == 2) { - strncpy(currentController->logfile, yytext, MAX_FILE_NAME); + strncpy(currentController->logfile, yytext, MAX_FILE_NAME - 1); } else { fprintf(stderr, "Got unhandled filetype %d with %s - abort\n", filetyp, yytext); return -1; diff --git a/tools/rename.c b/tools/rename.c index 3748289..e97495a 100644 --- a/tools/rename.c +++ b/tools/rename.c @@ -88,7 +88,7 @@ next_dev: } found_dev: devname.id = i; - strncpy(devname.name, argv[2], MISDN_MAX_IDLEN); + strncpy(devname.name, argv[2], MISDN_MAX_IDLEN - 1); ret = ioctl(sock, IMSETDEVNAME, &devname); if (ret < 0) { fprintf(stderr, "Cannot set device name for port %d: %s\n", i, strerror(errno));