From patchwork Wed Nov 12 09:22:38 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gert Doering X-Patchwork-Id: 4585 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:33c4:b0:7b1:439f:bdf with SMTP id u4csp2437631maf; Wed, 12 Nov 2025 01:22:57 -0800 (PST) X-Forwarded-Encrypted: i=2; AJvYcCVR2TTYrRYGyJMBrhC7/3lN4iq/PJVcD9TPL1gnnlmT/rSg2LXwFAIl/V7q2zNP3m5Tv2oiutBfQVQ=@openvpn.net X-Google-Smtp-Source: AGHT+IGO43jX2YtuPnH+nGYoI4nKxLBNDdEvxYKERrPPpNOBr8CHKG1SCtY0dCXBZc3gS+Ciw0ml X-Received: by 2002:a05:6808:1c0e:b0:44d:b300:e903 with SMTP id 5614622812f47-45074597134mr843149b6e.54.1762939376911; Wed, 12 Nov 2025 01:22:56 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1762939376; cv=none; d=google.com; s=arc-20240605; b=bq/kB38AGVi3rT0L0hS6x8I0vNJgnH6Zkr4ZLcuXVCiDFlODCP5ZaG8vN9XULBCS0T 2MQfctz/d/yO1ddUQ1DmT8mZHC2FeYCmspnyXKtFEqsYcdhetfTShm0mBWo+bZvQRnww LT0A4vhByspX2d/7130d8rxiGNvfZBPeA94OpaJ7XZeHLbqzU/vd8etfke7Hj378iOcB mWe56Ru9bNFFjK1RbghBfPzPplIv5fYRhkyULKGJwF7AeEs6cQ/NSjyfopNRXC/IrBRb U5+9Sfdu/Gkv7kdTtKdOI1CZwEwAO5ekBTvsf7g5stTnNl0aoAPJm9z4oGumJ38CMv9J sWlg== 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=/0KGg9weAuO8WHtIrkg5f2TcO+Z3B4i8jKraOOcDzaA=; fh=4NbAC/LsuMLI0S0hprUlLSLCiHwg6SCAifhH718Jh0Q=; b=Y0JQlByppZRf36kjIN4An9vbqYuz5yIBVAggYxTmR1cHFVme2Pbcq3u+FY6zx+b+SW 4eonj/wVQne4WwL67ZzuMQeaD6isKNYSGJIbsqmSq1KJhF5Ax0QQ75a9n1NhI1uOfqqJ wNsg3fRGZVpSwcQ0G1bG/4mO1xZ6yyfURlBQAxLmIsTQp9wjRzomShjFVbtnmhiLzsst 8glNfuxLa1izIwagCx9NaFxo8IWLVZudNWUfCFENYdF3hQDKXYS+cVENEpg+qLwFoQkK Q/d6zvjEV68XuPQYAMHjno6ang4b8nlMSUBJ73LSMjXC5WkhC6iyc37FZbeF8z7R4aAr 2VVw==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lists.sourceforge.net header.s=beta header.b=RqMdvWkD; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=ApMD3Q0p; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=lT0AwsAw; 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 006d021491bc7-656c57841e6si6238042eaf.135.2025.11.12.01.22.56 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 12 Nov 2025 01:22:56 -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=pass header.i=@lists.sourceforge.net header.s=beta header.b=RqMdvWkD; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=ApMD3Q0p; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=lT0AwsAw; 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=/0KGg9weAuO8WHtIrkg5f2TcO+Z3B4i8jKraOOcDzaA=; b=RqMdvWkDx8tLW5rHBLt1dt1hYZ DuA1KD2FZNiI19gVDPqXQ5bZswmVxBLSJNh7ddTKHiOc6PwmamVhKZ7xajF7FiYA2viQgeDxHyHGn YYwVBsFRbgYgUC0HQ8vCQNtfshgsrOy0XfkOD9ZvllePI8lo77WK27zBHzJk4BGHScw4=; Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1vJ740-0005gQ-9D; Wed, 12 Nov 2025 09:22:53 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1vJ73y-0005gD-VO for openvpn-devel@lists.sourceforge.net; Wed, 12 Nov 2025 09:22:51 +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=dF8XbP3cZJ9/VAUP3ipia1yS+rOXTugbzM0yXDNuWuo=; b=ApMD3Q0pxCh+4DvntD3Ne8Oy40 KESavyed27woC64goTWuFAegROo2hW9Bgn1rh1bHgJQu+2wXM9F9dK/ceMl+KweHejmGi5cntLRZt c2SsBCQqvtWHv/HpFhwP2ulRw/q20ar0AN2rrL7xCpJWNJNbC/tQ4JY7vlBKen8Clj1w=; 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=dF8XbP3cZJ9/VAUP3ipia1yS+rOXTugbzM0yXDNuWuo=; b=lT0AwsAwD5ZRAI/U0Tb+3Z35oF iPpjzRPhHHA/EaAEyeUjKQY4p22oDX/GABX+urITvDnFIGezMIK3fEDYXKj1MgjJpQ5AczNvnA2lz cpTYaOGkY2NpeE+MAWF+ajPXrFEi5IdS7zmvSkVk34AKcXwYz3UbiJm7juObVW+6dcgs=; 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 1vJ73y-0008Lt-T0 for openvpn-devel@lists.sourceforge.net; Wed, 12 Nov 2025 09:22:51 +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 5AC9MiEd022778 for ; Wed, 12 Nov 2025 10:22:44 +0100 Received: (from gert@localhost) by blue.greenie.muc.de (8.18.1/8.18.1/Submit) id 5AC9MicL022777 for openvpn-devel@lists.sourceforge.net; Wed, 12 Nov 2025 10:22:44 +0100 From: Gert Doering To: openvpn-devel@lists.sourceforge.net Date: Wed, 12 Nov 2025 10:22:38 +0100 Message-ID: <20251112092244.22764-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-2.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: Heiko Hund Instead of just rejecting any path that contains ".." use some WIN32 API functions to combine, canonicalize and then check if the resulting path is located under the config directory. Makes the code p [...] 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: 1vJ73y-0008Lt-T0 Subject: [Openvpn-devel] [PATCH v7] iservice: validate config path better 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?1848575919813149755?= X-GMAIL-MSGID: =?utf-8?q?1848575919813149755?= From: Heiko Hund Instead of just rejecting any path that contains ".." use some WIN32 API functions to combine, canonicalize and then check if the resulting path is located under the config directory. Makes the code prettier and more correct. Change-Id: I0e94068f467f2899daf133b032a785d2d7fc05e4 Signed-off-by: Heiko Hund Acked-by: Lev Stipakov Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1307 --- 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/+/1307 This mail reflects revision 7 of this Change. Acked-by according to Gerrit (reflected above): Lev Stipakov diff --git a/src/openvpnserv/validate.c b/src/openvpnserv/validate.c index ddaa381..2dcfe1a 100644 --- a/src/openvpnserv/validate.c +++ b/src/openvpnserv/validate.c @@ -24,8 +24,8 @@ #include #include -#include #include +#include #ifndef HAVE_PATHCCH_ENSURE_TRAILING_SLASH #define PATHCCH_ENSURE_TRAILING_SLASH 0x20 @@ -58,44 +58,31 @@ static PTOKEN_GROUPS GetTokenGroups(const HANDLE token); /* - * Check workdir\fname is inside config_dir - * The logic here is simple: we may reject some valid paths if ..\ is in any of the strings + * Check that config path is inside config_dir + * The logic here is simple: if the path isn't prefixed with config_dir it's rejected */ static BOOL CheckConfigPath(const WCHAR *workdir, const WCHAR *fname, const settings_t *s) { - WCHAR tmp[MAX_PATH]; - const WCHAR *config_file = NULL; - WCHAR config_dir[MAX_PATH]; + HRESULT res; + WCHAR config_path[MAX_PATH]; /* fname = stdin is special: do not treat it as a relative path */ if (wcscmp(fname, L"stdin") == 0) { return FALSE; } - /* convert fname to full path */ + /* convert fname to full canonical path */ if (PathIsRelativeW(fname)) { - swprintf(tmp, _countof(tmp), L"%ls\\%ls", workdir, fname); - config_file = tmp; + res = PathCchCombine(config_path, _countof(config_path), workdir, fname); } else { - config_file = fname; + res = PathCchCanonicalize(config_path, _countof(config_path), fname); } - /* canonicalize config_dir and add trailing slash before comparison */ - HRESULT res = PathCchCanonicalizeEx(config_dir, _countof(config_dir), s->config_dir, - PATHCCH_ENSURE_TRAILING_SLASH); - - if (res == S_OK - && wcsncmp(config_dir, config_file, wcslen(config_dir)) == 0 - && wcsstr(config_file + wcslen(config_dir), L"..") == NULL) - { - return TRUE; - } - - return FALSE; + return res == S_OK && wcsncmp(config_path, s->config_dir, wcslen(s->config_dir)) == 0; }