Hi Jose,
PLEASE DO NOT DO THIS! See at end
On Fri, Aug 16, 2019 at 11:25:11AM +0200, Jose M. Guisado Gomez wrote:
> -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@xxxxxxxxxx>
> ---
> src/main.c | 4 ++--
> src/read_config_yy.y | 35 +++++++++++++++++++++++++++--------
> 2 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/src/main.c b/src/main.c
> index 7062e12..d4fbbfa 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -165,13 +165,13 @@ 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';
> dlog(LOG_WARNING, "Path to config file"
> " to long. Cutting it down to %d"
> - " characters", PATH_MAX);
> + " characters", PATH_MAX - 1);
> }
> + snprintf(config_file, PATH_MAX, "%.*s", PATH_MAX - 1, argv[i]);
> break;
> }
> show_usage(argv[0]);
> diff --git a/src/read_config_yy.y b/src/read_config_yy.y
> index 4311cd6..696a86d 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, "Filename is longer than %u characters",
> + FILENAME_MAXLEN - 1);
> + exit(EXIT_FAILURE);
> + }
> + snprintf(conf.logfile, FILENAME_MAXLEN, "%.*s", FILENAME_MAXLEN - 1, $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, "Filename is longer than %u characters",
> + FILENAME_MAXLEN - 1);
> + exit(EXIT_FAILURE);
> + }
> + snprintf(conf.lockfile, FILENAME_MAXLEN, "%.*s", FILENAME_MAXLEN - 1, $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]) {
> + if (strlen($2) >= UNIX_PATH_MAX) {
> dlog(LOG_ERR, "UNIX Path is longer than %u characters",
> UNIX_PATH_MAX - 1);
> exit(EXIT_FAILURE);
> }
> + snprintf(conf.local.path, UNIX_PATH_MAX, "%.*s", UNIX_PATH_MAX - 1, $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, "Log file path is longer than %u characters",
> + FILENAME_MAXLEN - 1);
> + exit(EXIT_FAILURE);
> + }
> + snprintf(conf.stats.logfile, FILENAME_MAXLEN, "%.*s", FILENAME_MAXLEN - 1, $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, "CTD helper name is longer than %u characters",
> + CTD_HELPER_NAME_LEN - 1);
> + 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", CTD_HELPER_NAME_LEN - 1, $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);
> --
> 2.23.0.rc1
>
There is absolutely no need to change code to eliminate GCC warnings.
If you are satisfied that the code is good, put these lines near the start, e.g.
before any #include lines:
> #pragma GCC diagnostic ignored "-Wpragmas"
> #pragma GCC diagnostic ignored "-Wstringop-truncation"
The first pragma stops old GCC compilers warning about the unrecognised second
pragma
The second pragma suppresses the new warning.
Cheers ... Duncan.
