diff options
author | Jose M. Guisado Gomez <guigom@riseup.net> | 2019-08-16 11:25:11 +0200 |
---|---|---|
committer | Pablo Neira Ayuso <pablo@netfilter.org> | 2019-08-17 11:53:57 +0200 |
commit | f196de88cdd9764ddc2e4de737a960972d82fe9d (patch) | |
tree | 7fd6779ee6dbcf9cda6e09078b393cae5736b3f5 | |
parent | de12e29bf35b1da51944c826beb34acf48d90289 (diff) |
src: fix strncpy -Wstringop-truncation warnings
-Wstringop-truncation warning was introduced in GCC-8 as truncation
checker for strncpy and strncat.
Systems using gcc version >= 8 would receive the following warnings:
read_config_yy.c: In function ‘yyparse’:
read_config_yy.y:1594:2: warning: ‘strncpy’ specified bound 16 equals destination size [-Wstringop-truncation]
1594 | strncpy(policy->name, $2, CTD_HELPER_NAME_LEN);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
read_config_yy.y:1384:2: warning: ‘strncpy’ specified bound 256 equals destination size [-Wstringop-truncation]
1384 | strncpy(conf.stats.logfile, $2, FILENAME_MAXLEN);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
read_config_yy.y:692:2: warning: ‘strncpy’ specified bound 108 equals destination size [-Wstringop-truncation]
692 | strncpy(conf.local.path, $2, UNIX_PATH_MAX);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
read_config_yy.y:169:2: warning: ‘strncpy’ specified bound 256 equals destination size [-Wstringop-truncation]
169 | strncpy(conf.lockfile, $2, FILENAME_MAXLEN);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
read_config_yy.y:119:2: warning: ‘strncpy’ specified bound 256 equals destination size [-Wstringop-truncation]
119 | strncpy(conf.logfile, $2, FILENAME_MAXLEN);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
main.c: In function ‘main’:
main.c:168:5: warning: ‘strncpy’ specified bound 4096 equals destination size [-Wstringop-truncation]
168 | strncpy(config_file, argv[i], PATH_MAX);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fix the issue by checking for string length first. Also using
snprintf instead.
In addition, correct an off-by-one when warning about maximum config
file path length.
Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
-rw-r--r-- | include/conntrackd.h | 6 | ||||
-rw-r--r-- | include/helper.h | 2 | ||||
-rw-r--r-- | include/local.h | 4 | ||||
-rw-r--r-- | src/main.c | 7 | ||||
-rw-r--r-- | src/read_config_yy.y | 39 |
5 files changed, 38 insertions, 20 deletions
diff --git a/include/conntrackd.h b/include/conntrackd.h index 81dff22..fe9ec18 100644 --- a/include/conntrackd.h +++ b/include/conntrackd.h @@ -85,9 +85,9 @@ union inet_address { #define CONFIG(x) conf.x struct ct_conf { - char logfile[FILENAME_MAXLEN]; + char logfile[FILENAME_MAXLEN + 1]; int syslog_facility; - char lockfile[FILENAME_MAXLEN]; + char lockfile[FILENAME_MAXLEN + 1]; int hashsize; /* hashtable size */ int channel_num; int channel_default; @@ -132,7 +132,7 @@ struct ct_conf { int prio; } sched; struct { - char logfile[FILENAME_MAXLEN]; + char logfile[FILENAME_MAXLEN + 1]; int syslog_facility; size_t buffer_size; } stats; diff --git a/include/helper.h b/include/helper.h index d15c1c6..d540667 100644 --- a/include/helper.h +++ b/include/helper.h @@ -13,7 +13,7 @@ struct pkt_buff; #define CTD_HELPER_POLICY_MAX 4 struct ctd_helper_policy { - char name[CTD_HELPER_NAME_LEN]; + char name[CTD_HELPER_NAME_LEN + 1]; uint32_t expect_timeout; uint32_t expect_max; }; diff --git a/include/local.h b/include/local.h index 22859d7..9379446 100644 --- a/include/local.h +++ b/include/local.h @@ -7,12 +7,12 @@ struct local_conf { int reuseaddr; - char path[UNIX_PATH_MAX]; + char path[UNIX_PATH_MAX + 1]; }; struct local_server { int fd; - char path[UNIX_PATH_MAX]; + char path[UNIX_PATH_MAX + 1]; }; /* callback return values */ @@ -120,8 +120,8 @@ do_chdir(const char *d) int main(int argc, char *argv[]) { + char config_file[PATH_MAX + 1] = {}; int ret, i, action = -1; - char config_file[PATH_MAX] = {}; int type = 0; struct utsname u; int version, major, minor; @@ -165,13 +165,12 @@ int main(int argc, char *argv[]) break; case 'C': if (++i < argc) { - strncpy(config_file, argv[i], PATH_MAX); - if (strlen(argv[i]) >= PATH_MAX){ - config_file[PATH_MAX-1]='\0'; + if (strlen(argv[i]) > PATH_MAX) { dlog(LOG_WARNING, "Path to config file" " to long. Cutting it down to %d" " characters", PATH_MAX); } + snprintf(config_file, PATH_MAX, "%s", argv[i]); break; } show_usage(argv[0]); diff --git a/src/read_config_yy.y b/src/read_config_yy.y index 4311cd6..a4aa7f5 100644 --- a/src/read_config_yy.y +++ b/src/read_config_yy.y @@ -116,7 +116,12 @@ logfile_bool : T_LOG T_OFF logfile_path : T_LOG T_PATH_VAL { - strncpy(conf.logfile, $2, FILENAME_MAXLEN); + if (strlen($2) > FILENAME_MAXLEN) { + dlog(LOG_ERR, "LogFile path is longer than %u characters", + FILENAME_MAXLEN); + exit(EXIT_FAILURE); + } + snprintf(conf.logfile, FILENAME_MAXLEN, "%s", $2); free($2); }; @@ -166,7 +171,12 @@ syslog_facility : T_SYSLOG T_STRING lock : T_LOCK T_PATH_VAL { - strncpy(conf.lockfile, $2, FILENAME_MAXLEN); + if (strlen($2) > FILENAME_MAXLEN) { + dlog(LOG_ERR, "LockFile path is longer than %u characters", + FILENAME_MAXLEN); + exit(EXIT_FAILURE); + } + snprintf(conf.lockfile, FILENAME_MAXLEN, "%s", $2); free($2); }; @@ -689,13 +699,13 @@ unix_options: unix_option : T_PATH T_PATH_VAL { - strncpy(conf.local.path, $2, UNIX_PATH_MAX); - free($2); - if (conf.local.path[UNIX_PATH_MAX - 1]) { - dlog(LOG_ERR, "UNIX Path is longer than %u characters", - UNIX_PATH_MAX - 1); + if (strlen($2) > UNIX_PATH_MAX) { + dlog(LOG_ERR, "Path is longer than %u characters", + UNIX_PATH_MAX); exit(EXIT_FAILURE); } + snprintf(conf.local.path, UNIX_PATH_MAX, "%s", $2); + free($2); }; unix_option : T_BACKLOG T_NUMBER @@ -1381,7 +1391,12 @@ stat_logfile_bool : T_LOG T_OFF stat_logfile_path : T_LOG T_PATH_VAL { - strncpy(conf.stats.logfile, $2, FILENAME_MAXLEN); + if (strlen($2) > FILENAME_MAXLEN) { + dlog(LOG_ERR, "stats LogFile path is longer than %u characters", + FILENAME_MAXLEN); + exit(EXIT_FAILURE); + } + snprintf(conf.stats.logfile, FILENAME_MAXLEN, "%s", $2); free($2); }; @@ -1589,11 +1604,15 @@ helper_type: T_HELPER_POLICY T_STRING '{' helper_policy_list '}' exit(EXIT_FAILURE); break; } + if (strlen($2) > CTD_HELPER_NAME_LEN) { + dlog(LOG_ERR, "Helper Policy is longer than %u characters", + CTD_HELPER_NAME_LEN); + exit(EXIT_FAILURE); + } policy = (struct ctd_helper_policy *) &e->data; - strncpy(policy->name, $2, CTD_HELPER_NAME_LEN); + snprintf(policy->name, CTD_HELPER_NAME_LEN, "%s", $2); free($2); - policy->name[CTD_HELPER_NAME_LEN-1] = '\0'; /* Now object is complete. */ e->type = SYMBOL_HELPER_POLICY_EXPECT_ROOT; stack_item_push(&symbol_stack, e); |