mirror/openvpn
0
0
Fork 0
mirror of https://github.com/OpenVPN/openvpn.git synced 2024-09-19 19:42:30 +02:00
Code

interactive.c: Improve access control for gui<->service pipe

Browse Source 
At the moment everyone but anonymous are permitted
to create a pipe with the same name as interactive service creates,
which makes it possible for malicious process with SeImpersonatePrivilege
impersonate as local user.

This hardens the security of the pipe, making it possible only for
processes running as SYSTEM (such as interactive service) create the
pipe with the same name.

While on it, replace EXPLICIT_ACCESS structures with SDDL string.

CVE: 2024-4877

Change-Id: I35e783b79a332d247606e05a39e41b4d35d39b5d
Reported by: Zeze with TeamT5 <zeze7w@gmail.com>
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20240619144629.1718-2-lev@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28808.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
This commit is contained in:
Lev Stipakov 2024-06-19 17:46:08 +03:00 committed by Gert Doering
parent 414f428fa2
commit babf312ee0
1 changed files with 33 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

89
src/openvpnserv/interactive.c
View File

@ -2142,73 +2142,50 @@ ServiceCtrlInteractive(DWORD ctrl_code, DWORD event, LPVOID data, LPVOID ctx)
static HANDLE
CreateClientPipeInstance(VOID)
{
TCHAR pipe_name[256]; /* The entire pipe name string can be up to 256 characters long according to MSDN. */
HANDLE pipe = NULL;
PACL old_dacl, new_dacl;
PSECURITY_DESCRIPTOR sd;
static EXPLICIT_ACCESS ea[2];
static BOOL initialized = FALSE;
DWORD flags = PIPE_ACCESS_DUPLEX | WRITE_DAC | FILE_FLAG_OVERLAPPED;
/*
* allow all access for local system
* deny FILE_CREATE_PIPE_INSTANCE for everyone
* allow read/write for authenticated users
* deny all access to anonymous
*/
const TCHAR *sddlString = TEXT("D:(A;OICI;GA;;;S-1-5-18)(D;OICI;0x4;;;S-1-1-0)(A;OICI;GRGW;;;S-1-5-11)(D;;GA;;;S-1-5-7)");
if (!initialized)
PSECURITY_DESCRIPTOR sd = NULL;
if (!ConvertStringSecurityDescriptorToSecurityDescriptor(sddlString, SDDL_REVISION_1, &sd, NULL))
{
PSID everyone, anonymous;
ConvertStringSidToSid(TEXT("S-1-1-0"), &everyone);
ConvertStringSidToSid(TEXT("S-1-5-7"), &anonymous);
ea[0].grfAccessPermissions = FILE_GENERIC_WRITE;
ea[0].grfAccessMode = GRANT_ACCESS;
ea[0].grfInheritance = NO_INHERITANCE;
ea[0].Trustee.pMultipleTrustee = NULL;
ea[0].Trustee.MultipleTrusteeOperation = NO_MULTIPLE_TRUSTEE;
ea[0].Trustee.TrusteeForm = TRUSTEE_IS_SID;
ea[0].Trustee.TrusteeType = TRUSTEE_IS_UNKNOWN;
ea[0].Trustee.ptstrName = (LPTSTR) everyone;
ea[1].grfAccessPermissions = 0;
ea[1].grfAccessMode = REVOKE_ACCESS;
ea[1].grfInheritance = NO_INHERITANCE;
ea[1].Trustee.pMultipleTrustee = NULL;
ea[1].Trustee.MultipleTrusteeOperation = NO_MULTIPLE_TRUSTEE;
ea[1].Trustee.TrusteeForm = TRUSTEE_IS_SID;
ea[1].Trustee.TrusteeType = TRUSTEE_IS_UNKNOWN;
ea[1].Trustee.ptstrName = (LPTSTR) anonymous;
flags |= FILE_FLAG_FIRST_PIPE_INSTANCE;
initialized = TRUE;
MsgToEventLog(M_SYSERR, TEXT("ConvertStringSecurityDescriptorToSecurityDescriptor failed."));
return INVALID_HANDLE_VALUE;
}
/* Set up SECURITY_ATTRIBUTES */
SECURITY_ATTRIBUTES sa = {0};
sa.nLength = sizeof(SECURITY_ATTRIBUTES);
sa.lpSecurityDescriptor = sd;
sa.bInheritHandle = FALSE;
DWORD flags = PIPE_ACCESS_DUPLEX | WRITE_DAC | FILE_FLAG_OVERLAPPED;
static BOOL first = TRUE;
if (first)
{
flags |= FILE_FLAG_FIRST_PIPE_INSTANCE;
first = FALSE;
}
TCHAR pipe_name[256]; /* The entire pipe name string can be up to 256 characters long according to MSDN. */
swprintf(pipe_name, _countof(pipe_name), TEXT("\\\\.\\pipe\\" PACKAGE "%ls\\service"), service_instance);
pipe = CreateNamedPipe(pipe_name, flags,
PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_REJECT_REMOTE_CLIENTS,
PIPE_UNLIMITED_INSTANCES, 1024, 1024, 0, NULL);
HANDLE pipe = CreateNamedPipe(pipe_name, flags,
PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_REJECT_REMOTE_CLIENTS,
PIPE_UNLIMITED_INSTANCES, 1024, 1024, 0, &sa);
LocalFree(sd);
if (pipe == INVALID_HANDLE_VALUE)
{
MsgToEventLog(M_SYSERR, TEXT("Could not create named pipe"));
return INVALID_HANDLE_VALUE;
}
if (GetSecurityInfo(pipe, SE_KERNEL_OBJECT, DACL_SECURITY_INFORMATION,
NULL, NULL, &old_dacl, NULL, &sd) != ERROR_SUCCESS)
{
MsgToEventLog(M_SYSERR, TEXT("Could not get pipe security info"));
return CloseHandleEx(&pipe);
}
if (SetEntriesInAcl(2, ea, old_dacl, &new_dacl) != ERROR_SUCCESS)
{
MsgToEventLog(M_SYSERR, TEXT("Could not set entries in new acl"));
return CloseHandleEx(&pipe);
}
if (SetSecurityInfo(pipe, SE_KERNEL_OBJECT, DACL_SECURITY_INFORMATION,
NULL, NULL, new_dacl, NULL) != ERROR_SUCCESS)
{
MsgToEventLog(M_SYSERR, TEXT("Could not set pipe security info"));
return CloseHandleEx(&pipe);
}
return pipe;
}