obj/ct_timeout: Fix NFTA_CT_TIMEOUT_DATA parser

This is a necessary follow-up on commit 00b144bc9d093 ("obj/ct_timeout:
Avoid array overrun in timeout_parse_attr_data()") which fixed array out
of bounds access but missed the logic behind it:

The nested attribute type values are incremented by one when being
transferred between kernel and userspace, the zero type value is
reserved for "unspecified".

Kernel uses CTA_TIMEOUT_* symbols for that, libnftnl simply mangles the
type values in nftnl_obj_ct_timeout_build().

Return path was broken as it overstepped its nlattr array but apart from
that worked: Type values were decremented by one in
timeout_parse_attr_data().

This patch moves the type value mangling into
parse_timeout_attr_policy_cb() (which still overstepped nlattr array).
Consequently, when copying values from nlattr array into ct timeout
object in timeout_parse_attr_data(), loop is adjusted to start at index
0 and the type value decrement is dropped there.

Fixes: 0adceeab1597a ("src: add ct timeout support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/src/obj/ct_timeout.c b/src/obj/ct_timeout.c
index a09e25ae5d44f8f1d1cb07abc8d27e80ea510f32..2662cac69438d375df05ea6487595f30ca983f55 100644 (file)
--- a/src/obj/ct_timeout.c
+++ b/src/obj/ct_timeout.c
@@ -108,10 +108,10 @@ parse_timeout_attr_policy_cb(const struct nlattr *attr, void *data)
        if (mnl_attr_type_valid(attr, data_cb->nlattr_max) < 0)
                return MNL_CB_OK;
 
-       if (type <= data_cb->nlattr_max) {
+       if (type > 0 && type <= data_cb->nlattr_max) {
                if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
                        abi_breakage();
-               tb[type] = attr;
+               tb[type - 1] = attr;
        }
        return MNL_CB_OK;
 }
@@ -134,9 +134,9 @@ timeout_parse_attr_data(struct nftnl_obj *e,
        if (mnl_attr_parse_nested(nest, parse_timeout_attr_policy_cb, &cnt) < 0)
                return -1;
 
-       for (i = 1; i < array_size(tb); i++) {
+       for (i = 0; i < array_size(tb); i++) {
                if (tb[i]) {
-                       nftnl_timeout_policy_attr_set_u32(e, i-1,
+                       nftnl_timeout_policy_attr_set_u32(e, i,
                                ntohl(mnl_attr_get_u32(tb[i])));
                }
        }