226 lines
7.0 KiB
Diff
226 lines
7.0 KiB
Diff
|
From a045ef8abc1c81ac359103ac61841bae860d8960 Mon Sep 17 00:00:00 2001
|
|||
|
From: "Jose M. Guisado Gomez" <guigom@riseup.net>
|
|||
|
Date: Fri, 16 Aug 2019 11:25:11 +0200
|
|||
|
Subject: [PATCH] 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 <guigom@riseup.net>
|
|||
|
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|||
|
(cherry picked from commit f196de88cdd9764ddc2e4de737a960972d82fe9d)
|
|||
|
---
|
|||
|
include/conntrackd.h | 6 +++---
|
|||
|
include/helper.h | 2 +-
|
|||
|
include/local.h | 4 ++--
|
|||
|
src/main.c | 7 +++----
|
|||
|
src/read_config_yy.y | 39 +++++++++++++++++++++++++++++----------
|
|||
|
5 files changed, 38 insertions(+), 20 deletions(-)
|
|||
|
|
|||
|
diff --git a/include/conntrackd.h b/include/conntrackd.h
|
|||
|
index 81dff221e96de..fe9ec1854a7d2 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 7353dfa9b2073..08d4cf4642802 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 22859d7ab60aa..9379446732eed 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 */
|
|||
|
diff --git a/src/main.c b/src/main.c
|
|||
|
index 8c3fa1c943a96..de4773df8a204 100644
|
|||
|
--- a/src/main.c
|
|||
|
+++ b/src/main.c
|
|||
|
@@ -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 6aee67623953b..d963c494be1fc 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);
|
|||
|
--
|
|||
|
2.34.1
|
|||
|
|