On Tue, Aug 30, 2016 at 07:39:50PM +0200, Phil Sutter wrote:
> As netlink_get_register() may return NULL, we must not pass the returned
> data unchecked to expr_set_type() as that will dereference it. Since the
> parser has failed at that point anyway, by returning early we can skip
> the useless statement allocation that follows in
> netlink_parse_ct_stmt().
I found a couple more spots, such as the payload stmt that was not
covered by this patch.
Attaching a new one based on this, looks good to you?
Anyway, this is very unlikely to happen: Only if we ever get more
registers in the kernel, given that expl_clone() relies on the
xzalloc() function that just stops execution under OOM.
Actually this brings an interesting issue that is that we need to
provide a way to describe the vm capabilities so we can extend things
in the future without breaking userspace.
commit 098dae9e0cf2237b4cb3cf4c1ee89fbf9f9fb5e9
Author: Phil Sutter <phil@xxxxxx>
Date: Tue Aug 30 19:39:50 2016 +0200
netlink_delinearize: Avoid potential null pointer deref
As netlink_get_register() may return NULL, we must not pass the returned
data unchecked to expr_set_type() as that will dereference it. Since the
parser has failed at that point anyway, by returning early we can skip
the useless statement allocation that follows in
netlink_parse_ct_stmt().
Signed-off-by: Phil Sutter <phil@xxxxxx>
Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 1a1cfbd..cddbfa6 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -428,6 +428,10 @@ static void netlink_parse_payload_stmt(struct netlink_parse_ctx *ctx,
sreg = netlink_parse_register(nle, NFTNL_EXPR_PAYLOAD_SREG);
val = netlink_get_register(ctx, loc, sreg);
+ if (val == NULL)
+ return netlink_error(ctx, loc,
+ "payload statement has no expression");
+
stmt = payload_stmt_alloc(loc, expr, val);
list_add_tail(&stmt->list, &ctx->rule->stmts);
@@ -473,6 +477,9 @@ static void netlink_parse_hash(struct netlink_parse_ctx *ctx,
sreg = netlink_parse_register(nle, NFTNL_EXPR_HASH_SREG);
hexpr = netlink_get_register(ctx, loc, sreg);
+ if (hexpr == NULL)
+ return netlink_error(ctx, loc,
+ "hash statement has no expression");
seed = nftnl_expr_get_u32(nle, NFTNL_EXPR_HASH_SEED);
mod = nftnl_expr_get_u32(nle, NFTNL_EXPR_HASH_MODULUS);
@@ -517,6 +524,9 @@ static void netlink_parse_meta_stmt(struct netlink_parse_ctx *ctx,
sreg = netlink_parse_register(nle, NFTNL_EXPR_META_SREG);
expr = netlink_get_register(ctx, loc, sreg);
+ if (expr == NULL)
+ return netlink_error(ctx, loc,
+ "meta statement has no expression");
key = nftnl_expr_get_u32(nle, NFTNL_EXPR_META_KEY);
stmt = meta_stmt_alloc(loc, key, expr);
@@ -562,6 +572,9 @@ static void netlink_parse_ct_stmt(struct netlink_parse_ctx *ctx,
sreg = netlink_parse_register(nle, NFTNL_EXPR_CT_SREG);
expr = netlink_get_register(ctx, loc, sreg);
+ if (expr == NULL)
+ return netlink_error(ctx, loc,
+ "ct statement has no expression");
key = nftnl_expr_get_u32(nle, NFTNL_EXPR_CT_KEY);
stmt = ct_stmt_alloc(loc, key, expr);
