From patchwork Tue Jan 2 11:20:02 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steffan Karger X-Patchwork-Id: 158 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director2.mail.ord1d.rsapps.net ([172.27.255.7]) by backend31.mail.ord1d.rsapps.net (Dovecot) with LMTP id /ScBNOsFTFrqagAAgoeIoA for ; Tue, 02 Jan 2018 17:21:32 -0500 Received: from proxy10.mail.iad3a.rsapps.net ([172.27.255.7]) by director2.mail.ord1d.rsapps.net (Dovecot) with LMTP id vVhQAusFTFpCYAAAgYhSiA ; Tue, 02 Jan 2018 17:21:32 -0500 Received: from smtp4.gate.iad3a ([172.27.255.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy10.mail.iad3a.rsapps.net (Dovecot) with LMTP id 0ulVMesFTFo/HAAAnQ/bqA ; Tue, 02 Jan 2018 17:21:31 -0500 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.34.181.88] Authentication-Results: smtp4.gate.iad3a.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: 48835dd8-f00b-11e7-b457-5254003c557e-1-1 Received: from [216.34.181.88] ([216.34.181.88:17390] helo=lists.sourceforge.net) by smtp4.gate.iad3a.rsapps.net (envelope-from ) (ecelerity 4.2.1.56364 r(Core:4.2.1.14)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id BA/5A-10881-BE50C4A5; Tue, 02 Jan 2018 17:21:31 -0500 Received: from localhost ([127.0.0.1] helo=sfs-ml-1.v29.ch3.sourceforge.com) by sfs-ml-1.v29.ch3.sourceforge.com with esmtp (Exim 4.89) (envelope-from ) id 1eWUuw-0005Qz-Ms; Tue, 02 Jan 2018 22:20:18 +0000 Received: from sfi-mx-2.v28.ch3.sourceforge.com ([172.29.28.192] helo=mx.sourceforge.net) by sfs-ml-1.v29.ch3.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) (envelope-from ) id 1eWUuv-0005Qr-HJ for openvpn-devel@lists.sourceforge.net; Tue, 02 Jan 2018 22:20:17 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: MIME-Version:Content-Type:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=raxf7KMAGR8DgeVAQAHTjeNNtmxeHMlhF3rp+2bphxc=; b=i73vpFna+sJIsv8JUKI7/+OjSO 8uHcegKtPj2yUYv0WSkYPbjodTqwB3TGF1ZwNmZSbHwlmDJMNNpv1oO5EuYOEdNQ5ux4rGUuI+xnh m1mSoSEFpo4trHTm2Y9HYsrR52qrQfHSV94iio0+i4T9w2V9TW3iwlU0e6pVMLjOcBVc=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:MIME-Version: Content-Type:Content-Transfer-Encoding:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=raxf7KMAGR8DgeVAQAHTjeNNtmxeHMlhF3rp+2bphxc=; b=Pz1p+iJLxfz9I2FtRq6GDjKWw4 zx3WbXOuAHfh48+gdg+GS62OB9l9UkNUbS7Jjss69uiY1OSkLLWu4+zuwP+C5V3H5MSig9pbvnscs 3oWEbHyNlkToqvHVZtbYbo4o63vxvbWLnRBPvtFHGgHns0YMrrFhduPEqeq6JiBeEXRI=; Received: from mail-wm0-f65.google.com ([74.125.82.65]) by sfi-mx-2.v28.ch3.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.89) id 1eWUut-0005zu-P1 for openvpn-devel@lists.sourceforge.net; Tue, 02 Jan 2018 22:20:17 +0000 Received: by mail-wm0-f65.google.com with SMTP id t8so334401wmc.3 for ; Tue, 02 Jan 2018 14:20:15 -0800 (PST) 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; bh=raxf7KMAGR8DgeVAQAHTjeNNtmxeHMlhF3rp+2bphxc=; b=l4J01YnUNnIvO0q+3ypyLL/3U/9BXaru65yMOgh0MypC3kzfReSkQwCp1rbqzjv7Xs jfB0PWg9EIpk1bx+aRaxLshN+1/FkFkq22bSU5dlFcM/JO3eKtsc7R3PT8aoSMoK3Oqr CDjbpAzcqowQDlHJXwbHappnwJ2h5d03RAp29UsNHiMZa3dDRpeBoW+v7rLyY2vP2TO3 326Xpj10N5Gu7g3u0UoRzwp9rMheTGBrYtrYEsMfTFbSGb24AbQR8bgAguQdu/YXpwRi bRG6NRNRDMktinOjKuLJhVanAFlr1Jkcoc3WpnrDce2AqSrC/SNvnKgNQdUm6jQiHFCT 61wQ== 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; bh=raxf7KMAGR8DgeVAQAHTjeNNtmxeHMlhF3rp+2bphxc=; b=GeZ7lnbBBfw+4+3q9w4ohNgEWPnyLJFLUOZxfk5Eozw+HyGU66AVgqDbCz3IOsShI7 FYt+oC5CIOTAOtW6Id2RxcNK2yiKTsX0zeuOwlCY8lg6U0ry2EK2SOjnYcz8iQDt8O6f pf34mfMKrdDK0IrZEFsWxPSEFBUEHJD5WO6FujqjOOnDiRiRMsfeFzrTuokpQ7V0YtsA aZ/BdEMXP0zthSw/+NiqMSWqUEzOHBCadRnWzKrABb13yqp5Q6MlvfICy/Bzk2b0vMsA 6FSVZDbBDFfp6kzvcys2sbOqbvffmNCDEwhqKrRuUY4cWvyrSn+Zi9keuRKlC1RgPGTK vhNA== X-Gm-Message-State: AKGB3mIFTRvXmSGxP3hU5AdegFeMcCboks+qAVmpISW0DoujQ0QiKIRH h8mEccFQgPwLSQcZ1UCjJo0zLGtzqWQ= X-Google-Smtp-Source: ACJfBovi/nj5whj3kNrxKPThrhDYO7RlM/8HG0EvHq+ECsnkBYF6+48Up+VVfDoVH9ni2bRWkkhdrw== X-Received: by 10.80.149.152 with SMTP id w24mr64641487eda.76.1514931609492; Tue, 02 Jan 2018 14:20:09 -0800 (PST) Received: from vesta.fritz.box ([2001:985:e54:1:e919:5698:c7b3:13b6]) by smtp.gmail.com with ESMTPSA id a52sm41221364eda.92.2018.01.02.14.20.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 02 Jan 2018 14:20:08 -0800 (PST) From: Steffan Karger To: openvpn-devel@lists.sourceforge.net Date: Tue, 2 Jan 2018 23:20:02 +0100 Message-Id: <20180102222002.21454-1-steffan@karger.me> X-Mailer: git-send-email 2.14.1 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [74.125.82.65 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 X-Headers-End: 1eWUut-0005zu-P1 Subject: [Openvpn-devel] [PATCH] Check for time_t overflow in event_timeout_trigger() 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 As reported in trac #922, the wakeup computation in event_timeout_trigger() could overflow. Since time_t and int are signed types, that is officially undefined behvaiour. On systems with a 64-bit signed time_t (most if not all 64-bit system), the overflow was caused by the (unnecessary) cast to "int". Removing that, changing the time of "wakeup" to time_t, and assuming that the system clock on our host system will never be set to the year 292471210579 or later, this can no longer overflow on systems with a 64-bit time_t. For systems with a signed 32-bit time_t however, we need an additional check to prevent signed overflow (and thus undefined behaviour). Since time_t is only specified by C99 to be "an arithmetic type capable of representing times", and no TIME_MAX or TIME_MIN macros are available, checking for overflow is not trivial at all. This patch takes the practical approach, and assumes that time_t is of type "long int" (aka "long") or "long long int" (aka "long long"). POSIX requires time_t to be an "integer type", and all systems I know of use a long int or long long int. To be sure that this assumption holds, this patch uses static asserts. So much for this "simple" overflow check. Trac: #922 Signed-off-by: Steffan Karger --- src/openvpn/integer.h | 14 ++++++++++++++ src/openvpn/interval.c | 5 +++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h index 9bb00a38..51085450 100644 --- a/src/openvpn/integer.h +++ b/src/openvpn/integer.h @@ -77,6 +77,20 @@ constrain_int(int x, int min, int max) } } +/** Return true if the addition of a and b would overflow. */ +static inline bool +time_t_add_overflow(time_t a, time_t b) { + static_assert(((time_t) -1) < 0, "OpenVPN assumes time_t is signed"); + static_assert(((time_t) .9) == 0, "OpenVPN assumes time_t is integer type"); + static_assert(sizeof(time_t) == sizeof(long) || sizeof(time_t) == sizeof(long long), + "OpenVPN assumes that time_t is of type long int or long long int"); + static const time_t TIME_MAX = sizeof(time_t) == sizeof(long) ? + LONG_MAX : LLONG_MAX; + static const time_t TIME_MIN = sizeof(time_t) == sizeof(long) ? + LONG_MIN : LLONG_MIN; + return (a > 0 && b > TIME_MAX - a) || (a < 0 && b < TIME_MIN - a); +} + /* * Functions used for circular buffer index arithmetic. */ diff --git a/src/openvpn/interval.c b/src/openvpn/interval.c index 16343865..1f067cd2 100644 --- a/src/openvpn/interval.c +++ b/src/openvpn/interval.c @@ -51,7 +51,8 @@ event_timeout_trigger(struct event_timeout *et, if (et->defined) { - int wakeup = (int) et->last + et->n - local_now; + ASSERT(!time_t_add_overflow(et->last, et->n)); + time_t wakeup = et->last + et->n - local_now; if (wakeup <= 0) { #if INTERVAL_DEBUG @@ -72,7 +73,7 @@ event_timeout_trigger(struct event_timeout *et, if (tv && wakeup < tv->tv_sec) { #if INTERVAL_DEBUG - dmsg(D_INTERVAL, "EVENT event_timeout_wakeup (%d/%d) etcr=%d", wakeup, et->n, et_const_retry); + dmsg(D_INTERVAL, "EVENT event_timeout_wakeup (%ld/%d) etcr=%d", wakeup, et->n, et_const_retry); #endif tv->tv_sec = wakeup; tv->tv_usec = 0;