From patchwork Thu Oct 30 19:36:30 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 4550 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7001:2f13:b0:72f:f16c:e055 with SMTP id sa19csp1155693mab; Thu, 30 Oct 2025 12:36:50 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCVwxl9wbfmXrPJCyBWF2aaoath3ZR7Ai3547Xi/ybBn0+rH98r2YzjJZtW6YiAain0IbDMXJcNLq3A=@openvpn.net X-Google-Smtp-Source: AGHT+IHPchtPdX1z7/2qbipq6EIRLAZL9kfrRHv0ViCA4ACud4GtvpwxvqjpVpJ6WZ43MdPn73he X-Received: by 2002:a05:6e02:87:b0:432:fccd:2889 with SMTP id e9e14a558f8ab-4330d2217a8mr14006515ab.24.1761853010590; Thu, 30 Oct 2025 12:36:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1761853010; cv=none; d=google.com; s=arc-20240605; b=EIIy/KUUTgsmFnsxK6YXSIt+lWfM+p3jG1fJNb6qbZPzcLBwX120p2LjR9jEZ6qKvG SugDJ7PFrD+OT3YgomQaEK49q1NxmM1WGMTbOepfH24a3Xc/oV5cnrD8PNC7OBlCT4L8 Uf9p9wDpEnLuRzMot1icfyuS39uIR84ZCEJCBiVAIxywRS5kjO1KNTy+Gu7d/w27gApa EyKf3Z968n1d6L+eJ1u+D2VpZRR9PnXsPHymQs9DtYpj16TGYajGEfxshMdKhiw2kXYx oQR2T5rK9B6l/Duq4/++t+d9UalmhvfBoKb9gotiQSI8p8rd7UqsrCyQyixeUjlpSYep SbCA== 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=usl5bKgdKInX8F/C+SUoQ8PcA6ZEox+uWk4y5IM5t1k=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=Tdv9N885laxphGP0rSbTWSBP7412To2hMUx3mga3E2ctQYVLxgHKNkc5ViHkDYWvkD FN95f7ML0kmN+4gfzJatlT5B2TTQtquFXSs04deP+iz7rW/UrEM9szjlGKLwYAl2Jfq1 +Jpm4owqOAXj4hQ4KNZsH0MGVpIA8OIQNonS9nZsM08bwTjbUiC8S5aCPv2EPFptYwSb Q35H2E8JvCS6ZoREzEMKHPMeXQfBj8q2Z0YRsdRUm6LZ32tgZ3ka/ifnWYBzHawS+9ex JU37FQm2n6wO7NonRM4Tj75sUpW1+401lZgkGJwIec7PsVvPxTMW7k2BOIuNEEivcXGI H9iQ==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=KtNx43m0; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=D3cA9Gqq; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=leEjuhbk; 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 e9e14a558f8ab-43306cb74e9si19489295ab.96.2025.10.30.12.36.50 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 30 Oct 2025 12:36:50 -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=KtNx43m0; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=D3cA9Gqq; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=leEjuhbk; 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=usl5bKgdKInX8F/C+SUoQ8PcA6ZEox+uWk4y5IM5t1k=; b=KtNx43m0hIkOnk00bFPutbZHM2 K45SIxvsWqAUF5pGDXkwMv6KfCvbgB+WsGCK0HG1XUAzJzlIAlvlXrNtOMGJ9xYow+nsSfkl/6dxf RH1LWK0ZksQ2jqXgXPbG9yK1OHtQza1MDAjYU81/KCMowfmIsisdvOZja6MXQDWJDm2w=; Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1vEYS0-000698-Hu; Thu, 30 Oct 2025 19:36:48 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1vEYRz-000692-SX for openvpn-devel@lists.sourceforge.net; Thu, 30 Oct 2025 19:36:47 +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=FsIWMHtKd5b+ulFU5GQ1A64F3M0vR0MdgDVglBskUjA=; b=D3cA9GqqiNpEqSeAvP44fjMp7X YqRprd0iwrN/3XOZpS0k2+BH5Y5rsesnlfrrYH7DLSmu0x+JKkS394b7S39qpYwcrexs0U/6pXN8S u/B3qNB3pm7XQbExd/31+C/dzQ2LZoqJRsm2IH7uYoAkRUEEmqSvfZID6UKaTspELCAw=; 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=FsIWMHtKd5b+ulFU5GQ1A64F3M0vR0MdgDVglBskUjA=; b=leEjuhbkK9TaL9K4JoWz86dV8z nFhlqz2idd2/x5OIf3fAyzL29lSkJ58ftIsqUXbvDHr5vL6ex9B3tZuk1+B6yU2R0vquHy4PBgSFE n+f5isC2eaUUiWvNQf6ZcH9WCTg8attpYMZx76BvoW+OSxgvogAQw/hiNFtFi/8KCj+U=; Received: from [193.149.48.134] (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 1vEYRy-00016z-46 for openvpn-devel@lists.sourceforge.net; Thu, 30 Oct 2025 19:36:47 +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 59UJadiv001029 for ; Thu, 30 Oct 2025 20:36:39 +0100 Received: (from gert@localhost) by blue.greenie.muc.de (8.18.1/8.18.1/Submit) id 59UJad49001028 for openvpn-devel@lists.sourceforge.net; Thu, 30 Oct 2025 20:36:39 +0100 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Thu, 30 Oct 2025 20:36:30 +0100 Message-ID: <20251030193638.1010-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: Arne Schwabe Commit 130548fe4d change the usages of openvpn_snprintf to snprintf. When doing that conversion I did not notice that, despite the function name, openvpn_snprintf had a different semantic for the retu [...] 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: 1vEYRy-00016z-46 Subject: [Openvpn-devel] [PATCH v1] Ensure return value of snprintf is correctly checked 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?1847436782406385121?= X-GMAIL-MSGID: =?utf-8?q?1847436782406385121?= From: Arne Schwabe Commit 130548fe4d change the usages of openvpn_snprintf to snprintf. When doing that conversion I did not notice that, despite the function name, openvpn_snprintf had a different semantic for the return value. This commit goes through all the usages of snprintf and ensures that the return is correctly checked for overruns. Reported-by: Joshua Rogers Found-by: ZeroPath (https://zeropath.com/) Change-Id: I830b6b27fc3efe707e103ba629c4bfe3796a9cbe Signed-off-by: Arne Schwabe Acked-by: Gert Doering Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1330 --- 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/+/1330 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 2e328c3..89f0ab9 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -127,7 +127,7 @@ { char prefix[256]; - if (!snprintf(prefix, sizeof(prefix), "%s:%d", func, line)) + if (snprintf(prefix, sizeof(prefix), "%s:%d", func, line) >= sizeof(prefix)) { return mbed_log_err(flags, errval, func); } @@ -243,11 +243,11 @@ char header[1000 + 1] = { 0 }; char footer[1000 + 1] = { 0 }; - if (!snprintf(header, sizeof(header), "-----BEGIN %s-----\n", name)) + if (snprintf(header, sizeof(header), "-----BEGIN %s-----\n", name) >= sizeof(header)) { return false; } - if (!snprintf(footer, sizeof(footer), "-----END %s-----\n", name)) + if (snprintf(footer, sizeof(footer), "-----END %s-----\n", name) >= sizeof(footer)) { return false; } @@ -280,11 +280,11 @@ char header[1000 + 1] = { 0 }; char footer[1000 + 1] = { 0 }; - if (!snprintf(header, sizeof(header), "-----BEGIN %s-----", name)) + if (snprintf(header, sizeof(header), "-----BEGIN %s-----", name) >= sizeof(header)) { return false; } - if (!snprintf(footer, sizeof(footer), "-----END %s-----", name)) + if (snprintf(footer, sizeof(footer), "-----END %s-----", name) >= sizeof(footer)) { return false; } diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c index d2ff670..539023f 100644 --- a/src/openvpn/dns.c +++ b/src/openvpn/dns.c @@ -485,11 +485,13 @@ if (j < 0) { - name_ok = snprintf(name, sizeof(name), format, i); + const int ret = snprintf(name, sizeof(name), format, i); + name_ok = (ret > 0 && ret < sizeof(name)); } else { - name_ok = snprintf(name, sizeof(name), format, i, j); + const int ret = snprintf(name, sizeof(name), format, i, j); + name_ok = (ret > 0 && ret < sizeof(name)); } if (!name_ok) diff --git a/src/openvpn/env_set.c b/src/openvpn/env_set.c index 2ae71ab..1311e6f 100644 --- a/src/openvpn/env_set.c +++ b/src/openvpn/env_set.c @@ -334,7 +334,7 @@ strcpy(tmpname, name); while (NULL != env_set_get(es, tmpname) && counter < 1000) { - ASSERT(snprintf(tmpname, tmpname_len, "%s_%u", name, counter)); + ASSERT(snprintf(tmpname, tmpname_len, "%s_%u", name, counter) < tmpname_len); counter++; } if (counter < 1000) diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c index a1ffdaf..a612237 100644 --- a/src/openvpn/platform.c +++ b/src/openvpn/platform.c @@ -550,8 +550,9 @@ { ++attempts; - if (!snprintf(fname, sizeof(fname), fname_fmt, max_prefix_len, prefix, - (unsigned long)get_random(), (unsigned long)get_random())) + const int ret = snprintf(fname, sizeof(fname), fname_fmt, max_prefix_len, prefix, + (unsigned long)get_random(), (unsigned long)get_random()); + if (ret < 0 || ret >= sizeof(fname)) { msg(M_WARN, "ERROR: temporary filename too long"); return NULL; diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c index 790e50f..d4519b0 100644 --- a/src/openvpn/ssl_ncp.c +++ b/src/openvpn/ssl_ncp.c @@ -198,7 +198,7 @@ size_t newlen = strlen(o->ncp_ciphers) + 1 + strlen(ciphername) + 1; char *ncp_ciphers = gc_malloc(newlen, false, &o->gc); - ASSERT(snprintf(ncp_ciphers, newlen, "%s:%s", o->ncp_ciphers, ciphername)); + ASSERT(snprintf(ncp_ciphers, newlen, "%s:%s", o->ncp_ciphers, ciphername) < newlen); o->ncp_ciphers = ncp_ciphers; } diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 04ef27e..0331727 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -548,7 +548,7 @@ goto cleanup; } - if (!snprintf(fn, sizeof(fn), "%s%c%s", crl_dir, PATH_SEPARATOR, serial)) + if (snprintf(fn, sizeof(fn), "%s%c%s", crl_dir, PATH_SEPARATOR, serial) >= sizeof(fn)) { msg(D_HANDSHAKE, "VERIFY CRL: filename overflow"); goto cleanup; diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c index 472eb49..9e2aa19 100644 --- a/src/openvpn/ssl_verify_mbedtls.c +++ b/src/openvpn/ssl_verify_mbedtls.c @@ -85,8 +85,9 @@ ret = mbedtls_x509_crt_verify_info(errstr, sizeof(errstr) - 1, "", *flags); if (ret <= 0 - && !snprintf(errstr, sizeof(errstr), "Could not retrieve error string, flags=%" PRIx32, - *flags)) + && snprintf(errstr, sizeof(errstr), "Could not retrieve error string, flags=%" PRIx32, + *flags) + >= sizeof(errstr)) { errstr[0] = '\0'; } diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c index 3c11ec3..df29dd7 100644 --- a/src/openvpn/win32.c +++ b/src/openvpn/win32.c @@ -881,8 +881,9 @@ char force_path[256]; char *sysroot = get_win_sys_path(); - if (!snprintf(force_path, sizeof(force_path), "PATH=%s\\System32;%s;%s\\System32\\Wbem", - sysroot, sysroot, sysroot)) + if (snprintf(force_path, sizeof(force_path), "PATH=%s\\System32;%s;%s\\System32\\Wbem", + sysroot, sysroot, sysroot) + >= sizeof(force_path)) { msg(M_WARN, "env_block: default path truncated to %s", force_path); }