From patchwork Sat Jul 1 02:54:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steffan Karger X-Patchwork-Id: 104 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director2.mail.ord1d.rsapps.net ([172.30.191.6]) by backend31.mail.ord1d.rsapps.net (Dovecot) with LMTP id czqLFR3TH1pxQAAAgoeIoA for ; Thu, 30 Nov 2017 04:45:01 -0500 Received: from proxy17.mail.ord1d.rsapps.net ([172.30.191.6]) by director2.mail.ord1d.rsapps.net (Dovecot) with LMTP id c/mAFR3TH1q7XwAAgYhSiA ; Thu, 30 Nov 2017 04:45:01 -0500 Received: from smtp24.gate.ord1d ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy17.mail.ord1d.rsapps.net (Dovecot) with LMTP id QnpeFB3TH1ooLQAAWC7mWg ; Thu, 30 Nov 2017 04:45:01 -0500 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO Authentication-Results: smtp24.gate.ord1d.rsapps.net x-tls.subject="/OU=GT58646928/OU=See www.geotrust.com/resources/cps (c)14/OU=Domain Control Validated - QuickSSL(R) Premium/CN=ns2.fox-it.com"; auth=pass (cipher=DHE-RSA-AES256-SHA) X-Virus-Scanned: OK X-Orig-To: patchwork@openvpn.net X-Originating-Ip: [178.250.144.131] Authentication-Results: smtp24.gate.ord1d.rsapps.net; iprev=pass policy.iprev="178.250.144.131"; spf=pass smtp.mailfrom="steffan.karger@fox-it.com" smtp.helo="ns2.fox-it.com"; dkim=none (message not signed) header.d=none; dmarc=none (p=nil; dis=none) header.from=fox-it.com X-Classification-ID: 21b6435e-d5b3-11e7-9bf5-52540091a1c4-1-1 Received: from [178.250.144.131] ([178.250.144.131:8294] helo=ns2.fox-it.com) by smtp24.gate.ord1d.rsapps.net (envelope-from ) (ecelerity 4.2.1.56364 r(Core:4.2.1.14)) with ESMTPS (cipher=DHE-RSA-AES256-SHA subject="/OU=GT58646928/OU=See www.geotrust.com/resources/cps (c)14/OU=Domain Control Validated - QuickSSL(R) Premium/CN=ns2.fox-it.com") id 71/86-17519-C13DF1A5; Thu, 30 Nov 2017 04:45:01 -0500 Received: from FOXDFT52.FOX.local (unknown [10.0.0.129]) by ns2.fox-it.com (Postfix) with ESMTPS id B05841C50A4 for ; Thu, 30 Nov 2017 10:44:59 +0100 (CET) Received: from steffan-fox (172.16.5.186) by FOXDFT52.FOX.local (10.0.0.129) with Microsoft SMTP Server (TLS) id 15.0.1293.2; Thu, 30 Nov 2017 10:44:59 +0100 Resent-From: Steffan Karger Resent-Date: Thu, 30 Nov 2017 10:44:58 +0100 Resent-Message-ID: <20171130094458.GF5754@steffan-fox> Resent-To: Received: from FOXDFT52.FOX.local (10.0.0.129) by FOXDFT52.FOX.local (10.0.0.129) with Microsoft SMTP Server (TLS) id 15.0.1293.2 via Mailbox Transport; Sat, 1 Jul 2017 14:54:13 +0200 Received: from steffan-fox.fox.local (172.16.5.170) by FOXDFT52.FOX.local (10.0.0.129) with Microsoft SMTP Server (TLS) id 15.0.1293.2; Sat, 1 Jul 2017 14:54:13 +0200 From: Steffan Karger To: CC: Steffan Karger Subject: [PATCH 4/5] buffer_list_aggregate_separator(): prevent 0-byte malloc Date: Sat, 1 Jul 2017 14:54:01 +0200 Message-ID: <1498913642-32459-4-git-send-email-steffan.karger@fox-it.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1498913642-32459-1-git-send-email-steffan.karger@fox-it.com> References: <1498913642-32459-1-git-send-email-steffan.karger@fox-it.com> X-ClientProxiedBy: FOXDFT52.FOX.local (10.0.0.129) To FOXDFT52.FOX.local (10.0.0.129) MIME-Version: 1.0 X-ClientProxiedBy: FOXDFT52.FOX.local (10.0.0.129) To FOXDFT52.FOX.local (10.0.0.129) X-getmail-retrieved-from-mailbox: Inbox As pointed out in finding OVPN-05 of the cryptograpy engineering audit (funded by Private Internet Access), buffer_list_aggregate_separator() could perform a 0-byte malloc when called with a list of 0-length buffers and a "" separator. If other could would later try to access that buffer memory, this would result in undefined behaviour. To prevent this, always malloc() 1 byte. To simplify as we go, use alloc_buf() to allocate the buffer. This has the additional benefit that the actual buffer data (not the contents) is zero-terminated, because alloc_buf() calls calloc() and we have 1 extra byte of data. Signed-off-by: Steffan Karger --- src/openvpn/buffer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 2700ad1..ecbdc1b 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -1261,8 +1261,7 @@ buffer_list_aggregate_separator(struct buffer_list *bl, const size_t max_len, struct buffer_entry *e = bl->head, *f; ALLOC_OBJ_CLEAR(f, struct buffer_entry); - f->buf.data = malloc(size); - check_malloc_return(f->buf.data); + f->buf = alloc_buf(size+1); /* prevent 0-byte malloc */ f->buf.capacity = size; for (i = 0; e && i < count; ++i) {