From patchwork Wed Nov 1 11:03:42 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steffan Karger X-Patchwork-Id: 41 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director6.mail.ord1d.rsapps.net ([172.30.191.6]) by backend31.mail.ord1d.rsapps.net (Dovecot) with LMTP id rR05BOtE+ll8dwAAgoeIoA for ; Wed, 01 Nov 2017 18:04:27 -0400 Received: from proxy4.mail.ord1d.rsapps.net ([172.30.191.6]) by director6.mail.ord1d.rsapps.net (Dovecot) with LMTP id Ib6FHOtE+lmrbwAAhgvE6Q ; Wed, 01 Nov 2017 18:04:27 -0400 Received: from smtp50.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy4.mail.ord1d.rsapps.net (Dovecot) with LMTP id g69+FetE+lmXKgAAiYrejw ; Wed, 01 Nov 2017 18:04:27 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-MessageSniffer-Scan-Result: 0 X-MessageSniffer-Rules: 0-298-1118-1327-w 0-298-1118-1691-w 0-298-0-11882-f X-CMAE-Scan-Result: 0 X-CNFS-Analysis: v=2.2 cv=V4w/6qvi c=1 sm=1 tr=0 a=Q8DxjiC8O3VT/NpP1XjEZQ==:117 a=Q8DxjiC8O3VT/NpP1XjEZQ==:17 a=kj9zAlcOel0A:10 a=xqWC_Br6kY4A:10 a=MKtGQD3n3ToA:10 a=1oJP67jkp3AA:10 a=sC3jslCIGhcA:10 a=9hBjR9qptiMA:10 a=WiVod9pSvdkA:10 a=ZZnuYtJkoWoA:10 a=9sSjY8p1AAAA:8 a=P_JWiMecAAAA:8 a=FP58Ms26AAAA:8 a=avj4neX-z-FaSJBd_tMA:9 a=CjuIK1q_8ugA:10 a=-FEs8UIgK8oA:10 a=NWVoK91CQyQA:10 a=ub54wNWiXv_DzeFsgEJW:22 a=D0-HAvA3Hk9NMREbgwuX:22 X-Orig-To: justin@openvpn.net X-Originating-Ip: [216.34.181.88] Authentication-Results: smtp50.gate.ord1c.rsapps.net; iprev=pass policy.iprev="216.34.181.88"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dkim=fail (signature verification failed) header.d=karger-me.20150623.gappssmtp.com; dmarc=none (p=nil; dis=none) header.from=karger.me X-Classification-ID: a0590f4e-bf50-11e7-b77f-b8ca3a659864-1-1 Received: from [216.34.181.88] ([216.34.181.88:32504] helo=lists.sourceforge.net) by smtp50.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.1.56364 r(Core:4.2.1.14)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 87/87-18157-AE44AF95; Wed, 01 Nov 2017 18:04:26 -0400 Received: from localhost ([127.0.0.1] helo=sfs-ml-2.v29.ch3.sourceforge.com) by sfs-ml-2.v29.ch3.sourceforge.com with esmtp (Exim 4.89) (envelope-from ) id 1eA17C-0000As-Oy; Wed, 01 Nov 2017 22:04:02 +0000 Received: from sog-mx-4.v43.ch3.sourceforge.com ([172.29.43.194] helo=mx.sourceforge.net) by sfs-ml-2.v29.ch3.sourceforge.com with esmtps (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.89) (envelope-from ) id 1eA17B-0000AW-Le for openvpn-devel@lists.sourceforge.net; Wed, 01 Nov 2017 22:04:01 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=8n5OBZONiUJXnkNFVJ1GtvPCGGjr/Hbvqash5R9Qmh8=; b=fUw6P5GWD0gfPVv9qNQ3FIDXhftBoTtONqZieAz2JRr2hZfooTGZlcVbb9c8KLr8r9KnHpQBTEQSmguFiquQKRsXsYeubbGywReIV5TKrzi61w5l52FvmL11tCo4R74eZS2hFmLkFghgxvDSoOtoX6BzTa01dLs2Ab9NePp6XzQ=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=8n5OBZONiUJXnkNFVJ1GtvPCGGjr/Hbvqash5R9Qmh8=; b=VnHOnuAaGPjiO7qcfkeB/gXVGNuctqVEGYZM0Pz2j1A9jUiB0XwVB+IpmBednWXGz+gG0GQ6liOw5fEAYgE87g7r++73OKP8HoiAcP4uI90bg8bNlsCtg92Fx8PrG/2tjFqYijU5q3cPj8gG9C2axOgBumFBQtGiBXWCIli5Y8Y=; Received-SPF: pass (sog-mx-4.v43.ch3.sourceforge.com: domain of karger.me designates 74.125.82.67 as permitted sender) client-ip=74.125.82.67; envelope-from=steffan@karger.me; helo=mail-wm0-f67.google.com; Received: from mail-wm0-f67.google.com ([74.125.82.67]) by sog-mx-4.v43.ch3.sourceforge.com with esmtps (TLSv1:AES128-SHA:128) (Exim 4.76) id 1eA17A-00063d-I2 for openvpn-devel@lists.sourceforge.net; Wed, 01 Nov 2017 22:04:01 +0000 Received: by mail-wm0-f67.google.com with SMTP id z3so7497127wme.5 for ; Wed, 01 Nov 2017 15:04:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=karger-me.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=8n5OBZONiUJXnkNFVJ1GtvPCGGjr/Hbvqash5R9Qmh8=; b=ozzztnHdwq+4Sr9x2luRxlGFlNHpNBAGuUewunSy+oCmOi1oEmStdkEnRL79OhhQN6 xzawkq+S/UHO9ql+q5qYX6m4yNKXHlraPPdE1nc8RsyR4+TqeKZjiBlgmOCk5yQsf4z/ L0IdiWfAZjf4TIO8jlw6Ibh9K03vum72z+1WdUIosFjOVjrPMKcyazYQniw1pLIT+Htr mVV5QrHUUQGuF2U/JUmrylUlpsaTJsYV6YxYBjqY0d7H2iebUNr0r19J5ZFw/QOmw/qu KSHriEB9Oq1K04rKg4aYdyq2CDuJ3nyYc4hvfcXlrw40kPtynHGS8D6IDEtTrBNu74ml 28Rw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=8n5OBZONiUJXnkNFVJ1GtvPCGGjr/Hbvqash5R9Qmh8=; b=s8o4dKWY2Az3PhX0tviGHDIWrkkObMF6AJH4tpWyTj0JR1SAPi44GhmD6ZuyTZcf2A Sm3xEjjkcpSqpdrNNsXHQq665L/yy85tkS0nhXocd75CpFjUVuNe1VLWUd7p9XdLqzgU 3wpLiCSkjd9Wyq7IKycfAtlZN8g6/o7d/O7l16BpjV2u3H1MmEBKdLoPxymLn0MjqN8W wToquGsftwe8+XiMET7dYQk9wP+9GdQCpCogmFqivLIwo3LTUV6+fDwG18lPhoUb4muv i5ykEJa4BNGqx75e2MCZ1FANwN6iUbK+znme0s5b6lbhvm1JOP7ZbnmDAvFO7/ylNM26 IyOg== X-Gm-Message-State: AMCzsaWfy0if2LbdtQqMEaeoeGE6k2+BkF/jmFVUuunWnZDn4EmGviIj LH/ZCyLHz2YjFx7Zc6uWUyfsHJ/Zo/Q= X-Google-Smtp-Source: ABhQp+SZfjtMIifC37kHL94UyBg/1h2jUHpOaPAlwjsD0YiP6MGKH/mf7opbijpKmSb14waIlxBa9A== X-Received: by 10.80.205.28 with SMTP id z28mr1941593edi.264.1509573834269; Wed, 01 Nov 2017 15:03:54 -0700 (PDT) Received: from vesta.fritz.box ([2001:985:e54:1:f834:91b2:a7cf:128b]) by smtp.gmail.com with ESMTPSA id f39sm2096642edf.83.2017.11.01.15.03.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 01 Nov 2017 15:03:53 -0700 (PDT) From: Steffan Karger To: openvpn-devel@lists.sourceforge.net Date: Wed, 1 Nov 2017 23:03:42 +0100 Message-Id: <20171101220342.14648-5-steffan@karger.me> X-Mailer: git-send-email 2.14.1 In-Reply-To: <20171101220342.14648-1-steffan@karger.me> References: <1505424872-27434-1-git-send-email-steffan.karger@fox-it.com> <20171101220342.14648-1-steffan@karger.me> X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.5 RCVD_IN_SORBS_SPAM RBL: SORBS: sender is a spam source [74.125.82.67 listed in dnsbl.sorbs.net] -1.5 SPF_CHECK_PASS SPF reports sender host as permitted sender for sender-domain -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [74.125.82.67 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.0 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1eA17A-00063d-I2 Subject: [Openvpn-devel] [PATCH 4/4 v3] create_temp_file/gen_path: prevent memory leak if gc == NULL 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: , MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox If gc == NULL, the data allocated in the alloc_gc_buf() call in create_temp_file or the string_mod_const call in gen_path would never be free'd. These functions are currently never called that way, but let's prevent future problems. While touching create_temp_file, also remove the counter variable, which is never read, simplify the do-while to a while loop, and truncate the prefix (if needed) to preserve the random and extension of the created filename. Signed-off-by: Steffan Karger Acked-by: Antonio Quartulli --- v2: - change create_temp_file to avoid using a struct buffer (simpler) - add gc != NULL check for gen_path (avoid similar memleak pitfall) v3: - Check the return value of openvpn_snprintf() src/openvpn/misc.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 25f38003..67011169 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -723,21 +723,26 @@ test_file(const char *filename) const char * create_temp_file(const char *directory, const char *prefix, struct gc_arena *gc) { - static unsigned int counter; - struct buffer fname = alloc_buf_gc(256, gc); int fd; const char *retfname = NULL; unsigned int attempts = 0; + char fname[256] = { 0 }; + const char *fname_fmt = PACKAGE "_%.*s_%08lx%08lx.tmp"; + const int max_prefix_len = sizeof(fname) - (sizeof(PACKAGE) + 7 + (2 * 8)); - do + while (attempts < 6) { ++attempts; - ++counter; - buf_printf(&fname, PACKAGE "_%s_%08lx%08lx.tmp", prefix, - (unsigned long) get_random(), (unsigned long) get_random()); + if (!openvpn_snprintf(fname, sizeof(fname), fname_fmt, max_prefix_len, + prefix, (unsigned long) get_random(), + (unsigned long) get_random())) + { + msg(M_WARN, "ERROR: temporary filename too long"); + return NULL; + } - retfname = gen_path(directory, BSTR(&fname), gc); + retfname = gen_path(directory, fname, gc); if (!retfname) { msg(M_WARN, "Failed to create temporary filename and path"); @@ -760,7 +765,6 @@ create_temp_file(const char *directory, const char *prefix, struct gc_arena *gc) return NULL; } } - while (attempts < 6); msg(M_WARN, "Failed to create temporary file after %i attempts", attempts); return NULL; @@ -812,6 +816,12 @@ gen_path(const char *directory, const char *filename, struct gc_arena *gc) #else const int CC_PATH_RESERVED = CC_SLASH; #endif + + if (!gc) + { + return NULL; /* Would leak memory otherwise */ + } + const char *safe_filename = string_mod_const(filename, CC_PRINT, CC_PATH_RESERVED, '_', gc); if (safe_filename