Skip to contentSkip to conversation

netfilter: Zero the tuple in nfnl_cthelper_parse_tuple()

PATCH In review Ian Wilson 2 participants 2 messages Mar 11, 2015

Subsystems (3)
Networking Network drivers netfilter
lore
Share & tools
lore

Ian Wilson

author
#0 lore raw

PATCH 1/1 netfilter: Zero the tuple in nfnl_cthelper_parse_tuple()

From 0d63c5b06276d7946ba2f1f4aace4f0486ec74cf Mon Sep 17 00:00:00 2001
From: Ian Wilson <[email protected]>
Date: Wed, 11 Mar 2015 20:34:21 +0000
Subject: [PATCH] netfilter: Zero the tuple in nfnl_cthelper_parse_tuple()

nfnl_cthelper_parse_tuple() is called from nfnl_cthelper_new(), nfnl_cthelper_get() and nfnl_cthelper_del(). In each case they pass a pointer to an nf_conntrack_tuple data structure local variable:

    struct nf_conntrack_tuple tuple;
    ...
    ret = nfnl_cthelper_parse_tuple(&tuple, tb[NFCTH_TUPLE]);

The problem is that this local variable is not initialized, and nfnl_cthelper_parse_tuple() only initializes two fields: src.l3num and dst.protonum. This leaves all other fields with undefined values based on whatever is on the stack:

    tuple->src.l3num = ntohs(nla_get_be16(tb[NFCTH_TUPLE_L3PROTONUM]));
    tuple->dst.protonum = nla_get_u8(tb[NFCTH_TUPLE_L4PROTONUM]);

The symptom observed was that when the rpc and tns helpers were added then traffic to port 1536 was being sent to user-space.

---
 net/netfilter/nfnetlink_cthelper.c | 3 +++
 1 file changed, 3 insertions(+)
net/netfilter/nfnetlink_cthelper.c +3 -0
index a5599fc..01f4c45 100644
@@ -77,6 +77,9 @@ nfnl_cthelper_parse_tuple(struct nf_conntrack_tuple *tuple,
     if (!tb[NFCTH_TUPLE_L3PROTONUM] || !tb[NFCTH_TUPLE_L4PROTONUM])
         return -EINVAL;
 
+     /* Not all fields are initialized so first zero the tuple */
+    memset(tuple, 0, sizeof(struct nf_conntrack_tuple));
+
     tuple->src.l3num = ntohs(nla_get_be16(tb[NFCTH_TUPLE_L3PROTONUM]));
     tuple->dst.protonum = nla_get_u8(tb[NFCTH_TUPLE_L4PROTONUM]);
Signature · 1 line
1.9.1
#1 lore raw
Quoted 35 lines · On Wed, 11 Mar 2015 21:01:55 +0000 Ian Wilson <[email protected]> wrote:

From 0d63c5b06276d7946ba2f1f4aace4f0486ec74cf Mon Sep 17 00:00:00 2001
From: Ian Wilson <[email protected]>
Date: Wed, 11 Mar 2015 20:34:21 +0000
Subject: [PATCH] netfilter: Zero the tuple in nfnl_cthelper_parse_tuple()

nfnl_cthelper_parse_tuple() is called from nfnl_cthelper_new(), nfnl_cthelper_get() and nfnl_cthelper_del(). In each case they pass a pointer to an nf_conntrack_tuple data structure local variable:

     struct nf_conntrack_tuple tuple;
     ...
     ret = nfnl_cthelper_parse_tuple(&tuple, tb[NFCTH_TUPLE]);

The problem is that this local variable is not initialized, and nfnl_cthelper_parse_tuple() only initializes two fields: src.l3num and dst.protonum. This leaves all other fields with undefined values based on whatever is on the stack:

     tuple->src.l3num = ntohs(nla_get_be16(tb[NFCTH_TUPLE_L3PROTONUM]));
     tuple->dst.protonum = nla_get_u8(tb[NFCTH_TUPLE_L4PROTONUM]);

The symptom observed was that when the rpc and tns helpers were added then traffic to port 1536 was being sent to user-space.

---
  net/netfilter/nfnetlink_cthelper.c | 3 +++
  1 file changed, 3 insertions(+)
net/netfilter/nfnetlink_cthelper.c +0 -0
index a5599fc..01f4c45 100644

net/netfilter/nfnetlink_cthelper.c nfnl_cthelper_parse_tuple(struct nf_conntrack_tuple · line 85

Quoted 9 lines
net/netfilter/nfnetlink_cthelper.c +3 -0
@@ -77,6 +77,9 @@ nfnl_cthelper_parse_tuple(struct nf_conntrack_tuple 
 *tuple,
      if (!tb[NFCTH_TUPLE_L3PROTONUM] || !tb[NFCTH_TUPLE_L4PROTONUM])
          return -EINVAL;
 
+     /* Not all fields are initialized so first zero the tuple */
+    memset(tuple, 0, sizeof(struct nf_conntrack_tuple));
+
      tuple->src.l3num = ntohs(nla_get_be16(tb[NFCTH_TUPLE_L3PROTONUM]));
      tuple->dst.protonum = nla_get_u8(tb[NFCTH_TUPLE_L4PROTONUM]);

The code looks correct but you still have some more hurdles.
1. Don't use Outlook to send patches, it corrupts line wrapping.
2. Missing signed off by
3. Indentation is wrong.

$ ~/kernel/linux/scripts/checkpatch.pl /tmp/nfnt.patch 
ERROR: patch seems to be corrupt (line wrapped?)
#60: FILE: net/netfilter/nfnetlink_cthelper.c:76:
*tuple,

WARNING: please, no spaces at the start of a line
#65: FILE: net/netfilter/nfnetlink_cthelper.c:81:
+ memset(tuple, 0, sizeof(struct nf_conntrack_tuple));$

ERROR: Missing Signed-off-by: line(s)

total: 2 errors, 1 warnings, 0 checks, 10 lines checked

/tmp/nfnt.patch has style problems, please review.

If any of these errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.