From patchwork Wed Feb 1 17:07:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Selva Nair X-Patchwork-Id: 3037 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7300:c95:b0:82:e4b3:40a0 with SMTP id p21csp584726dyk; Wed, 1 Feb 2023 09:08:25 -0800 (PST) X-Google-Smtp-Source: AK7set/H2ggSIQz0vn3ny79VfBuRymohY75JWJNrco13QJC7krIHBqWgcDfxAd3E7pR0XdI36mq9 X-Received: by 2002:a9d:19a6:0:b0:68b:c6d9:962a with SMTP id k35-20020a9d19a6000000b0068bc6d9962amr1408924otk.22.1675271305482; Wed, 01 Feb 2023 09:08:25 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1675271305; cv=none; d=google.com; s=arc-20160816; b=sv5J12OZ4VHAaZP6v5+N2Jg4VENpqtVZzAHb0XWi6jzhnrmfTYdlg0/W7icDgTzFHy /eCBo8swn/Vd07TPPUyi3DzpiLYnya2379VhUwICCCs7kWG7JepTcqp43LCwgRp/OtaF 5/N7re+GE1XvZUraj0dbuAgDn30iV/0xBNk6l5FQrsziOtes3gCR2/6IliQefq/L3A04 XIpYnMIaqJBbMTB1xfZm6zogYijwh69xCJFA5bjvSN45hO/RR4eoMSkVHr+6REyApMb4 nP1fRqvxVwbDx4lHMhmhX1okypi7SfMkuhZc4B9KuF0Nu6dQwafYwDFt594nhl0xQHcA YEfA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:content-transfer-encoding:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence:subject :mime-version:references:in-reply-to:message-id:date:to:from :dkim-signature:dkim-signature:dkim-signature; bh=Niu+2VcIs4vSwEy4AAABnfefvR6VmOV2YU5zWU5Osys=; b=nfeq1oSVBEWwxWB/AbwywUzVl+WMvag1SwXDu8nFBjHrDkxj+oVqGfxaZlBZlAFeQB jnN4J0TQoEkhsE/kVTpHDk6K+9fo7ns7SnryNXAySGZIaX73ER2E91DvoL8q1VXce9Q2 3T/JE2eZIP4vy3MwVDpfijj4RqPaEWTANh8KJsUjglMozCeZJ8FtOoY2C+oXjM/z9ngn 0B2d1bH5In3PjFfy6J6E+raxcpYp4fNZoiPN4jjELEmAiwOr3JyIQWc8dRvISIjqNkCl g9uX391tKnf0+/siSuuiaULOAmioyRfBAONSh5fAhJcpHLSMC247n7VYHnR3o3JyBwHH ITyQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=VJaPizHb; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=mmsMRqJJ; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20210112 header.b=A2myE4zt; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id l83-20020acabb56000000b0037805c4fb56si13112471oif.54.2023.02.01.09.08.25 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 01 Feb 2023 09:08:25 -0800 (PST) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=VJaPizHb; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=mmsMRqJJ; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20210112 header.b=A2myE4zt; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1pNGaa-0003XQ-9v; Wed, 01 Feb 2023 17:08:02 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1pNGaZ-0003XA-9V for openvpn-devel@lists.sourceforge.net; Wed, 01 Feb 2023 17:08:01 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=06bqQSQzNvGLoRvieH5Ls8dYiVifkgjc0DzNzNNG3PY=; b=VJaPizHb7lwFnBKZc/m/dkkT77 yNbi4Vbt0OuGUvaZA2ABhZeCOmIICRyFuGOL/ThoqLygquUZXE79I0qO37pYe5XoSIND3CdZhFmCy tYOdcFWdWHdsF1GJgOAv0cZaW1bU7AGZH1ypMkgkvYEe4HP7l/yRiD/elRVYQ4S0klTU=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id: Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=06bqQSQzNvGLoRvieH5Ls8dYiVifkgjc0DzNzNNG3PY=; b=mmsMRqJJZQYFylZ63E3Ec4Hylm GLUx93UdIMh3SEfAMiNNb7u9I/rwaFdreOKO69haXZd2UVxPhXTZbKnyzykONMIGvF/F4YlvfTVMI MsajfDRGyXy1p8gSUPhw91UZMB29GWj3HRv9tEAedVoPOMDnzKd/K5kov3WQ+R+LyG/M=; Received: from mail-il1-f173.google.com ([209.85.166.173]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1pNGaS-00036B-Ow for openvpn-devel@lists.sourceforge.net; Wed, 01 Feb 2023 17:07:58 +0000 Received: by mail-il1-f173.google.com with SMTP id x6so1028434ilv.7 for ; Wed, 01 Feb 2023 09:07:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=06bqQSQzNvGLoRvieH5Ls8dYiVifkgjc0DzNzNNG3PY=; b=A2myE4zt+UsClmaUjdzKIkRiY/lEOTRJlghIBsnoM3/Vzi9F8HECnUIkuK/Aqtl9mI dOBv41s71ZrwKBOScUXPv+ECYISio8GGJQdrqz1Onlqiq/2ZObIeRb1Ccnmgxtw4G/dg yBFqJrNOcIXKpkeUgALbdy6bDY7WaZ8/Q0fBp6oK7SPQOghNTthq2jWzRNNeu3tPjvGP 6BcVQhLHXVdD8y2JgRjcTujN7rDKQfUJ9WjoAybzjVhRjxC161Mi1ctVxOQ/LYolWSAy ASMz57qVkycZ7s0vzMclgHJsU1+sBZ1ipDuHd7airnP7TA8VO+RSIn7D7LLKoQVe0dRg wyoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=06bqQSQzNvGLoRvieH5Ls8dYiVifkgjc0DzNzNNG3PY=; b=X1BJK6ddPBW7dZbgE7uw7fBkGi0bAujQ/N/KvJDAqwnXdRjjdhJm85LKwCqN53eN4k lVP/FPoVBisPzw0/zxKfMuImln707/ezVS8+ZfA5yKv/b7zuf1KUorRh++0B/LIwiDtc nRkP/BbGflvK5OpcSMzicQgCKIj/klRPOQ5qg75f6wwh3Sp+B7mn7kRNxFa0r7W+7o+N P7DUKwjJWZ5jgllMe8pgiBX1bwXRgPOfhSg+5TdOu12F+Ic8MktfUpIRWcZgTdXa/Q2t 1y7JT3aIRCLcKpsfK+iDEQl+wvmzXtOzN0dca6htirqQPQ1l2tI4yugs5Jx7iuB3aShL 6RSw== X-Gm-Message-State: AO0yUKUXc1htKyDauunpW851UqJQDolBbo+y3tmzoKIf8v3Czb5eBOp7 RpMDwxI1Zdbn4mrMG+WyS/OTi5BiJLg= X-Received: by 2002:a92:1e0f:0:b0:30c:1dda:42dd with SMTP id e15-20020a921e0f000000b0030c1dda42ddmr1875778ile.1.1675271271574; Wed, 01 Feb 2023 09:07:51 -0800 (PST) Received: from uranus.sansel.ca (bras-vprn-tnhlon4053w-lp130-01-70-51-222-66.dsl.bell.ca. [70.51.222.66]) by smtp.gmail.com with ESMTPSA id p4-20020a056e0206c400b002fc323a2902sm5896012ils.62.2023.02.01.09.07.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Feb 2023 09:07:51 -0800 (PST) From: selva.nair@gmail.com To: openvpn-devel@lists.sourceforge.net Date: Wed, 1 Feb 2023 12:07:35 -0500 Message-Id: <20230201170735.2266851-1-selva.nair@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230201041831.2259419-1-selva.nair@gmail.com> References: <20230201041831.2259419-1-selva.nair@gmail.com> MIME-Version: 1.0 X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-2.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: From: Selva Nair - An item added to undo-list was not removed on error, causing attempt to free again in Undo(). Also fix a memory leak possibility in the same context. Github: fixes OpenVPN/openvpn#232 Content analysis details: (-0.2 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 SPF_PASS SPF: sender matches SPF record 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider [selva.nair[at]gmail.com] -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.166.173 listed in list.dnswl.org] -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.166.173 listed in wl.mailspike.net] -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain X-Headers-End: 1pNGaS-00036B-Ow Subject: [Openvpn-devel] [PATCH v2] block-dns using iservice: fix a potential double free X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1756600948824892958?= X-GMAIL-MSGID: =?utf-8?q?1756649284241095664?= From: Selva Nair - An item added to undo-list was not removed on error, causing attempt to free again in Undo(). Also fix a memory leak possibility in the same context. Github: fixes OpenVPN/openvpn#232 v2: Split add and delete functions and reuse the delete function for cleanup. Signed-off-by: Selva Nair Acked-by: Lev Stipakov --- Same as PR #235 except for DeleteBlockDNS moved up and the its forward declaration removed. src/openvpnserv/interactive.c | 132 ++++++++++++++++++++-------------- 1 file changed, 78 insertions(+), 54 deletions(-) diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c index 03361d66..a3d43752 100644 --- a/src/openvpnserv/interactive.c +++ b/src/openvpnserv/interactive.c @@ -777,87 +777,111 @@ CmpAny(LPVOID item, LPVOID any) } static DWORD -HandleBlockDNSMessage(const block_dns_message_t *msg, undo_lists_t *lists) +DeleteBlockDNS(const block_dns_message_t *msg, undo_lists_t *lists) { DWORD err = 0; - block_dns_data_t *interface_data; + block_dns_data_t *interface_data = RemoveListItem(&(*lists)[block_dns], CmpAny, NULL); + + if (interface_data) + { + err = delete_block_dns_filters(interface_data->engine); + if (interface_data->metric_v4 >= 0) + { + set_interface_metric(msg->iface.index, AF_INET, + interface_data->metric_v4); + } + if (interface_data->metric_v6 >= 0) + { + set_interface_metric(msg->iface.index, AF_INET6, + interface_data->metric_v6); + } + free(interface_data); + } + else + { + MsgToEventLog(M_ERR, TEXT("No previous block DNS filters to delete")); + } + + return err; +} + +static DWORD +AddBlockDNS(const block_dns_message_t *msg, undo_lists_t *lists) +{ + DWORD err = 0; + block_dns_data_t *interface_data = NULL; HANDLE engine = NULL; LPCWSTR exe_path; exe_path = settings.exe_path; - if (msg->header.type == msg_add_block_dns) + err = add_block_dns_filters(&engine, msg->iface.index, exe_path, BlockDNSErrHandler); + if (!err) { - err = add_block_dns_filters(&engine, msg->iface.index, exe_path, BlockDNSErrHandler); + interface_data = malloc(sizeof(block_dns_data_t)); + if (!interface_data) + { + err = ERROR_OUTOFMEMORY; + goto out; + } + interface_data->engine = engine; + interface_data->index = msg->iface.index; + int is_auto = 0; + interface_data->metric_v4 = get_interface_metric(msg->iface.index, + AF_INET, &is_auto); + if (is_auto) + { + interface_data->metric_v4 = 0; + } + interface_data->metric_v6 = get_interface_metric(msg->iface.index, + AF_INET6, &is_auto); + if (is_auto) + { + interface_data->metric_v6 = 0; + } + + err = AddListItem(&(*lists)[block_dns], interface_data); if (!err) { - interface_data = malloc(sizeof(block_dns_data_t)); - if (!interface_data) - { - return ERROR_OUTOFMEMORY; - } - interface_data->engine = engine; - interface_data->index = msg->iface.index; - int is_auto = 0; - interface_data->metric_v4 = get_interface_metric(msg->iface.index, - AF_INET, &is_auto); - if (is_auto) - { - interface_data->metric_v4 = 0; - } - interface_data->metric_v6 = get_interface_metric(msg->iface.index, - AF_INET6, &is_auto); - if (is_auto) - { - interface_data->metric_v6 = 0; - } - err = AddListItem(&(*lists)[block_dns], interface_data); + err = set_interface_metric(msg->iface.index, AF_INET, + BLOCK_DNS_IFACE_METRIC); if (!err) { - err = set_interface_metric(msg->iface.index, AF_INET, + err = set_interface_metric(msg->iface.index, AF_INET6, BLOCK_DNS_IFACE_METRIC); - if (!err) - { - set_interface_metric(msg->iface.index, AF_INET6, - BLOCK_DNS_IFACE_METRIC); - } - } - } - } - else - { - interface_data = RemoveListItem(&(*lists)[block_dns], CmpAny, NULL); - if (interface_data) - { - engine = interface_data->engine; - err = delete_block_dns_filters(engine); - engine = NULL; - if (interface_data->metric_v4 >= 0) - { - set_interface_metric(msg->iface.index, AF_INET, - interface_data->metric_v4); } - if (interface_data->metric_v6 >= 0) + if (err) { - set_interface_metric(msg->iface.index, AF_INET6, - interface_data->metric_v6); + /* delete the filters, remove undo item and free interface data */ + DeleteBlockDNS(msg, lists); + engine = NULL; } - free(interface_data); - } - else - { - MsgToEventLog(M_ERR, TEXT("No previous block DNS filters to delete")); } } +out: if (err && engine) { delete_block_dns_filters(engine); + free(interface_data); } return err; } +static DWORD +HandleBlockDNSMessage(const block_dns_message_t *msg, undo_lists_t *lists) +{ + if (msg->header.type == msg_add_block_dns) + { + return AddBlockDNS(msg, lists); + } + else + { + return DeleteBlockDNS(msg, lists); + } +} + /* * Execute a command and return its exit code. If timeout > 0, terminate * the process if still running after timeout milliseconds. In that case