From patchwork Mon Jan 27 12:25:31 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 4084 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:c127:b0:5e7:b9eb:58e8 with SMTP id jm39csp2280160mab; Mon, 27 Jan 2025 04:25:50 -0800 (PST) X-Forwarded-Encrypted: i=2; AJvYcCXlyGsKQqCYlz2+5Fa+kUoFeudT1+1leZXx8C65fr8a9O5FJl3mjWldiTE4i42Uz9tcFEtfrXA3Gvo=@openvpn.net X-Google-Smtp-Source: AGHT+IGkDkLTV5Jw1AfN5yA6pB8FaCFnphDA+zTpnZQ6MJPCDFD0UtyspE98DZwfERmmeRjAfqiP X-Received: by 2002:a05:6808:2289:b0:3eb:3cca:8829 with SMTP id 5614622812f47-3f19fcd445emr27555329b6e.34.1737980750481; Mon, 27 Jan 2025 04:25:50 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1737980750; cv=none; d=google.com; s=arc-20240605; b=eUp07180gmb1ZdMIyCiGSVuixVBu45zte2WsMX2/4qSlajES1tO3pJnUGGV9afKIkk tSJqzP3cTQE/2zGDP/uJNh78Hz0qmTu97rxgXvE/4M3ud43A5HFJtz/KgF68SLE2OgYP iYoV9GyNszrKbaPnbEzYDkTDovAriEHmFiIH0pO1mq4gSDd9cGkAjkz8y71ks/AMe+SC ctBVP8oUj8wi4gAwa3mmiOdGmnC1E9+xw/cD+sv3g+PMAphUZJPcZYcuZBgcbcwZN9FK LWMbnijDQdqFOL1QgiTGiVH/PwqG3ihd3iGUU4UmYXM2w4AgUjFlluuXHfDsagTkjigl EQ0A== 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; bh=LfqopARVEUQUI4OdIgF8V+hC76dKcc1d+cmHvw46P4k=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=dYzZoAG+K2G/G+8WWAbGwnen/i9VNNyAdDmMZZ9goPxSuK9utDHYTit/Srfs6UGfjJ OAdCQ2cVQhtDUDZJeBxCO0gKhfChJGhTgQTvfa0i62CYQIv+1PoMBJRS9B+w2H0grekr kZT6hPueeNqbsUbqc+xv9BBjh7oEf/KaDIiiVKOJfLZwxX0DLDIkU7nZXK2YYfOzjsP/ PA65YWz9M1dI1nvmj2LX8MYvN4uT+gIDzQ+EIhBwIiqmGvNZfcMRrtVMyTFJeHVFcQSP UiOqoNWMyLc2zkRTKAA70M8n5cb64mXYyQ7q8j94+IhtGgH0SskOBQnR4U87PnJS1jra mAdQ==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=RRODjezk; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=TOP5Iywb; 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 5614622812f47-3f1f09fb1c4si5542983b6e.234.2025.01.27.04.25.50 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 27 Jan 2025 04:25:50 -0800 (PST) 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=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=RRODjezk; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=TOP5Iywb; 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 [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 1tcOBV-0007v8-VV; Mon, 27 Jan 2025 12:25:46 +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 1tcOBU-0007uz-Le for openvpn-devel@lists.sourceforge.net; Mon, 27 Jan 2025 12:25:45 +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=eaC/1vYIcezjE4o35uhr15Ies/lryKIYoxX74HgSdLo=; b=RRODjezkjB+jTB9LSCU9h88Vjp INXmFUU1e1kLZNCw4WJ6VhPJvXoxv37SCDNuZVnP40mbXXJw6Ebw+SoJZLAd5PBUnywVTOGPDpRD+ IDw2skLaDVlhNMT3/sndbXvYJGdOoo5QY1PYvAVJ2D0ktO5+rpsb13oB/alB6DTx+TdA=; 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=eaC/1vYIcezjE4o35uhr15Ies/lryKIYoxX74HgSdLo=; b=TOP5IywbE4JlVSddCMpqECpO+s v+1HX54XCZdmC0O7st9yYwEhi2Aa5OlyNJJXmYStHJ9BXhKhW54ayC9oYAajK37lQyx2JidDwY/Ho f46KLGBIqHeNjT5QcDLrY/RgANhK2VH9lglOG/rF2OcZhuky3GrGAf2KfjzxVFkPMWTE=; Received: from dhcp-174.greenie.muc.de ([193.149.48.174] 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 1tcOBT-0003fJ-9p for openvpn-devel@lists.sourceforge.net; Mon, 27 Jan 2025 12:25:45 +0000 Received: from blue.greenie.muc.de (localhost [127.0.0.1]) by blue.greenie.muc.de (8.17.1.9/8.17.1.9) with ESMTP id 50RCPV73013119 for ; Mon, 27 Jan 2025 13:25:31 +0100 Received: (from gert@localhost) by blue.greenie.muc.de (8.17.1.9/8.17.1.9/Submit) id 50RCPVNm013118 for openvpn-devel@lists.sourceforge.net; Mon, 27 Jan 2025 13:25:31 +0100 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Mon, 27 Jan 2025 13:25:31 +0100 Message-ID: <20250127122531.13105-1-gert@greenie.muc.de> X-Mailer: git-send-email 2.45.2 In-Reply-To: References: MIME-Version: 1.0 X-Spam-Score: -0.0 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-2.v13.lw.sourceforge.com", 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: Arne Schwabe Using the atoi method is a best effort method that parses as much of the input string as possible as integer and ignores the rest or return 0 if the string cannot be parsed. This is lead to unexpected [...] Content analysis details: (-0.0 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_HELO_PASS SPF: HELO matches SPF record 0.0 RCVD_IN_VALIDITY_SAFE_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [193.149.48.174 listed in sa-accredit.habeas.com] 0.0 RCVD_IN_VALIDITY_RPBL_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [193.149.48.174 listed in bl.score.senderscore.com] -0.0 SPF_PASS SPF: sender matches SPF record -0.0 T_SCC_BODY_TEXT_LINE No description available. X-Headers-End: 1tcOBT-0003fJ-9p Subject: [Openvpn-devel] [PATCH v7] Print warnings/errors when numerical parameters cannot be parsed 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?1822404903601193769?= X-GMAIL-MSGID: =?utf-8?q?1822404903601193769?= From: Arne Schwabe Using the atoi method is a best effort method that parses as much of the input string as possible as integer and ignores the rest or return 0 if the string cannot be parsed. This is lead to unexpected results. Change the behaviour by printing a warning in these cases instead. When parsing a configuration, these warnings will error out since the msglevel is M_USAGE in this case. Example: ./src/openvpn/openvpn --resolv-retry 198jj Options error: Cannot parse argument '198jj' as non-negative integer Reported-By: Anqi Chen Reported-By: Cristina Nita-Rotaru Change-Id: Ie1e2eb54d516b3ae87c5ca56fe8edd77ee2be4de Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- 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/+/873 This mail reflects revision 7 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/src/openvpn/options.c b/src/openvpn/options.c index bd5c056..4510bea 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -60,6 +60,8 @@ #include "platform.h" #include "xkey_common.h" #include "dco.h" +#include "options_util.h" + #include #include "memdbg.h" @@ -5014,13 +5016,6 @@ } #endif -static int -positive_atoi(const char *str) -{ - const int i = atoi(str); - return i < 0 ? 0 : i; -} - #ifdef _WIN32 /* This function is only used when compiling on Windows */ static unsigned int atou(const char *str) @@ -6043,7 +6038,7 @@ int cache; VERIFY_PERMISSION(OPT_P_GENERAL); - cache = atoi(p[1]); + cache = atoi_warn(p[1], msglevel); if (cache < 1) { msg(msglevel, "--management-log-cache parameter is out of range"); @@ -6355,7 +6350,7 @@ } else { - options->resolve_retry_seconds = positive_atoi(p[1]); + options->resolve_retry_seconds = positive_atoi(p[1], msglevel); } } else if ((streq(p[0], "preresolve") || streq(p[0], "ip-remote-hint")) && !p[2]) @@ -6372,7 +6367,7 @@ else if (streq(p[0], "connect-retry") && p[1] && !p[3]) { VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION); - options->ce.connect_retry_seconds = positive_atoi(p[1]); + options->ce.connect_retry_seconds = positive_atoi(p[1], msglevel); /* * Limit the base value of retry wait interval to 16 bits to avoid * overflow when scaled up for exponential backoff @@ -6387,19 +6382,19 @@ if (p[2]) { options->ce.connect_retry_seconds_max = - max_int(positive_atoi(p[2]), options->ce.connect_retry_seconds); + max_int(positive_atoi(p[2], msglevel), options->ce.connect_retry_seconds); } } else if ((streq(p[0], "connect-timeout") || streq(p[0], "server-poll-timeout")) && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION); - options->ce.connect_timeout = positive_atoi(p[1]); + options->ce.connect_timeout = positive_atoi(p[1], msglevel); } else if (streq(p[0], "connect-retry-max") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION); - options->connect_retry_max = positive_atoi(p[1]); + options->connect_retry_max = positive_atoi(p[1], msglevel); } else if (streq(p[0], "ipchange") && p[1]) { @@ -6422,7 +6417,7 @@ else if (streq(p[0], "gremlin") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL); - options->gremlin = positive_atoi(p[1]); + options->gremlin = positive_atoi(p[1], msglevel); } #endif else if (streq(p[0], "chroot") && p[1] && !p[2]) @@ -6554,7 +6549,7 @@ else if (streq(p[0], "verb") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_MESSAGES); - options->verbosity = positive_atoi(p[1]); + options->verbosity = positive_atoi(p[1], msglevel); if (options->verbosity >= (D_TLS_DEBUG_MED & M_DEBUG_LEVEL)) { /* We pass this flag to the SSL library to avoid @@ -6573,7 +6568,7 @@ else if (streq(p[0], "mute") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_MESSAGES); - options->mute = positive_atoi(p[1]); + options->mute = positive_atoi(p[1], msglevel); } else if (streq(p[0], "errors-to-stderr") && !p[1]) { @@ -6586,7 +6581,7 @@ options->status_file = p[1]; if (p[2]) { - options->status_file_update_freq = positive_atoi(p[2]); + options->status_file_update_freq = positive_atoi(p[2], msglevel); } } else if (streq(p[0], "status-version") && p[1] && !p[2]) @@ -6594,7 +6589,7 @@ int version; VERIFY_PERMISSION(OPT_P_GENERAL); - version = atoi(p[1]); + version = atoi_warn(p[1], msglevel); if (version < 1 || version > 3) { msg(msglevel, "--status-version must be 1 to 3"); @@ -6622,17 +6617,17 @@ else if ((streq(p[0], "link-mtu") || streq(p[0], "udp-mtu")) && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_MTU|OPT_P_CONNECTION); - options->ce.link_mtu = positive_atoi(p[1]); + options->ce.link_mtu = positive_atoi(p[1], msglevel); options->ce.link_mtu_defined = true; } else if (streq(p[0], "tun-mtu") && p[1] && !p[3]) { VERIFY_PERMISSION(OPT_P_PUSH_MTU|OPT_P_CONNECTION); - options->ce.tun_mtu = positive_atoi(p[1]); + options->ce.tun_mtu = positive_atoi(p[1], msglevel); options->ce.tun_mtu_defined = true; if (p[2]) { - options->ce.occ_mtu = positive_atoi(p[2]); + options->ce.occ_mtu = positive_atoi(p[2], msglevel); } else { @@ -6642,7 +6637,7 @@ else if (streq(p[0], "tun-mtu-max") && p[1] && !p[3]) { VERIFY_PERMISSION(OPT_P_MTU|OPT_P_CONNECTION); - int max_mtu = positive_atoi(p[1]); + int max_mtu = positive_atoi(p[1], msglevel); if (max_mtu < 68 || max_mtu > 65536) { msg(msglevel, "--tun-mtu-max value '%s' is invalid", p[1]); @@ -6655,13 +6650,13 @@ else if (streq(p[0], "tun-mtu-extra") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_MTU|OPT_P_CONNECTION); - options->ce.tun_mtu_extra = positive_atoi(p[1]); + options->ce.tun_mtu_extra = positive_atoi(p[1], msglevel); options->ce.tun_mtu_extra_defined = true; } else if (streq(p[0], "max-packet-size") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_MTU|OPT_P_CONNECTION); - int maxmtu = positive_atoi(p[1]); + int maxmtu = positive_atoi(p[1], msglevel); options->ce.tls_mtu = constrain_int(maxmtu, TLS_CHANNEL_MTU_MIN, TLS_CHANNEL_BUF_SIZE); if (maxmtu < TLS_CHANNEL_MTU_MIN || maxmtu > TLS_CHANNEL_BUF_SIZE) @@ -6687,7 +6682,7 @@ else if (streq(p[0], "fragment") && p[1] && !p[3]) { VERIFY_PERMISSION(OPT_P_MTU|OPT_P_CONNECTION); - options->ce.fragment = positive_atoi(p[1]); + options->ce.fragment = positive_atoi(p[1], msglevel); if (options->ce.fragment < 68) { @@ -6718,23 +6713,23 @@ else if (streq(p[0], "nice") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_NICE); - options->nice = atoi(p[1]); + options->nice = atoi_warn(p[1], msglevel); } else if (streq(p[0], "rcvbuf") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_SOCKBUF); - options->rcvbuf = positive_atoi(p[1]); + options->rcvbuf = positive_atoi(p[1], msglevel); } else if (streq(p[0], "sndbuf") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_SOCKBUF); - options->sndbuf = positive_atoi(p[1]); + options->sndbuf = positive_atoi(p[1], msglevel); } else if (streq(p[0], "mark") && p[1] && !p[2]) { #if defined(TARGET_LINUX) && HAVE_DECL_SO_MARK VERIFY_PERMISSION(OPT_P_GENERAL); - options->mark = atoi(p[1]); + options->mark = atoi_warn(p[1], msglevel); #endif } else if (streq(p[0], "socket-flags")) @@ -6764,7 +6759,7 @@ { VERIFY_PERMISSION(OPT_P_GENERAL); #ifdef TARGET_LINUX - options->tuntap_options.txqueuelen = positive_atoi(p[1]); + options->tuntap_options.txqueuelen = positive_atoi(p[1], msglevel); #else msg(msglevel, "--txqueuelen not supported on this OS"); goto err; @@ -6775,7 +6770,7 @@ int shaper; VERIFY_PERMISSION(OPT_P_SHAPER); - shaper = atoi(p[1]); + shaper = atoi_warn(p[1], msglevel); if (shaper < SHAPER_MIN || shaper > SHAPER_MAX) { msg(msglevel, "Bad shaper value, must be between %d and %d", @@ -6823,7 +6818,7 @@ else if (streq(p[0], "inactive") && p[1] && !p[3]) { VERIFY_PERMISSION(OPT_P_TIMER); - options->inactivity_timeout = positive_atoi(p[1]); + options->inactivity_timeout = positive_atoi(p[1], msglevel); if (p[2]) { int64_t val = atoll(p[2]); @@ -6841,7 +6836,7 @@ else if (streq(p[0], "session-timeout") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_TIMER); - options->session_timeout = positive_atoi(p[1]); + options->session_timeout = positive_atoi(p[1], msglevel); } else if (streq(p[0], "proto") && p[1] && !p[2]) { @@ -7008,24 +7003,24 @@ else if (streq(p[0], "keepalive") && p[1] && p[2] && !p[3]) { VERIFY_PERMISSION(OPT_P_GENERAL); - options->keepalive_ping = atoi(p[1]); - options->keepalive_timeout = atoi(p[2]); + options->keepalive_ping = atoi_warn(p[1], msglevel); + options->keepalive_timeout = atoi_warn(p[2], msglevel); } else if (streq(p[0], "ping") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_TIMER); - options->ping_send_timeout = positive_atoi(p[1]); + options->ping_send_timeout = positive_atoi(p[1], msglevel); } else if (streq(p[0], "ping-exit") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_TIMER); - options->ping_rec_timeout = positive_atoi(p[1]); + options->ping_rec_timeout = positive_atoi(p[1], msglevel); options->ping_rec_timeout_action = PING_EXIT; } else if (streq(p[0], "ping-restart") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_TIMER); - options->ping_rec_timeout = positive_atoi(p[1]); + options->ping_rec_timeout = positive_atoi(p[1], msglevel); options->ping_rec_timeout_action = PING_RESTART; } else if (streq(p[0], "ping-timer-rem") && !p[1]) @@ -7038,7 +7033,7 @@ VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION|OPT_P_EXPLICIT_NOTIFY); if (p[1]) { - options->ce.explicit_exit_notification = positive_atoi(p[1]); + options->ce.explicit_exit_notification = positive_atoi(p[1], msglevel); } else { @@ -7158,7 +7153,7 @@ else if (streq(p[0], "route-metric") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_ROUTE); - options->route_default_metric = positive_atoi(p[1]); + options->route_default_metric = positive_atoi(p[1], msglevel); } else if (streq(p[0], "route-delay") && !p[3]) { @@ -7166,10 +7161,10 @@ options->route_delay_defined = true; if (p[1]) { - options->route_delay = positive_atoi(p[1]); + options->route_delay = positive_atoi(p[1], msglevel); if (p[2]) { - options->route_delay_window = positive_atoi(p[2]); + options->route_delay_window = positive_atoi(p[2], msglevel); } } else @@ -7334,7 +7329,7 @@ } else if (streq(p[1], "SERVER_POLL_TIMEOUT") && p[2]) { - options->ce.connect_timeout = positive_atoi(p[2]); + options->ce.connect_timeout = positive_atoi(p[2], msglevel); } else { @@ -7366,14 +7361,14 @@ else if (streq(p[0], "script-security") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL); - script_security_set(atoi(p[1])); + script_security_set(atoi_warn(p[1], msglevel)); } else if (streq(p[0], "mssfix") && !p[3]) { VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION); if (p[1]) { - int mssfix = positive_atoi(p[1]); + int mssfix = positive_atoi(p[1], msglevel); /* can be 0, but otherwise it needs to be high enough so we can * substract room for headers. */ if (mssfix != 0 @@ -7556,7 +7551,7 @@ options->ifconfig_pool_persist_filename = p[1]; if (p[2]) { - options->ifconfig_pool_persist_refresh_freq = positive_atoi(p[2]); + options->ifconfig_pool_persist_refresh_freq = positive_atoi(p[2], msglevel); } } else if (streq(p[0], "ifconfig-ipv6-pool") && p[1] && !p[2]) @@ -7588,8 +7583,8 @@ int real, virtual; VERIFY_PERMISSION(OPT_P_GENERAL); - real = atoi(p[1]); - virtual = atoi(p[2]); + real = atoi_warn(p[1], msglevel); + virtual = atoi_warn(p[2], msglevel); if (real < 1 || virtual < 1) { msg(msglevel, "--hash-size sizes must be >= 1 (preferably a power of 2)"); @@ -7603,8 +7598,8 @@ int cf_max, cf_per; VERIFY_PERMISSION(OPT_P_GENERAL); - cf_max = atoi(p[1]); - cf_per = atoi(p[2]); + cf_max = atoi_warn(p[1], msglevel); + cf_per = atoi_warn(p[2], msglevel); if (cf_max < 0 || cf_per < 0) { msg(msglevel, "--connect-freq parms must be > 0"); @@ -7634,7 +7629,7 @@ int max_clients; VERIFY_PERMISSION(OPT_P_GENERAL); - max_clients = atoi(p[1]); + max_clients = atoi_warn(p[1], msglevel); if (max_clients < 0) { msg(msglevel, "--max-clients must be at least 1"); @@ -7650,7 +7645,7 @@ else if (streq(p[0], "max-routes-per-client") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_INHERIT); - options->max_routes_per_client = max_int(atoi(p[1]), 1); + options->max_routes_per_client = max_int(positive_atoi(p[1], msglevel), 1); } else if (streq(p[0], "client-cert-not-required") && !p[1]) { @@ -7734,14 +7729,14 @@ { VERIFY_PERMISSION(OPT_P_GENERAL); options->auth_token_generate = true; - options->auth_token_lifetime = p[1] ? positive_atoi(p[1]) : 0; + options->auth_token_lifetime = p[1] ? positive_atoi(p[1], msglevel) : 0; for (int i = 2; i < MAX_PARMS && p[i] != NULL; i++) { /* the second parameter can be the renewal time */ - if (i == 2 && positive_atoi(p[i])) + if (i == 2 && valid_integer(p[i], true)) { - options->auth_token_renewal = positive_atoi(p[i]); + options->auth_token_renewal = positive_atoi(p[i], msglevel); } else if (streq(p[i], "external-auth")) { @@ -7821,7 +7816,7 @@ int n_bcast_buf; VERIFY_PERMISSION(OPT_P_GENERAL); - n_bcast_buf = atoi(p[1]); + n_bcast_buf = atoi_warn(p[1], msglevel); if (n_bcast_buf < 1) { msg(msglevel, "--bcast-buffers parameter must be > 0"); @@ -7833,7 +7828,7 @@ int tcp_queue_limit; VERIFY_PERMISSION(OPT_P_GENERAL); - tcp_queue_limit = atoi(p[1]); + tcp_queue_limit = atoi_warn(p[1], msglevel); if (tcp_queue_limit < 1) { msg(msglevel, "--tcp-queue-limit parameter must be > 0"); @@ -7964,10 +7959,10 @@ int ageing_time, check_interval; VERIFY_PERMISSION(OPT_P_GENERAL); - ageing_time = atoi(p[1]); + ageing_time = atoi_warn(p[1], msglevel); if (p[2]) { - check_interval = atoi(p[2]); + check_interval = atoi_warn(p[2], msglevel); } else { @@ -7996,7 +7991,7 @@ else if (streq(p[0], "push-continuation") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_PULL_MODE); - options->push_continuation = atoi(p[1]); + options->push_continuation = atoi_warn(p[1], msglevel); } else if (streq(p[0], "auth-user-pass") && !p[2]) { @@ -8021,7 +8016,7 @@ { VERIFY_PERMISSION(OPT_P_GENERAL); options->sc_info.challenge_text = p[1]; - if (atoi(p[2])) + if (atoi_warn(p[2], msglevel)) { options->sc_info.flags |= SC_ECHO; } @@ -8117,7 +8112,7 @@ { if (!streq(p[2], "default")) { - int offset = atoi(p[2]); + int offset = atoi_warn(p[2], msglevel); if (!(offset > -256 && offset < 256)) { @@ -8133,7 +8128,7 @@ { const int min_lease = 30; int lease_time; - lease_time = atoi(p[3]); + lease_time = atoi_warn(p[3], msglevel); if (lease_time < min_lease) { msg(msglevel, "--ip-win32 dynamic [offset] [lease-time]: lease time parameter (%d) must be at least %d seconds", lease_time, min_lease); @@ -8257,7 +8252,7 @@ else if (streq(p[1], "NBT") && p[2] && !p[3]) { int t; - t = atoi(p[2]); + 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); @@ -8315,7 +8310,7 @@ #if defined(TARGET_ANDROID) else if (streq(p[1], "PROXY_HTTP") && p[3] && !p[4]) { - o->http_proxy_port = atoi(p[3]); + o->http_proxy_port = positiove_atoi(p[3], msglevel); o->http_proxy = p[2]; } #endif @@ -8349,7 +8344,7 @@ { int s; VERIFY_PERMISSION(OPT_P_DHCPDNS); - s = atoi(p[1]); + s = atoi_warn(p[1], msglevel); if (s < 0 || s >= 256) { msg(msglevel, "--tap-sleep parameter must be between 0 and 255"); @@ -8432,7 +8427,7 @@ options->exit_event_name = p[1]; if (p[2]) { - options->exit_event_initial_state = (atoi(p[2]) != 0); + options->exit_event_initial_state = (atoi_warn(p[2], msglevel) != 0); } } else if (streq(p[0], "allow-nonadmin") && !p[2]) @@ -8799,7 +8794,7 @@ { int replay_window; - replay_window = atoi(p[1]); + replay_window = atoi_warn(p[1], msglevel); if (!(MIN_SEQ_BACKTRACK <= replay_window && replay_window <= MAX_SEQ_BACKTRACK)) { msg(msglevel, "replay-window window size parameter (%d) must be between %d and %d", @@ -8814,7 +8809,7 @@ { int replay_time; - replay_time = atoi(p[2]); + replay_time = atoi_warn(p[2], msglevel); if (!(MIN_TIME_BACKTRACK <= replay_time && replay_time <= MAX_TIME_BACKTRACK)) { msg(msglevel, "replay-window time window parameter (%d) must be between %d and %d", @@ -9256,7 +9251,7 @@ else if (streq(p[0], "tls-timeout") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_TLS_PARMS); - options->tls_timeout = positive_atoi(p[1]); + options->tls_timeout = positive_atoi(p[1], msglevel); } else if (streq(p[0], "reneg-bytes") && p[1] && !p[2]) { @@ -9285,21 +9280,21 @@ else if (streq(p[0], "reneg-sec") && p[1] && !p[3]) { VERIFY_PERMISSION(OPT_P_TLS_PARMS); - options->renegotiate_seconds = positive_atoi(p[1]); + options->renegotiate_seconds = positive_atoi(p[1], msglevel); if (p[2]) { - options->renegotiate_seconds_min = positive_atoi(p[2]); + options->renegotiate_seconds_min = positive_atoi(p[2], msglevel); } } else if (streq(p[0], "hand-window") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_TLS_PARMS); - options->handshake_window = positive_atoi(p[1]); + options->handshake_window = positive_atoi(p[1], msglevel); } else if (streq(p[0], "tran-window") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_TLS_PARMS); - options->transition_window = positive_atoi(p[1]); + options->transition_window = positive_atoi(p[1], msglevel); } else if (streq(p[0], "tls-auth") && p[1] && !p[3]) { @@ -9436,7 +9431,7 @@ else if (streq(p[0], "show-pkcs11-ids") && !p[3]) { char *provider = p[1]; - bool cert_private = (p[2] == NULL ? false : ( atoi(p[2]) != 0 )); + bool cert_private = (p[2] == NULL ? false : (atoi_warn(p[2], msglevel) != 0 )); #ifdef DEFAULT_PKCS11_MODULE if (!provider) @@ -9488,7 +9483,7 @@ for (j = 1; j < MAX_PARMS && p[j] != NULL; ++j) { - options->pkcs11_protected_authentication[j-1] = atoi(p[j]) != 0 ? 1 : 0; + options->pkcs11_protected_authentication[j-1] = atoi_warn(p[j], msglevel) != 0 ? 1 : 0; } } else if (streq(p[0], "pkcs11-private-mode") && p[1]) @@ -9510,13 +9505,13 @@ for (j = 1; j < MAX_PARMS && p[j] != NULL; ++j) { - options->pkcs11_cert_private[j-1] = atoi(p[j]) != 0 ? 1 : 0; + options->pkcs11_cert_private[j-1] = (bool) (atoi_warn(p[j], msglevel)); } } else if (streq(p[0], "pkcs11-pin-cache") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL); - options->pkcs11_pin_cache_period = atoi(p[1]); + options->pkcs11_pin_cache_period = atoi_warn(p[1], msglevel); } else if (streq(p[0], "pkcs11-id") && p[1] && !p[2]) { @@ -9545,12 +9540,12 @@ { VERIFY_PERMISSION(OPT_P_PEER_ID); options->use_peer_id = true; - options->peer_id = atoi(p[1]); + options->peer_id = atoi_warn(p[1], msglevel); } #ifdef HAVE_EXPORT_KEYING_MATERIAL else if (streq(p[0], "keying-material-exporter") && p[1] && p[2]) { - int ekm_length = positive_atoi(p[2]); + int ekm_length = positive_atoi(p[2], msglevel); VERIFY_PERMISSION(OPT_P_GENERAL); @@ -9609,7 +9604,7 @@ else if (streq(p[0], "vlan-pvid") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_INSTANCE); - options->vlan_pvid = positive_atoi(p[1]); + options->vlan_pvid = positive_atoi(p[1], msglevel); if (options->vlan_pvid < OPENVPN_8021Q_MIN_VID || options->vlan_pvid > OPENVPN_8021Q_MAX_VID) { diff --git a/src/openvpn/options_util.c b/src/openvpn/options_util.c index a9e020a..fea4c3d 100644 --- a/src/openvpn/options_util.c +++ b/src/openvpn/options_util.c @@ -98,3 +98,50 @@ gc_free(&gc); return message; } + +bool +valid_integer(const char *str, bool positive) +{ + char *endptr; + long long i = strtoll(str, &endptr, 10); + + if (i < INT_MIN || (positive && i < 0) || *endptr != '\0' || i > INT_MAX) + { + return false; + } + else + { + return true; + } +} + +int +positive_atoi(const char *str, int msglevel) +{ + char *endptr; + long long i = strtoll(str, &endptr, 10); + + if (i < 0 || *endptr != '\0' || i > INT_MAX) + { + msg(msglevel, "Cannot parse argument '%s' as non-negative integer", + str); + i = 0; + } + + return (int) i; +} + +int +atoi_warn(const char *str, int msglevel) +{ + char *endptr; + long long i = strtoll(str, &endptr, 10); + + if (i < INT_MIN || *endptr != '\0' || i > INT_MAX) + { + msg(msglevel, "Cannot parse argument '%s' as integer", str); + i = 0; + } + + return (int) i; +} diff --git a/src/openvpn/options_util.h b/src/openvpn/options_util.h index 29bc9dc..f34d997 100644 --- a/src/openvpn/options_util.h +++ b/src/openvpn/options_util.h @@ -30,4 +30,24 @@ const char * parse_auth_failed_temp(struct options *o, const char *reason); -#endif + +/** Checks if the string is a valid integer by checking if it can be + * converted to an integer */ +bool +valid_integer(const char *str, bool positive); + +/** + * Converts a str to a positive number if the string represents a postive + * integer number. Otherwise print a warning with msglevel and return 0 + */ +int +positive_atoi(const char *str, int msglevel); + +/** + * 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 + */ +int +atoi_warn(const char *str, int msglevel); + +#endif /* ifndef OPTIONS_UTIL_H_ */