From 71047a9d6d1eea71b3fbd430983541a54049cc69 Mon Sep 17 00:00:00 2001 From: David Steele Date: Sat, 2 Oct 2021 16:17:33 -0400 Subject: [PATCH] Use strncpy() to limit characters copied to optionName. Valgrind complained about uninitialized values on arm64 when comparing the reset prefix, probably because "reset" ended up being larger than the option name: Conditional jump or move depends on uninitialised value(s) at cfgParseOption (parse.c:568). Coverity complained because it could not verify the size of the string to be copied into optionName, probably because it does not understand the purpose of strSize(): You might overrun the 65-character fixed-size string "optionName" by copying the return value of "strZ" without checking the length. Use strncpy() even though we have already checked the size and make sure the string is terminated. Keep the size check because searching for truncated option names is not a good idea. This is not a production bug since the code has not been released yet. --- doc/xml/release.xml | 5 ++++- src/config/parse.c | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 860fc5c9f..594be64b4 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -18,7 +18,10 @@ - + + + + diff --git a/src/config/parse.c b/src/config/parse.c index f42ae5a18..45e2ea253 100644 --- a/src/config/parse.c +++ b/src/config/parse.c @@ -553,7 +553,8 @@ cfgParseOption(const String *const optionCandidate, const CfgParseOptionParam pa OptionInvalidError, "option '%s' exceeds maximum size of " STRINGIFY(OPTION_NAME_SIZE_MAX), strZ(optionCandidate)); } - strcpy(optionName, strZ(optionCandidate)); + strncpy(optionName, strZ(optionCandidate), sizeof(optionName) - 1); + optionName[OPTION_NAME_SIZE_MAX] = '\0'; // If this looks like negate if (strncmp(optionName, OPTION_PREFIX_NEGATE, sizeof(OPTION_PREFIX_NEGATE) - 1) == 0)