From patchwork Tue Jan 2 11:28:54 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steffan Karger X-Patchwork-Id: 168 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director4.mail.ord1d.rsapps.net ([172.30.191.6]) by backend31.mail.ord1d.rsapps.net (Dovecot) with LMTP id VNENEFY8TlqESgAAgoeIoA for ; Thu, 04 Jan 2018 09:38:14 -0500 Received: from proxy1.mail.ord1d.rsapps.net ([172.30.191.6]) by director4.mail.ord1d.rsapps.net (Dovecot) with LMTP id W7UsJVY8TlrcRwAAHDmxtw ; Thu, 04 Jan 2018 09:38:14 -0500 Received: from smtp26.gate.ord1d ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy1.mail.ord1d.rsapps.net (Dovecot) with LMTP id LGvhF1Y8TloxHQAAasrz9Q ; Thu, 04 Jan 2018 09:38:14 -0500 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO Authentication-Results: smtp26.gate.ord1d.rsapps.net x-tls.subject="/OU=Domain Control Validated/CN=www.neomailbox.net"; auth=pass (cipher=DHE-RSA-AES256-GCM-SHA384) X-Virus-Scanned: OK X-Orig-To: patchwork@openvpn.net X-Originating-Ip: [5.148.176.60] Authentication-Results: smtp26.gate.ord1d.rsapps.net; iprev=pass policy.iprev="5.148.176.60"; spf=permerror smtp.mailfrom="a@unstable.cc" smtp.helo="s2.neomailbox.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: e439b3ce-f15c-11e7-9199-525400c5b129-1-1 Received: from [5.148.176.60] ([5.148.176.60:27453] helo=s2.neomailbox.net) by smtp26.gate.ord1d.rsapps.net (envelope-from ) (ecelerity 4.2.1.56364 r(Core:4.2.1.14)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384 subject="/OU=Domain Control Validated/CN=www.neomailbox.net") id D6/81-23414-55C3E4A5; Thu, 04 Jan 2018 09:38:14 -0500 Resent-From: Antonio Quartulli Resent-To: patchwork@openvpn.net Resent-Date: Thu, 4 Jan 2018 22:37:09 +0800 Resent-Message-ID: <55a30193-7ef0-277b-46c7-c8c37bde87b9@unstable.cc> Resent-User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 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: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:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=tvQqlKwI7Kigi7O4iBI1Qk8GWzAYm63KwCklLVTbGGs=; b=gBOuems2fn/Izoaor89BJabtwg 1K/8mQEqCRwJrVZ1f+ivMoL79isnaeVZGrmZ8iPCBOSirOPjHk+zYGoK+VeFOVcbnhi5WH+OPznfT zOCEW5bOusUZhej1u1SUro0MCd9LQMd0Cw/FX9Dd3vvRh6vMDMwLbF0GogMkB2CwINMY=; 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: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:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=tvQqlKwI7Kigi7O4iBI1Qk8GWzAYm63KwCklLVTbGGs=; b=hyTSIEcMLcqQpnu9cTbZXfwl7G 6gzB5BuM+8ZcRDmzXxgsbp7TtMXMzdIvFoOPk3YswWLU7vIQFD0DYaU0sHzpSYTjaTDKlAbmmCMjP JOZjUiuRSx+bmCB1rF3MVeAc4n6+fgiZVGXzn8dkNFrttqwjUxBj1hMZGcM0kpCuRJUs=; 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=tvQqlKwI7Kigi7O4iBI1Qk8GWzAYm63KwCklLVTbGGs=; b=K+qMXGJf6DsgJZbvjDwX6oySzWSOyquCNqcS6j1FRK8rLrH2HnJK3fIJ4GIyWoF8yw MgQsjJz7J1hOpSABXHTZ9KhuEtDMITawQRfHZnRVsfQIcImvef9S6pkheTSq1K+6Oeix qWY5vL1gS5sBEROObph+qmqDVoivEKEddvSycxVUAhy7n3V/aiS0lnAOGSvkLE3MJVkY QbfUbOhTQIGdVw4q0H9bkp4ZAsVy/hMMfDTs0sS0O8tcRQrrjKuGGyvgR3Rjy+i4itYT Z+zTfEeru4vdkCEYPIQxx57w8JGT7nXSg6avEz5WwrajPs67wJ2X6Mdvnnz880yd531E VhZw== 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=tvQqlKwI7Kigi7O4iBI1Qk8GWzAYm63KwCklLVTbGGs=; b=gtOF/nGIzjm9xE3O0ZFufhFSONTdX3tFSdWiUAVYR9IvKnGlgucheS2sqcGOr52dvN Ft1k2TWEVL77MC3Ys6Q9PYk54r/24RAO/ZVyTuqwUjsDl1xuclCGaAtUZ7OQNzteZo6E e7a03/DyRrP/IgS9OjYaMMSHcbLX1N2vrvGk8rCQ2sMek4zKhqZYOc2RFpP8S99VUyUk znvi2R3LpiNHOnICdVMWDSampy/vL9aWBTmxOhHUHXNOCKcpWb8XIvHCaQk1TrN8bHQ8 FzICmpI5WWS0bPl7JiA6whZ6WkeN5QgbNsiS0xrSs496kCch+LmRBT+TX0Th3MNrqCkN LnYA== X-Gm-Message-State: AKGB3mLvclfOlMZAPFOLjMBOTbDWAU0aWWzbCMFcU+v56ldtWD8TGvh0 6oxNPcgDymVOMckCDKIKUqANcw9yHX8= X-Google-Smtp-Source: ACJfBos6eb7u5OD8Bi/xfV5Xmg77Tyw9QQy/copgE2uTd5Jw6etSTQ+F2W09zcXz0TBo0zW746zHxQ== X-Received: by 10.80.172.226 with SMTP id x89mr65943388edc.43.1514932138613; Tue, 02 Jan 2018 14:28:58 -0800 (PST) From: Steffan Karger To: openvpn-devel@lists.sourceforge.net Date: Tue, 2 Jan 2018 23:28:54 +0100 Message-Id: <20180102222854.29198-1-steffan@karger.me> In-Reply-To: <20180102222002.21454-1-steffan@karger.me> References: <20180102222002.21454-1-steffan@karger.me> 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.66 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: 1eWV3Q-0002bA-Tc Subject: [Openvpn-devel] [PATCH v2] 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-SA-Score: -4.8 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. Since I can't come up with anything useful to do if this check fails, and the input are not based on remote input, this patch just turns the undefined behaviour into a defined ASSERT(). And while we touch this function, make it obey the 80-char line length limit. So much for this "simple" overflow check. Trac: #922 Signed-off-by: Steffan Karger --- v2: Fix (#if 0'd) debug print format specifier for changed type. src/openvpn/integer.h | 14 ++++++++++++++ src/openvpn/interval.c | 9 ++++++--- 2 files changed, 20 insertions(+), 3 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..909e3fcd 100644 --- a/src/openvpn/interval.c +++ b/src/openvpn/interval.c @@ -51,11 +51,13 @@ 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 - dmsg(D_INTERVAL, "EVENT event_timeout_trigger (%d) etcr=%d", et->n, et_const_retry); + dmsg(D_INTERVAL, "EVENT event_timeout_trigger (%d) etcr=%d", et->n, + et_const_retry); #endif if (et_const_retry < 0) { @@ -72,7 +74,8 @@ 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", + (long) wakeup, et->n, et_const_retry); #endif tv->tv_sec = wakeup; tv->tv_usec = 0;