From b51e999dcf0f87e125a81d8623b1a34c6407e624 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Wed, 16 Oct 2019 23:22:53 +0200 Subject: 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 Acked-by: Pablo Neira Ayuso --- src/obj/ct_timeout.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/obj/ct_timeout.c') diff --git a/src/obj/ct_timeout.c b/src/obj/ct_timeout.c index a09e25a..2662cac 100644 --- 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]))); } } -- cgit v1.2.3