From patchwork Tue Sep 2 14:46:50 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 4389 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:2a1c:b0:671:5a2c:6455 with SMTP id k28csp1859144maz; Tue, 2 Sep 2025 07:47:17 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCV/chGrC2GJqBMPF48sHsJYhGV4VwuJkr95QOuytdp283jKR1TGbdfZLAyizSrg8geUWBvT53n/h1I=@openvpn.net X-Google-Smtp-Source: AGHT+IHThflKaOD6RkjtubGPIoPU0xpCluLiNZXOQdQrxb9wb+pBXr0TkcRY6xVGqt3fA3zY4V/P X-Received: by 2002:a05:6871:81cf:20b0:319:f60c:acf with SMTP id 586e51a60fabf-319f60c5452mr94615fac.7.1756824437182; Tue, 02 Sep 2025 07:47:17 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1756824437; cv=none; d=google.com; s=arc-20240605; b=Cy/GOXkBbv8ap3RtdYVeggkf5UjgRpT2V0HaHTuobO5+KdaV1RkCCf67UFupTl2YS/ 6CFshF5mFNMXHjn1l3r0zoMKGvhKOr7ovZp80kGEx+xyKyuvrr1nvwoySWFaVVu9SD0m qNM8l4lh5tu9yyJBsFJavbT0gL7t9LU6g5y2CKTS5AHQPZ5jTb+9m9WiBQ60QMZ/9gRm Sw+q0fVgrrBDDCcFeBJGfl8wJfzBmrNFJ5v5mcPw+PwgrP9AKZCZ18ZdLwdbVoZ6DFUF Bcr6nVHvPwbFaH0DAG8yPX7wiU3r4ZA/OFizhG+X1554kCYOomsAmCSlxJfz3+bj92bD Xd+A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; 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=jaczEW64MrnKcSFlQfbmAASXWAKMciGJ2gnu4PLOyjI=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=hnHN9ka4sNpzeJOIVD9RMI9jjTmAkU0cHyCXe5hkRXV7lGlnugoOH5CKWZVhSzsetW jmjeCZB5+30jHnp8zuKvjZs4SBiWIKBBSnDXX6BZj/wqAckk/bHTUGzS2fNu4erAXNut EVSfegSOKLDSlrmgt1hcsXWHrlNnYJ50gbwYyzikGdOUK0c4jAu1XLrNZAtJ+mM4jbi7 uleYeyDPFXrOCLmbk99uByNO7LxZhcOHi0aL4L+p2bfOEjn48k7NM93Wz6gSmkH1DWzY /GNuWb1NWx2kWr1B70YLLImc4C6G1X9mWAucSgbAifhTmJ4Nf2ADrbcT4LKQ2//4+EuD 5Z9g==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=NOhwJpVF; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=grfUhGHN; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=WDdKJxby; 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=NONE dis=NONE) header.from=muc.de Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 586e51a60fabf-319b588cb2asi432841fac.26.2025.09.02.07.47.15 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 02 Sep 2025 07:47:16 -0700 (PDT) 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=pass header.i=@lists.sourceforge.net header.s=beta header.b=NOhwJpVF; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=grfUhGHN; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=WDdKJxby; 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=NONE dis=NONE) header.from=muc.de DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.sourceforge.net; s=beta; h=Content-Transfer-Encoding:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: Subject:MIME-Version:References:In-Reply-To:Message-ID:Date:To:From:Sender: Reply-To:Cc:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=jaczEW64MrnKcSFlQfbmAASXWAKMciGJ2gnu4PLOyjI=; b=NOhwJpVF5P9K+HQ26hNmGJ6yPW IownA90C/m7jmgR4sqPiC25iYgCqOm6ERJ+ciTAP9PJ9EXuyF2xrlEJvdjAGvgBBE2VsMuFklESUo H/s0dYt5Ox6cbAhzdq7JI4RkmrKHDt9BfSDnjze+/Zgaa6fpLFpAWs0uAqTRlKOzfPIQ=; Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1utSHx-0008Le-2s; Tue, 02 Sep 2025 14:47:13 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1utSHv-0008Cj-MA for openvpn-devel@lists.sourceforge.net; Tue, 02 Sep 2025 14:47:11 +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:To:From:Sender:Reply-To:Cc: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=2mrSboTEetQdO1tlDfoM1KBz61iGVX63gP/sDGAtCeY=; b=grfUhGHNUGbzISCb8G47kv/dGi FAr6e4o232bc4FtR+CwWF7k3NAZmYdTsoZVEACYgscOQLA3hOR0i2eY96yWrSOf8XdesZFjVmH1nq 6Ln0XzzPp+T8Ftm2phynJlE3+FWLtozTMdp8vbDwLZR4yJhPIhw5qAHWJEKTfXavXobM=; 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:To:From:Sender:Reply-To:Cc: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=2mrSboTEetQdO1tlDfoM1KBz61iGVX63gP/sDGAtCeY=; b=WDdKJxby+3voreol6TJV9wrtpY VVIy1zaW3+S2ijgGno78wjQM1hZLHjQi2Bd/7YzX8i1Ef7j3d2wphBpx95N2CGirsUYInuLfDEDJ5 MHGL8Dgg3X3LdU0N3iOLMbubkq15i2bS2HPL7WhlA+scrkbUCtXOz1QihGtwNU53AsWY=; Received: from [193.149.48.143] (helo=blue.greenie.muc.de) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.95) id 1utSHp-0000bn-Sz for openvpn-devel@lists.sourceforge.net; Tue, 02 Sep 2025 14:47:07 +0000 Received: from blue.greenie.muc.de (localhost [127.0.0.1]) by blue.greenie.muc.de (8.18.1/8.18.1) with ESMTP id 582EkwvC011872 for ; Tue, 2 Sep 2025 16:46:58 +0200 Received: (from gert@localhost) by blue.greenie.muc.de (8.18.1/8.18.1/Submit) id 582EkwC5011871 for openvpn-devel@lists.sourceforge.net; Tue, 2 Sep 2025 16:46:58 +0200 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Tue, 2 Sep 2025 16:46:50 +0200 Message-ID: <20250902144657.11854-1-gert@greenie.muc.de> X-Mailer: git-send-email 2.49.1 In-Reply-To: References: MIME-Version: 1.0 X-Spam-Score: 1.3 (+) X-Spam-Report: Spam detection software, running on the system "sfi-spamd-1.hosts.colo.sdot.me", 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: Frank Lichtenheld This is a more powerful version of atoi_warn that can - check minimum and maximum values - report error seperately from parsed value This can be used to simplify a lot of option parsing. Content analysis details: (1.3 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 1.3 RDNS_NONE Delivered to internal network by a host with no rDNS X-Headers-End: 1utSHp-0000bn-Sz Subject: [Openvpn-devel] [PATCH v5] options: Introduce atoi_constrained and review usages of atoi_warn 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?1842163939737169676?= X-GMAIL-MSGID: =?utf-8?q?1842163939737169676?= From: Frank Lichtenheld This is a more powerful version of atoi_warn that can - check minimum and maximum values - report error seperately from parsed value This can be used to simplify a lot of option parsing. Change-Id: Ibc7526d59c1de17a0f9d8ed88f75c6f070ab11e7 Signed-off-by: Frank Lichtenheld Acked-by: Arne Schwabe --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1150 This mail reflects revision 5 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 74946a4..a268b92 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -6411,16 +6411,12 @@ } else if (streq(p[0], "management-log-cache") && p[1] && !p[2]) { - int cache; - VERIFY_PERMISSION(OPT_P_GENERAL); - cache = atoi_warn(p[1], msglevel); - if (cache < 1) + if (!atoi_constrained(p[1], &options->management_log_history_cache, + p[0], 1, INT_MAX, msglevel)) { - msg(msglevel, "--management-log-cache parameter is out of range"); goto err; } - options->management_log_history_cache = cache; } #endif /* ifdef ENABLE_MANAGEMENT */ #ifdef ENABLE_PLUGIN @@ -6969,16 +6965,11 @@ } else if (streq(p[0], "status-version") && p[1] && !p[2]) { - int version; - VERIFY_PERMISSION(OPT_P_GENERAL); - version = atoi_warn(p[1], msglevel); - if (version < 1 || version > 3) + if (!atoi_constrained(p[1], &options->status_file_version, p[0], 1, 3, msglevel)) { - msg(msglevel, "--status-version must be 1 to 3"); goto err; } - options->status_file_version = version; } else if (streq(p[0], "remap-usr1") && p[1] && !p[2]) { @@ -7151,16 +7142,11 @@ } else if (streq(p[0], "shaper") && p[1] && !p[2]) { - int shaper; - VERIFY_PERMISSION(OPT_P_SHAPER); - shaper = atoi_warn(p[1], msglevel); - if (shaper < SHAPER_MIN || shaper > SHAPER_MAX) + if (!atoi_constrained(p[1], &options->shaper, p[0], SHAPER_MIN, SHAPER_MAX, msglevel)) { - msg(msglevel, "Bad shaper value, must be between %d and %d", SHAPER_MIN, SHAPER_MAX); goto err; } - options->shaper = shaper; } else if (streq(p[0], "port") && p[1] && !p[2]) { @@ -7739,7 +7725,11 @@ else if (streq(p[0], "script-security") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL); - script_security_set(atoi_warn(p[1], msglevel)); + int security; + if (atoi_constrained(p[1], &security, p[0], SSEC_NONE, SSEC_PW_ENV, msglevel)) + { + script_security_set(security); + } } else if (streq(p[0], "mssfix") && !p[3]) { @@ -7959,11 +7949,9 @@ int real, virtual; VERIFY_PERMISSION(OPT_P_GENERAL); - real = atoi_warn(p[1], msglevel); - virtual = atoi_warn(p[2], msglevel); - if (real < 1 || virtual < 1) + if (!atoi_constrained(p[1], &real, "hash-size real", 1, INT_MAX, msglevel) + || !atoi_constrained(p[2], &virtual, "hash-size virtual", 1, INT_MAX, msglevel)) { - msg(msglevel, "--hash-size sizes must be >= 1 (preferably a power of 2)"); goto err; } options->real_hash_size = real; @@ -7974,11 +7962,9 @@ int cf_max, cf_per; VERIFY_PERMISSION(OPT_P_GENERAL); - cf_max = atoi_warn(p[1], msglevel); - cf_per = atoi_warn(p[2], msglevel); - if (cf_max < 0 || cf_per < 0) + if (!atoi_constrained(p[1], &cf_max, "connect-freq n", 1, INT_MAX, msglevel) + || !atoi_constrained(p[2], &cf_per, "connect-freq seconds", 1, INT_MAX, msglevel)) { - msg(msglevel, "--connect-freq parms must be > 0"); goto err; } options->cf_max = cf_max; @@ -7986,15 +7972,12 @@ } else if (streq(p[0], "connect-freq-initial") && p[1] && p[2] && !p[3]) { - long cf_max, cf_per; + int cf_max, cf_per; VERIFY_PERMISSION(OPT_P_GENERAL); - char *e1, *e2; - cf_max = strtol(p[1], &e1, 10); - cf_per = strtol(p[2], &e2, 10); - if (cf_max < 0 || cf_per < 0 || *e1 != '\0' || *e2 != '\0') + if (!atoi_constrained(p[1], &cf_max, "connect-freq-initial n", 1, INT_MAX, msglevel) + || !atoi_constrained(p[2], &cf_per, "connect-freq-initial seconds", 1, INT_MAX, msglevel)) { - msg(msglevel, "--connect-freq-initial parameters must be integers and >= 0"); goto err; } options->cf_initial_max = cf_max; @@ -8002,21 +7985,11 @@ } else if (streq(p[0], "max-clients") && p[1] && !p[2]) { - int max_clients; - VERIFY_PERMISSION(OPT_P_GENERAL); - max_clients = atoi_warn(p[1], msglevel); - if (max_clients < 0) + if (!atoi_constrained(p[1], &options->max_clients, p[0], 1, MAX_PEER_ID, msglevel)) { - msg(msglevel, "--max-clients must be at least 1"); goto err; } - if (max_clients >= MAX_PEER_ID) /* max peer-id value */ - { - msg(msglevel, "--max-clients must be less than %d", MAX_PEER_ID); - goto err; - } - options->max_clients = max_clients; } else if (streq(p[0], "max-routes-per-client") && p[1] && !p[2]) { @@ -8188,27 +8161,13 @@ } else if (streq(p[0], "bcast-buffers") && p[1] && !p[2]) { - int n_bcast_buf; - VERIFY_PERMISSION(OPT_P_GENERAL); - n_bcast_buf = atoi_warn(p[1], msglevel); - if (n_bcast_buf < 1) - { - msg(msglevel, "--bcast-buffers parameter must be > 0"); - } - options->n_bcast_buf = n_bcast_buf; + atoi_constrained(p[1], &options->n_bcast_buf, p[0], 1, INT_MAX, msglevel); } else if (streq(p[0], "tcp-queue-limit") && p[1] && !p[2]) { - int tcp_queue_limit; - VERIFY_PERMISSION(OPT_P_GENERAL); - tcp_queue_limit = atoi_warn(p[1], msglevel); - if (tcp_queue_limit < 1) - { - msg(msglevel, "--tcp-queue-limit parameter must be > 0"); - } - options->tcp_queue_limit = tcp_queue_limit; + atoi_constrained(p[1], &options->tcp_queue_limit, p[0], 1, INT_MAX, msglevel); } #if PORT_SHARE else if (streq(p[0], "port-share") && p[1] && p[2] && !p[4]) @@ -8354,21 +8313,24 @@ int ageing_time, check_interval; VERIFY_PERMISSION(OPT_P_GENERAL); - ageing_time = atoi_warn(p[1], msglevel); + if (!atoi_constrained(p[1], &ageing_time, "stale-routes-check age", 1, INT_MAX, msglevel)) + { + goto err; + } + if (p[2]) { - check_interval = atoi_warn(p[2], msglevel); + if (!atoi_constrained(p[2], &check_interval, + "stale-routes-check interval", 1, INT_MAX, msglevel)) + { + goto err; + } } else { check_interval = ageing_time; } - if (ageing_time < 1 || check_interval < 1) - { - msg(msglevel, "--stale-routes-check aging time and check interval must be >= 1"); - goto err; - } options->stale_routes_ageing_time = ageing_time; options->stale_routes_check_interval = check_interval; } @@ -8386,7 +8348,7 @@ else if (streq(p[0], "push-continuation") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_PULL_MODE); - options->push_continuation = atoi_warn(p[1], msglevel); + atoi_constrained(p[1], &options->push_continuation, p[0], 0, 2, msglevel); } else if (streq(p[0], "auth-user-pass") && !p[2]) { @@ -8505,33 +8467,23 @@ { if (!streq(p[2], "default")) { - int offset = atoi_warn(p[2], msglevel); + int offset; - if (!(offset > -256 && offset < 256)) + if (!atoi_constrained(p[2], &offset, "ip-win32 offset", -256, 256, msglevel)) { - msg(msglevel, - "--ip-win32 dynamic [offset] [lease-time]: offset (%d) must be > -256 and < 256", - offset); goto err; } - to->dhcp_masq_custom_offset = true; to->dhcp_masq_offset = offset; } if (p[3]) { - const int min_lease = 30; - int lease_time; - lease_time = atoi_warn(p[3], msglevel); - if (lease_time < min_lease) + if (!atoi_constrained(p[3], &to->dhcp_lease_time, + "ip-win32 lease time", 30, INT_MAX, msglevel)) { - msg(msglevel, - "--ip-win32 dynamic [offset] [lease-time]: lease time parameter (%d) must be at least %d seconds", - lease_time, min_lease); goto err; } - to->dhcp_lease_time = lease_time; } } } @@ -8629,8 +8581,7 @@ } else if (streq(p[1], "NBT") && p[2] && !p[3]) { - int t; - t = atoi_warn(p[2], msglevel); + int t = atoi_warn(p[2], msglevel); if (!(t == 1 || t == 2 || t == 4 || t == 8)) { msg(msglevel, "--dhcp-option NBT: parameter (%d) must be 1, 2, 4, or 8", t); @@ -8704,15 +8655,11 @@ } else if (streq(p[0], "tap-sleep") && p[1] && !p[2]) { - int s; VERIFY_PERMISSION(OPT_P_DHCPDNS); - s = atoi_warn(p[1], msglevel); - if (s < 0 || s >= 256) + if (!atoi_constrained(p[1], &options->tuntap_options.tap_sleep, p[0], 0, 255, msglevel)) { - msg(msglevel, "--tap-sleep parameter must be between 0 and 255"); goto err; } - options->tuntap_options.tap_sleep = s; } else if (streq(p[0], "dhcp-renew") && !p[1]) { @@ -9152,30 +9099,19 @@ VERIFY_PERMISSION(OPT_P_GENERAL); if (p[1]) { - int replay_window; - - replay_window = atoi_warn(p[1], msglevel); - if (!(MIN_SEQ_BACKTRACK <= replay_window && replay_window <= MAX_SEQ_BACKTRACK)) + if (!atoi_constrained(p[1], &options->replay_window, "replay-window windows size", + MIN_SEQ_BACKTRACK, MAX_SEQ_BACKTRACK, msglevel)) { - msg(msglevel, "replay-window window size parameter (%d) must be between %d and %d", - replay_window, MIN_SEQ_BACKTRACK, MAX_SEQ_BACKTRACK); goto err; } - options->replay_window = replay_window; if (p[2]) { - int replay_time; - - replay_time = atoi_warn(p[2], msglevel); - if (!(MIN_TIME_BACKTRACK <= replay_time && replay_time <= MAX_TIME_BACKTRACK)) + if (!atoi_constrained(p[2], &options->replay_time, "replay-window time window", + MIN_TIME_BACKTRACK, MAX_TIME_BACKTRACK, msglevel)) { - msg(msglevel, - "replay-window time window parameter (%d) must be between %d and %d", - replay_time, MIN_TIME_BACKTRACK, MAX_TIME_BACKTRACK); goto err; } - options->replay_time = replay_time; } } else @@ -9771,7 +9707,7 @@ else if (!p[2]) { char *endp = NULL; - int i = strtol(provider, &endp, 10); + long i = strtol(provider, &endp, 10); if (*endp == 0) { @@ -9842,7 +9778,7 @@ else if (streq(p[0], "pkcs11-pin-cache") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL); - options->pkcs11_pin_cache_period = atoi_warn(p[1], msglevel); + options->pkcs11_pin_cache_period = positive_atoi(p[1], msglevel); } else if (streq(p[0], "pkcs11-id") && p[1] && !p[2]) { diff --git a/src/openvpn/options_util.c b/src/openvpn/options_util.c index c3938a7..be1eefc 100644 --- a/src/openvpn/options_util.c +++ b/src/openvpn/options_util.c @@ -146,6 +146,37 @@ return (int)i; } +bool +atoi_constrained(const char *str, int *value, const char *name, int min, int max, int msglevel) +{ + ASSERT(min < max); + + char *endptr; + long long i = strtoll(str, &endptr, 10); + if (i < INT_MIN || *endptr != '\0' || i > INT_MAX) + { + msg(msglevel, "%s: Cannot parse '%s' as integer", name, str); + return false; + } + if (i < min || i > max) + { + if (max == INT_MAX) /* nicer message for common case */ + { + msg(msglevel, "%s: Must be an integer >= %d, not %lld", + name, min, i); + } + else + { + msg(msglevel, "%s: Must be an integer between %d and %d, not %lld", + name, min, max, i); + } + return false; + } + + *value = i; + return true; +} + static const char *updatable_options[] = { "block-ipv6", "block-outside-dns", "dhcp-option", "dns", "ifconfig", "ifconfig-ipv6", diff --git a/src/openvpn/options_util.h b/src/openvpn/options_util.h index 6f81b1e..9eca7b6 100644 --- a/src/openvpn/options_util.h +++ b/src/openvpn/options_util.h @@ -41,11 +41,23 @@ /** * Converts a str to an integer if the string can be represented as an - * integer number. Otherwise print a warning with msglevel and return 0 + * integer number. Otherwise print a warning with \p msglevel and return 0 */ int atoi_warn(const char *str, int msglevel); /** + * Converts a str to an integer if the string can be represented as an + * integer number and is between \p min and \p max. + * The integer is stored in \p value. + * On error, print a warning with \p msglevel using \p name. \p value is + * not changed on error. + * + * @return \c true if the integer has been parsed and stored in value, \c false otherwise + */ +bool atoi_constrained(const char *str, int *value, const char *name, int min, int max, + int msglevel); + +/** * Filter an option line by all pull filters. * * If a match is found, the line is modified depending on diff --git a/tests/unit_tests/openvpn/test_misc.c b/tests/unit_tests/openvpn/test_misc.c index 1857922..c3f80dc 100644 --- a/tests/unit_tests/openvpn/test_misc.c +++ b/tests/unit_tests/openvpn/test_misc.c @@ -351,6 +351,14 @@ assert_int_equal(atoi_warn("0", msglevel), 0); assert_int_equal(atoi_warn("-1194", msglevel), -1194); + int parameter = 0; + assert_true(atoi_constrained("1234", ¶meter, "test", 0, INT_MAX, msglevel)); + assert_int_equal(parameter, 1234); + assert_true(atoi_constrained("0", ¶meter, "test", -1, 0, msglevel)); + assert_int_equal(parameter, 0); + assert_true(atoi_constrained("-1194", ¶meter, "test", INT_MIN, INT_MAX, msglevel)); + assert_int_equal(parameter, -1194); + CLEAR(mock_msg_buf); assert_int_equal(positive_atoi("-1234", msglevel), 0); assert_string_equal(mock_msg_buf, "Cannot parse argument '-1234' as non-negative integer"); @@ -365,6 +373,12 @@ assert_string_equal(mock_msg_buf, "Cannot parse argument '2147483653' as integer"); CLEAR(mock_msg_buf); + parameter = -42; + assert_false(atoi_constrained("2147483653", ¶meter, "test", 0, INT_MAX, msglevel)); + assert_string_equal(mock_msg_buf, "test: Cannot parse '2147483653' as integer"); + assert_int_equal(parameter, -42); + + CLEAR(mock_msg_buf); assert_int_equal(positive_atoi("foo77", msglevel), 0); assert_string_equal(mock_msg_buf, "Cannot parse argument 'foo77' as non-negative integer"); @@ -373,6 +387,18 @@ assert_string_equal(mock_msg_buf, "Cannot parse argument '77foo' as non-negative integer"); CLEAR(mock_msg_buf); + parameter = -42; + assert_false(atoi_constrained("foo77", ¶meter, "test", 0, INT_MAX, msglevel)); + assert_string_equal(mock_msg_buf, "test: Cannot parse 'foo77' as integer"); + assert_int_equal(parameter, -42); + + CLEAR(mock_msg_buf); + parameter = -42; + assert_false(atoi_constrained("77foo", ¶meter, "test", 0, INT_MAX, msglevel)); + assert_string_equal(mock_msg_buf, "test: Cannot parse '77foo' as integer"); + assert_int_equal(parameter, -42); + + CLEAR(mock_msg_buf); assert_int_equal(atoi_warn("foo77", msglevel), 0); assert_string_equal(mock_msg_buf, "Cannot parse argument 'foo77' as integer"); @@ -380,6 +406,31 @@ assert_int_equal(atoi_warn("77foo", msglevel), 0); assert_string_equal(mock_msg_buf, "Cannot parse argument '77foo' as integer"); + /* special tests for _constrained */ + CLEAR(mock_msg_buf); + parameter = -42; + assert_false(atoi_constrained("77", ¶meter, "test", 0, 76, msglevel)); + assert_string_equal(mock_msg_buf, "test: Must be an integer between 0 and 76, not 77"); + assert_int_equal(parameter, -42); + + CLEAR(mock_msg_buf); + parameter = -42; + assert_false(atoi_constrained("-77", ¶meter, "test", -76, 76, msglevel)); + assert_string_equal(mock_msg_buf, "test: Must be an integer between -76 and 76, not -77"); + assert_int_equal(parameter, -42); + + CLEAR(mock_msg_buf); + parameter = -42; + assert_false(atoi_constrained("-77", ¶meter, "test", 0, INT_MAX, msglevel)); + assert_string_equal(mock_msg_buf, "test: Must be an integer >= 0, not -77"); + assert_int_equal(parameter, -42); + + CLEAR(mock_msg_buf); + parameter = -42; + assert_false(atoi_constrained("0", ¶meter, "test", 1, INT_MAX, msglevel)); + assert_string_equal(mock_msg_buf, "test: Must be an integer >= 1, not 0"); + assert_int_equal(parameter, -42); + mock_set_debug_level(saved_log_level); }