tc/m_xt: Fix for potential string buffer overflows

- Use strncpy() when writing to target->t->u.user.name and make sure the
  final byte remains untouched (xtables_calloc() set it to zero).
- 'tname' length sanitization was completely wrong: If it's length
  exceeded the 16 bytes available in 'k', passing a length value of 16
  to strncpy() would overwrite the previously NULL'ed 'k[15]'. Also, the
  sanitization has to happen if 'tname' is exactly 16 bytes long as
  well.

Signed-off-by: Phil Sutter <phil@nwl.cc>

diff --git a/tc/m_xt.c b/tc/m_xt.c
index ad52d239caf6118a862d11e5a9ecf7a0c3daf646..9218b145944032bcc56fb63dcf6e1722ae5ed0da 100644 (file)
--- a/tc/m_xt.c
+++ b/tc/m_xt.c
@@ -95,7 +95,8 @@ build_st(struct xtables_target *target, struct xt_entry_target *t)
        if (t == NULL) {
                target->t = xtables_calloc(1, size);
                target->t->u.target_size = size;
-               strcpy(target->t->u.user.name, target->name);
+               strncpy(target->t->u.user.name, target->name,
+                       sizeof(target->t->u.user.name) - 1);
                target->t->u.user.revision = target->revision;
 
                if (target->init != NULL)
@@ -277,8 +278,8 @@ static int parse_ipt(struct action_util *a, int *argc_p,
        }
        fprintf(stdout, " index %d\n", index);
 
-       if (strlen(tname) > 16) {
-               size = 16;
+       if (strlen(tname) >= 16) {
+               size = 15;
                k[15] = 0;
        } else {
                size = 1 + strlen(tname);