From patchwork Mon Jan 20 16:25:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 1226057 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=nwl.cc Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 481cVJ6bNhz9sRd for ; Tue, 21 Jan 2020 03:26:00 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729165AbgATQ0A (ORCPT ); Mon, 20 Jan 2020 11:26:00 -0500 Received: from orbyte.nwl.cc ([151.80.46.58]:60808 "EHLO orbyte.nwl.cc" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726897AbgATQ0A (ORCPT ); Mon, 20 Jan 2020 11:26:00 -0500 Received: from localhost ([::1]:45666 helo=tatos) by orbyte.nwl.cc with esmtp (Exim 4.91) (envelope-from ) id 1itZsF-00063P-G9; Mon, 20 Jan 2020 17:25:59 +0100 From: Phil Sutter To: Pablo Neira Ayuso Cc: netfilter-devel@vger.kernel.org Subject: [nft PATCH 1/4] netlink: Fix leak in unterminated string deserializer Date: Mon, 20 Jan 2020 17:25:37 +0100 Message-Id: <20200120162540.9699-2-phil@nwl.cc> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200120162540.9699-1-phil@nwl.cc> References: <20200120162540.9699-1-phil@nwl.cc> MIME-Version: 1.0 Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Allocated 'mask' expression is not freed before returning to caller, although it is used temporarily only. Fixes: b851ba4731d9f ("src: add interface wildcard matching") Signed-off-by: Phil Sutter Acked-by: Pablo Neira Ayuso --- src/netlink_delinearize.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index 154353b8161a0..06a0312b9921a 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -2030,7 +2030,7 @@ static bool __expr_postprocess_string(struct expr **exprp) static struct expr *expr_postprocess_string(struct expr *expr) { - struct expr *mask; + struct expr *mask, *out; assert(expr_basetype(expr)->type == TYPE_STRING); if (__expr_postprocess_string(&expr)) @@ -2040,7 +2040,9 @@ static struct expr *expr_postprocess_string(struct expr *expr) BYTEORDER_HOST_ENDIAN, expr->len + BITS_PER_BYTE, NULL); mpz_init_bitmask(mask->value, expr->len); - return string_wildcard_expr_alloc(&expr->location, mask, expr); + out = string_wildcard_expr_alloc(&expr->location, mask, expr); + expr_free(mask); + return out; } static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp) From patchwork Mon Jan 20 16:25:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 1226055 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=nwl.cc Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 481cV64bx1z9sR1 for ; Tue, 21 Jan 2020 03:25:50 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729146AbgATQZu (ORCPT ); Mon, 20 Jan 2020 11:25:50 -0500 Received: from orbyte.nwl.cc ([151.80.46.58]:60796 "EHLO orbyte.nwl.cc" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726988AbgATQZt (ORCPT ); Mon, 20 Jan 2020 11:25:49 -0500 Received: from localhost ([::1]:45654 helo=tatos) by orbyte.nwl.cc with esmtp (Exim 4.91) (envelope-from ) id 1itZs4-00062L-6X; Mon, 20 Jan 2020 17:25:48 +0100 From: Phil Sutter To: Pablo Neira Ayuso Cc: netfilter-devel@vger.kernel.org Subject: [nft PATCH 2/4] netlink: Fix leaks in netlink_parse_cmp() Date: Mon, 20 Jan 2020 17:25:38 +0100 Message-Id: <20200120162540.9699-3-phil@nwl.cc> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200120162540.9699-1-phil@nwl.cc> References: <20200120162540.9699-1-phil@nwl.cc> MIME-Version: 1.0 Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org This fixes several problems at once: * Err path would leak expr 'right' in two places and 'left' in one. * Concat case would leak 'right' by overwriting the pointer. Introduce a temporary variable to hold the new pointer. Fixes: 6377380bc265f ("netlink_delinearize: handle relational and lookup concat expressions") Signed-off-by: Phil Sutter Acked-by: Pablo Neira Ayuso --- src/netlink_delinearize.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index 06a0312b9921a..88dbd5a8ecdf3 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -274,7 +274,7 @@ static void netlink_parse_cmp(struct netlink_parse_ctx *ctx, { struct nft_data_delinearize nld; enum nft_registers sreg; - struct expr *expr, *left, *right; + struct expr *expr, *left, *right, *tmp; enum ops op; sreg = netlink_parse_register(nle, NFTNL_EXPR_CMP_SREG); @@ -291,19 +291,26 @@ static void netlink_parse_cmp(struct netlink_parse_ctx *ctx, if (left->len > right->len && expr_basetype(left) != &string_type) { - return netlink_error(ctx, loc, "Relational expression size mismatch"); + netlink_error(ctx, loc, "Relational expression size mismatch"); + goto err_free; } else if (left->len > 0 && left->len < right->len) { expr_free(left); left = netlink_parse_concat_expr(ctx, loc, sreg, right->len); if (left == NULL) - return; - right = netlink_parse_concat_data(ctx, loc, sreg, right->len, right); - if (right == NULL) - return; + goto err_free; + tmp = netlink_parse_concat_data(ctx, loc, sreg, right->len, right); + if (tmp == NULL) + goto err_free; + expr_free(right); + right = tmp; } expr = relational_expr_alloc(loc, op, left, right); ctx->stmt = expr_stmt_alloc(loc, expr); + return; +err_free: + expr_free(left); + expr_free(right); } static void netlink_parse_lookup(struct netlink_parse_ctx *ctx, From patchwork Mon Jan 20 16:25:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 1226058 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=nwl.cc Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 481cVS0ZZ5z9sPJ for ; Tue, 21 Jan 2020 03:26:08 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729242AbgATQ0H (ORCPT ); Mon, 20 Jan 2020 11:26:07 -0500 Received: from orbyte.nwl.cc ([151.80.46.58]:60814 "EHLO orbyte.nwl.cc" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726897AbgATQ0H (ORCPT ); Mon, 20 Jan 2020 11:26:07 -0500 Received: from localhost ([::1]:45672 helo=tatos) by orbyte.nwl.cc with esmtp (Exim 4.91) (envelope-from ) id 1itZsL-00063k-6C; Mon, 20 Jan 2020 17:26:05 +0100 From: Phil Sutter To: Pablo Neira Ayuso Cc: netfilter-devel@vger.kernel.org Subject: [nft PATCH 3/4] segtree: Fix for potential NULL-pointer deref in ei_insert() Date: Mon, 20 Jan 2020 17:25:39 +0100 Message-Id: <20200120162540.9699-4-phil@nwl.cc> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200120162540.9699-1-phil@nwl.cc> References: <20200120162540.9699-1-phil@nwl.cc> MIME-Version: 1.0 Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Covscan complained about potential deref of NULL 'lei' pointer, Interestingly this can't happen as the relevant goto leading to that (in line 260) sits in code checking conflicts between new intervals and since those are sorted upon insertion, only the lower boundary may conflict (or both, but that's covered before). Given the needed investigation to proof covscan wrong and the actually wrong (but impossible) code, better fix this as if element ordering was arbitrary to avoid surprises if at some point it really becomes that. Fixes: 4d6ad0f310d6c ("segtree: check for overlapping elements at insertion") Signed-off-by: Phil Sutter --- src/segtree.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/segtree.c b/src/segtree.c index e8e32412f3a41..04c0e915263b9 100644 --- a/src/segtree.c +++ b/src/segtree.c @@ -205,8 +205,11 @@ static int ei_insert(struct list_head *msgs, struct seg_tree *tree, pr_gmp_debug("insert: [%Zx %Zx]\n", new->left, new->right); if (lei != NULL && rei != NULL && lei == rei) { - if (!merge) + if (!merge) { + expr_binary_error(msgs, lei->expr, new->expr, + "conflicting intervals specified"); goto err; + } /* * The new interval is entirely contained in the same interval, * split it into two parts: @@ -228,8 +231,11 @@ static int ei_insert(struct list_head *msgs, struct seg_tree *tree, ei_destroy(lei); } else { if (lei != NULL) { - if (!merge) + if (!merge) { + expr_binary_error(msgs, lei->expr, new->expr, + "conflicting intervals specified"); goto err; + } /* * Left endpoint is within lei, adjust it so we have: * @@ -248,8 +254,11 @@ static int ei_insert(struct list_head *msgs, struct seg_tree *tree, } } if (rei != NULL) { - if (!merge) + if (!merge) { + expr_binary_error(msgs, rei->expr, new->expr, + "conflicting intervals specified"); goto err; + } /* * Right endpoint is within rei, adjust it so we have: * @@ -276,8 +285,7 @@ static int ei_insert(struct list_head *msgs, struct seg_tree *tree, return 0; err: errno = EEXIST; - return expr_binary_error(msgs, lei->expr, new->expr, - "conflicting intervals specified"); + return -1; } /* From patchwork Mon Jan 20 16:25:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Sutter X-Patchwork-Id: 1226056 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=nwl.cc Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 481cVC6JfJz9sR1 for ; Tue, 21 Jan 2020 03:25:55 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729147AbgATQZz (ORCPT ); Mon, 20 Jan 2020 11:25:55 -0500 Received: from orbyte.nwl.cc ([151.80.46.58]:60802 "EHLO orbyte.nwl.cc" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726897AbgATQZz (ORCPT ); Mon, 20 Jan 2020 11:25:55 -0500 Received: from localhost ([::1]:45660 helo=tatos) by orbyte.nwl.cc with esmtp (Exim 4.91) (envelope-from ) id 1itZs9-000633-RM; Mon, 20 Jan 2020 17:25:53 +0100 From: Phil Sutter To: Pablo Neira Ayuso Cc: netfilter-devel@vger.kernel.org Subject: [nft PATCH 4/4] netlink: Avoid potential NULL-pointer deref in netlink_gen_payload_stmt() Date: Mon, 20 Jan 2020 17:25:40 +0100 Message-Id: <20200120162540.9699-5-phil@nwl.cc> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200120162540.9699-1-phil@nwl.cc> References: <20200120162540.9699-1-phil@nwl.cc> MIME-Version: 1.0 Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org With payload_needs_l4csum_update_pseudohdr() unconditionally dereferencing passed 'desc' parameter and a previous check for it to be non-NULL, make sure to call the function only if input is sane. Fixes: 68de70f2b3fc6 ("netlink_linearize: fix IPv6 layer 4 checksum mangling") Signed-off-by: Phil Sutter Acked-by: Pablo Neira Ayuso --- src/netlink_linearize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c index 498326d0087a1..cb1b7fe1748b2 100644 --- a/src/netlink_linearize.c +++ b/src/netlink_linearize.c @@ -941,7 +941,7 @@ static void netlink_gen_payload_stmt(struct netlink_linearize_ctx *ctx, nftnl_expr_set_u32(nle, NFTNL_EXPR_PAYLOAD_CSUM_OFFSET, csum_off / BITS_PER_BYTE); } - if (expr->payload.base == PROTO_BASE_NETWORK_HDR && + if (expr->payload.base == PROTO_BASE_NETWORK_HDR && desc && payload_needs_l4csum_update_pseudohdr(expr, desc)) nftnl_expr_set_u32(nle, NFTNL_EXPR_PAYLOAD_FLAGS, NFT_PAYLOAD_L4CSUM_PSEUDOHDR);