From 5e9e2dc7e972f6bbbc0156ad97b4ee9d11fcb837 Mon Sep 17 00:00:00 2001

From: Phil Sutter <psutter@redhat.com>

Date: Wed, 20 Jun 2018 09:22:47 +0200

Subject: [PATCH] netlink_delinearize: Fix resource leaks

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1504157

Upstream Status: nftables commit 671851617c8d8

commit 671851617c8d8c1dfe9822eee8dcc7b827fff850

Author: Phil Sutter <phil@nwl.cc>

Date:   Thu Mar 1 15:00:32 2018 +0100

    netlink_delinearize: Fix resource leaks

    Most of the cases are basically the same: Error path fails to free the

    previously allocated statement or expression. A few cases received

    special treatment though:

    - In netlink_parse_payload_stmt(), the leak is easily avoided by code

      reordering.

    - In netlink_parse_exthdr(), there's no point in introducing a goto

      label since there is but a single affected error check.

    - In netlink_parse_hash() non-error path leaked as well if sreg

      contained a concatenated expression.

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

    Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

---

 src/netlink_delinearize.c | 144 +++++++++++++++++++++++++++++-----------------

 1 file changed, 92 insertions(+), 52 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c

index 61cba52..e25160a 100644

--- a/src/netlink_delinearize.c

+++ b/src/netlink_delinearize.c

@@ -470,15 +470,15 @@ static void netlink_parse_payload_stmt(struct netlink_parse_ctx *ctx,

 	offset = nftnl_expr_get_u32(nle, NFTNL_EXPR_PAYLOAD_OFFSET) * BITS_PER_BYTE;

 	len    = nftnl_expr_get_u32(nle, NFTNL_EXPR_PAYLOAD_LEN) * BITS_PER_BYTE;

-	expr = payload_expr_alloc(loc, NULL, 0);

-	payload_init_raw(expr, base, offset, len);

-

 	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");

+	expr = payload_expr_alloc(loc, NULL, 0);

+	payload_init_raw(expr, base, offset, len);

+

 	stmt = payload_stmt_alloc(loc, expr, val);

 	list_add_tail(&stmt->list, &ctx->rule->stmts);

@@ -523,9 +523,11 @@ static void netlink_parse_exthdr(struct netlink_parse_ctx *ctx,

 		sreg = netlink_parse_register(nle, NFTNL_EXPR_EXTHDR_SREG);

 		val = netlink_get_register(ctx, loc, sreg);

-		if (val == NULL)

+		if (val == NULL) {

+			xfree(expr);

 			return netlink_error(ctx, loc,

 					     "exthdr statement has no expression");

+		}

 		expr_set_type(val, expr->dtype, expr->byteorder);

@@ -556,22 +558,27 @@ 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

+		if (hexpr == NULL) {

 			netlink_error(ctx, loc,

 				      "hash statement has no expression");

+			goto out_err;

+		}

 		len = nftnl_expr_get_u32(nle,

 					 NFTNL_EXPR_HASH_LEN) * BITS_PER_BYTE;

 		if (hexpr->len < len) {

+			xfree(hexpr);

 			hexpr = netlink_parse_concat_expr(ctx, loc, sreg, len);

 			if (hexpr == NULL)

-				return;

+				goto out_err;

 		}

 		expr->hash.expr = hexpr;

 	}

 	dreg = netlink_parse_register(nle, NFTNL_EXPR_HASH_DREG);

 	netlink_set_register(ctx, dreg, expr);

+	return;

+out_err:

+	xfree(expr);

 }

 static void netlink_parse_fib(struct netlink_parse_ctx *ctx,

@@ -853,10 +860,11 @@ static void netlink_parse_nat(struct netlink_parse_ctx *ctx,

 	reg1 = netlink_parse_register(nle, NFTNL_EXPR_NAT_REG_ADDR_MIN);

 	if (reg1) {

 		addr = netlink_get_register(ctx, loc, reg1);

-		if (addr == NULL)

-			return netlink_error(ctx, loc,

-					     "NAT statement has no address "

-					     "expression");

+		if (addr == NULL) {

+			netlink_error(ctx, loc,

+				      "NAT statement has no address expression");

+			goto out_err;

+		}

 		if (family == AF_INET)

 			expr_set_type(addr, &ipaddr_type, BYTEORDER_BIG_ENDIAN);

@@ -869,10 +877,11 @@ static void netlink_parse_nat(struct netlink_parse_ctx *ctx,

 	reg2 = netlink_parse_register(nle, NFTNL_EXPR_NAT_REG_ADDR_MAX);

 	if (reg2 && reg2 != reg1) {

 		addr = netlink_get_register(ctx, loc, reg2);

-		if (addr == NULL)

-			return netlink_error(ctx, loc,

-					     "NAT statement has no address "

-					     "expression");

+		if (addr == NULL) {

+			netlink_error(ctx, loc,

+				      "NAT statement has no address expression");

+			goto out_err;

+		}

 		if (family == AF_INET)

 			expr_set_type(addr, &ipaddr_type, BYTEORDER_BIG_ENDIAN);

@@ -887,10 +896,11 @@ static void netlink_parse_nat(struct netlink_parse_ctx *ctx,

 	reg1 = netlink_parse_register(nle, NFTNL_EXPR_NAT_REG_PROTO_MIN);

 	if (reg1) {

 		proto = netlink_get_register(ctx, loc, reg1);

-		if (proto == NULL)

-			return netlink_error(ctx, loc,

-					     "NAT statement has no proto "

-					     "expression");

+		if (proto == NULL) {

+			netlink_error(ctx, loc,

+				      "NAT statement has no proto expression");

+			goto out_err;

+		}

 		expr_set_type(proto, &inet_service_type, BYTEORDER_BIG_ENDIAN);

 		stmt->nat.proto = proto;

@@ -899,10 +909,11 @@ static void netlink_parse_nat(struct netlink_parse_ctx *ctx,

 	reg2 = netlink_parse_register(nle, NFTNL_EXPR_NAT_REG_PROTO_MAX);

 	if (reg2 && reg2 != reg1) {

 		proto = netlink_get_register(ctx, loc, reg2);

-		if (proto == NULL)

-			return netlink_error(ctx, loc,

-					     "NAT statement has no proto "

-					     "expression");

+		if (proto == NULL) {

+			netlink_error(ctx, loc,

+				      "NAT statement has no proto expression");

+			goto out_err;

+		}

 		expr_set_type(proto, &inet_service_type, BYTEORDER_BIG_ENDIAN);

 		if (stmt->nat.proto != NULL)

@@ -911,6 +922,9 @@ static void netlink_parse_nat(struct netlink_parse_ctx *ctx,

 	}

 	ctx->stmt = stmt;

+	return;

+out_err:

+	xfree(stmt);

 }

 static void netlink_parse_masq(struct netlink_parse_ctx *ctx,

@@ -931,10 +945,11 @@ static void netlink_parse_masq(struct netlink_parse_ctx *ctx,

 	reg1 = netlink_parse_register(nle, NFTNL_EXPR_MASQ_REG_PROTO_MIN);

 	if (reg1) {

 		proto = netlink_get_register(ctx, loc, reg1);

-		if (proto == NULL)

-			return netlink_error(ctx, loc,

-					     "MASQUERADE statement"

-					     "has no proto expression");

+		if (proto == NULL) {

+			netlink_error(ctx, loc,

+				      "MASQUERADE statement has no proto expression");

+			goto out_err;

+		}

 		expr_set_type(proto, &inet_service_type, BYTEORDER_BIG_ENDIAN);

 		stmt->masq.proto = proto;

 	}

@@ -942,10 +957,11 @@ static void netlink_parse_masq(struct netlink_parse_ctx *ctx,

 	reg2 = netlink_parse_register(nle, NFTNL_EXPR_MASQ_REG_PROTO_MAX);

 	if (reg2 && reg2 != reg1) {

 		proto = netlink_get_register(ctx, loc, reg2);

-		if (proto == NULL)

-			return netlink_error(ctx, loc,

-					     "MASQUERADE statement"

-					     "has no proto expression");

+		if (proto == NULL) {

+			netlink_error(ctx, loc,

+				      "MASQUERADE statement has no proto expression");

+			goto out_err;

+		}

 		expr_set_type(proto, &inet_service_type, BYTEORDER_BIG_ENDIAN);

 		if (stmt->masq.proto != NULL)

 			proto = range_expr_alloc(loc, stmt->masq.proto, proto);

@@ -953,6 +969,9 @@ static void netlink_parse_masq(struct netlink_parse_ctx *ctx,

 	}

 	ctx->stmt = stmt;

+	return;

+out_err:

+	xfree(stmt);

 }

 static void netlink_parse_redir(struct netlink_parse_ctx *ctx,

@@ -974,10 +993,11 @@ static void netlink_parse_redir(struct netlink_parse_ctx *ctx,

 	reg1 = netlink_parse_register(nle, NFTNL_EXPR_REDIR_REG_PROTO_MIN);

 	if (reg1) {

 		proto = netlink_get_register(ctx, loc, reg1);

-		if (proto == NULL)

-			return netlink_error(ctx, loc,

-					     "redirect statement has no proto "

-					     "expression");

+		if (proto == NULL) {

+			netlink_error(ctx, loc,

+				      "redirect statement has no proto expression");

+			goto out_err;

+		}

 		expr_set_type(proto, &inet_service_type, BYTEORDER_BIG_ENDIAN);

 		stmt->redir.proto = proto;

@@ -986,10 +1006,11 @@ static void netlink_parse_redir(struct netlink_parse_ctx *ctx,

 	reg2 = netlink_parse_register(nle, NFTNL_EXPR_REDIR_REG_PROTO_MAX);

 	if (reg2 && reg2 != reg1) {

 		proto = netlink_get_register(ctx, loc, reg2);

-		if (proto == NULL)

-			return netlink_error(ctx, loc,

-					     "redirect statement has no proto "

-					     "expression");

+		if (proto == NULL) {

+			netlink_error(ctx, loc,

+				      "redirect statement has no proto expression");

+			goto out_err;

+		}

 		expr_set_type(proto, &inet_service_type, BYTEORDER_BIG_ENDIAN);

 		if (stmt->redir.proto != NULL)

@@ -999,6 +1020,9 @@ static void netlink_parse_redir(struct netlink_parse_ctx *ctx,

 	}

 	ctx->stmt = stmt;

+	return;

+out_err:

+	xfree(stmt);

 }

 static void netlink_parse_dup(struct netlink_parse_ctx *ctx,

@@ -1014,9 +1038,11 @@ static void netlink_parse_dup(struct netlink_parse_ctx *ctx,

 	reg1 = netlink_parse_register(nle, NFTNL_EXPR_DUP_SREG_ADDR);

 	if (reg1) {

 		addr = netlink_get_register(ctx, loc, reg1);

-		if (addr == NULL)

-			return netlink_error(ctx, loc,

-					     "DUP statement has no destination expression");

+		if (addr == NULL) {

+			netlink_error(ctx, loc,

+				      "DUP statement has no destination expression");

+			goto out_err;

+		}

 		switch (ctx->table->handle.family) {

 		case NFPROTO_IPV4:

@@ -1033,9 +1059,11 @@ static void netlink_parse_dup(struct netlink_parse_ctx *ctx,

 	reg2 = netlink_parse_register(nle, NFTNL_EXPR_DUP_SREG_DEV);

 	if (reg2) {

 		dev = netlink_get_register(ctx, loc, reg2);

-		if (dev == NULL)

-			return netlink_error(ctx, loc,

-					     "DUP statement has no output expression");

+		if (dev == NULL) {

+			netlink_error(ctx, loc,

+				      "DUP statement has no output expression");

+			goto out_err;

+		}

 		expr_set_type(dev, &ifindex_type, BYTEORDER_HOST_ENDIAN);

 		if (stmt->dup.to == NULL)

@@ -1045,6 +1073,9 @@ static void netlink_parse_dup(struct netlink_parse_ctx *ctx,

 	}

 	ctx->stmt = stmt;

+	return;

+out_err:

+	xfree(stmt);

 }

 static void netlink_parse_fwd(struct netlink_parse_ctx *ctx,

@@ -1060,15 +1091,20 @@ static void netlink_parse_fwd(struct netlink_parse_ctx *ctx,

 	reg1 = netlink_parse_register(nle, NFTNL_EXPR_FWD_SREG_DEV);

 	if (reg1) {

 		dev = netlink_get_register(ctx, loc, reg1);

-		if (dev == NULL)

-			return netlink_error(ctx, loc,

-					     "fwd statement has no output expression");

+		if (dev == NULL) {

+			netlink_error(ctx, loc,

+				      "fwd statement has no output expression");

+			goto out_err;

+		}

 		expr_set_type(dev, &ifindex_type, BYTEORDER_HOST_ENDIAN);

 		stmt->fwd.to = dev;

 	}

 	ctx->stmt = stmt;

+	return;

+out_err:

+	xfree(stmt);

 }

 static void netlink_parse_queue(struct netlink_parse_ctx *ctx,

@@ -1135,10 +1171,11 @@ static void netlink_parse_dynset(struct netlink_parse_ctx *ctx,

 	dnle = nftnl_expr_get(nle, NFTNL_EXPR_DYNSET_EXPR, NULL);

 	if (dnle != NULL) {

 		if (netlink_parse_expr(dnle, ctx) < 0)

-			return;

-		if (ctx->stmt == NULL)

-			return netlink_error(ctx, loc,

-					     "Could not parse dynset stmt");

+			goto out_err;

+		if (ctx->stmt == NULL) {

+			netlink_error(ctx, loc, "Could not parse dynset stmt");

+			goto out_err;

+		}

 		dstmt = ctx->stmt;

 	}

@@ -1155,6 +1192,9 @@ static void netlink_parse_dynset(struct netlink_parse_ctx *ctx,

 	}

 	ctx->stmt = stmt;

+	return;

+out_err:

+	xfree(expr);

 }

 static void netlink_parse_objref(struct netlink_parse_ctx *ctx,

-- 

1.8.3.1