@@ -427,8 +427,8 @@ static void copy_attr_repl_off_aft(struct nf_conntrack *dest,
static void copy_attr_helper_name(struct nf_conntrack *dest,
const struct nf_conntrack *orig)
{
- strncpy(dest->helper_name, orig->helper_name, NFCT_HELPER_NAME_MAX);
- dest->helper_name[NFCT_HELPER_NAME_MAX-1] = '\0';
+ snprintf(dest->helper_name, NFCT_HELPER_NAME_MAX, "%s",
+ orig->helper_name);
}
static void copy_attr_zone(struct nf_conntrack *dest,
@@ -690,9 +690,8 @@ nfct_parse_helper(const struct nlattr *attr, struct nf_conntrack *ct)
if (!tb[CTA_HELP_NAME])
return 0;
- strncpy(ct->helper_name, mnl_attr_get_str(tb[CTA_HELP_NAME]),
- NFCT_HELPER_NAME_MAX);
- ct->helper_name[NFCT_HELPER_NAME_MAX-1] = '\0';
+ snprintf(ct->helper_name, NFCT_HELPER_NAME_MAX, "%s",
+ mnl_attr_get_str(tb[CTA_HELP_NAME]));
set_bit(ATTR_HELPER_NAME, ct->head.set);
if (!tb[CTA_HELP_INFO])
@@ -389,8 +389,7 @@ set_attr_repl_off_aft(struct nf_conntrack *ct, const void *value, size_t len)
static void
set_attr_helper_name(struct nf_conntrack *ct, const void *value, size_t len)
{
- strncpy(ct->helper_name, value, NFCT_HELPER_NAME_MAX);
- ct->helper_name[NFCT_HELPER_NAME_MAX-1] = '\0';
+ snprintf(ct->helper_name, NFCT_HELPER_NAME_MAX, "%s", (char*)value);
}
static void
@@ -10,6 +10,7 @@
*/
#include "internal/internal.h"
+#include <assert.h>
#include <libmnl/libmnl.h>
static int nlmsg_parse_expection_attr_cb(const struct nlattr *attr, void *data)
@@ -139,11 +140,9 @@ int nfexp_nlmsg_parse(const struct nlmsghdr *nlh, struct nf_expect *exp)
set_bit(ATTR_EXP_FLAGS, exp->set);
}
if (tb[CTA_EXPECT_HELP_NAME]) {
- strncpy(exp->helper_name,
- mnl_attr_get_str(tb[CTA_EXPECT_HELP_NAME]),
- NFCT_HELPER_NAME_MAX);
- exp->helper_name[NFCT_HELPER_NAME_MAX - 1] = '\0';
- set_bit(ATTR_EXP_HELPER_NAME, exp->set);
+ snprintf(exp->helper_name, NFCT_HELPER_NAME_MAX, "%s",
+ mnl_attr_get_str(tb[CTA_EXPECT_HELP_NAME]));
+ set_bit(ATTR_EXP_HELPER_NAME, exp->set);
}
if (tb[CTA_EXPECT_CLASS]) {
exp->class = ntohl(mnl_attr_get_u32(tb[CTA_EXPECT_CLASS]));
@@ -153,9 +152,10 @@ int nfexp_nlmsg_parse(const struct nlmsghdr *nlh, struct nf_expect *exp)
nfexp_nlmsg_parse_nat(nfg, tb[CTA_EXPECT_NAT], exp);
if (tb[CTA_EXPECT_FN]) {
- strncpy(exp->expectfn, mnl_attr_get_payload(tb[CTA_EXPECT_FN]),
- __NFCT_EXPECTFN_MAX);
- exp->expectfn[__NFCT_EXPECTFN_MAX - 1] = '\0';
+ int len = mnl_attr_get_payload_len(tb[CTA_EXPECT_FN]);
+ assert(len <= __NFCT_EXPECTFN_MAX); /* the kernel doesn't impose a max lenght on this str */
+ snprintf(exp->expectfn, __NFCT_EXPECTFN_MAX, "%s",
+ (char*)mnl_attr_get_payload(tb[CTA_EXPECT_FN]));
set_bit(ATTR_EXP_FN, exp->set);
}
@@ -46,8 +46,7 @@ static void set_exp_attr_class(struct nf_expect *exp, const void *value)
static void set_exp_attr_helper_name(struct nf_expect *exp, const void *value)
{
- strncpy(exp->helper_name, value, NFCT_HELPER_NAME_MAX);
- exp->helper_name[NFCT_HELPER_NAME_MAX-1] = '\0';
+ snprintf(exp->helper_name, NFCT_HELPER_NAME_MAX, "%s", (char*)value);
}
static void set_exp_attr_nat_dir(struct nf_expect *exp, const void *value)
@@ -62,8 +61,7 @@ static void set_exp_attr_nat_tuple(struct nf_expect *exp, const void *value)
static void set_exp_attr_expectfn(struct nf_expect *exp, const void *value)
{
- strncpy(exp->expectfn, value, __NFCT_EXPECTFN_MAX);
- exp->expectfn[__NFCT_EXPECTFN_MAX-1] = '\0';
+ snprintf(exp->expectfn, __NFCT_EXPECTFN_MAX, "%s", (char*)value);
}
const set_exp_attr set_exp_attr_array[ATTR_EXP_MAX] = {
We currently use strncpy in a bunch of places which has this weird quirk where it doesn't write a terminating null byte if the input string is >= the max length. To mitigate this we write a null byte to the last character manually. While this works it is easy to forget. Instead we should just be using snprintf which has more sensible behaviour as it always writes a null byte even when truncating the string. Signed-off-by: Daniel Gröber <dxld@darkboxed.org> --- src/conntrack/copy.c | 4 ++-- src/conntrack/parse_mnl.c | 5 ++--- src/conntrack/setter.c | 3 +-- src/expect/parse_mnl.c | 16 ++++++++-------- src/expect/setter.c | 6 ++---- 5 files changed, 15 insertions(+), 19 deletions(-)