From f196de88cdd9764ddc2e4de737a960972d82fe9d Mon Sep 17 00:00:00 2001 From: "Jose M. Guisado Gomez" Date: Fri, 16 Aug 2019 11:25:11 +0200 Subject: src: fix strncpy -Wstringop-truncation warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit -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 Signed-off-by: Pablo Neira Ayuso --- src/read_config_yy.y | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) (limited to 'src/read_config_yy.y') 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); -- cgit v1.2.3