From 43d0ef3a5aea4e0f2f18a812c216c59b281b2a08 Mon Sep 17 00:00:00 2001 From: Selva Nair Date: Fri, 26 Feb 2016 12:21:23 -0500 Subject: [PATCH 1/3] Handle interactive service policy restrictions When a connection is attempted using a config in a location that would fail, offer an option to add the user to the "OpenVPN Administrators" group. This is done using shell-execute which will show a UAC prompt for elevation. If it fails (due to user chooses NO or the UAC dialog fails) the connection is not started. v2 Changes - Rebase to master - Automaticlaly add the admin group if it doesn't exist - Allow unicode strings in debug output - Use domain\username to identify user - Fix the PrintDebug macro Minor changes based on user feedback - Bring the window back to foreground after UAC prompt completion - Show a message if another connection is tried during authorization - Do not add user to ovpn_admin_group if it is same as the built-in admin group Signed-off-by: Selva Nair --- Makefile.am | 5 +- access.c | 312 ++++++++++++++++++++++++++++++++++++++ access.h | 29 ++++ main.c | 8 +- main.h | 9 +- misc.c | 16 ++ openvpn-gui-res.h | 4 + openvpn.c | 8 + options.c | 2 + options.h | 5 + registry.c | 6 + res/openvpn-gui-res-en.rc | 10 ++ service.c | 8 +- service.h | 2 +- 14 files changed, 413 insertions(+), 11 deletions(-) create mode 100644 access.c create mode 100644 access.h diff --git a/Makefile.am b/Makefile.am index 8b78fc1..ca13d21 100644 --- a/Makefile.am +++ b/Makefile.am @@ -88,6 +88,7 @@ openvpn_gui_SOURCES = \ misc.c misc.h \ openvpn_config.c \ openvpn_config.h \ + access.c access.h \ chartable.h \ openvpn-gui-res.h @@ -99,7 +100,9 @@ openvpn_gui_LDADD = \ -lcomctl32 \ -lwinhttp \ -lwtsapi32 \ - -lcrypt32 + -lcrypt32 \ + -lnetapi32 \ + -lsecur32 openvpn-gui-res.o: $(openvpn_gui_RESOURCES) $(srcdir)/openvpn-gui-res.h $(RCCOMPILE) -i $< -o $@ diff --git a/access.c b/access.c new file mode 100644 index 0000000..4fb6224 --- /dev/null +++ b/access.c @@ -0,0 +1,312 @@ +/* + * This file is a part of OpenVPN-GUI -- A Windows GUI for OpenVPN. + * + * Copyright (C) 2016 Selva Nair + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program (see the file COPYING included with this + * distribution); if not, write to the Free Software Foundation, Inc., + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifdef HAVE_CONFIG_H +#include +#endif + +#ifndef SECURITY_WIN32 +#define SECURITY_WIN32 +#endif + +#include +#include +#include +#include +#include +#include "main.h" +#include "options.h" +#include "service.h" +#include "localization.h" +#include "openvpn-gui-res.h" + +extern options_t o; + +#define MAX_UNAME_LEN (UNLEN + DNLEN + 2) /* UNLEN, DNLEN from lmcons.h +2 for '\' and NULL */ + +/* + * Check whether current user is a member of specified groups + */ +static BOOL +CheckGroupMember(DWORD count, WCHAR *grp[]) +{ + LOCALGROUP_USERS_INFO_0 *groups = NULL; + DWORD nread, nmax, err; + WCHAR username[MAX_UNAME_LEN]; + DWORD size; + DWORD i, j; + BOOL ret = FALSE; + + size = _countof (username); + /* get username in domain\user format */ + if (!GetUserNameExW (NameSamCompatible, username, &size)) + return FALSE; +#ifdef DEBUG + PrintDebug(L"Username: \"%s\"", username); +#endif + + /* Get an array of groups the user is member of */ + err = NetUserGetLocalGroups (NULL, username, 0, LG_INCLUDE_INDIRECT, + (LPBYTE *) &groups, MAX_PREFERRED_LENGTH, &nread, &nmax); + if (err && err != ERROR_MORE_DATA) + goto out; + + /* Check if user's groups include any of the admin groups */ + for (i = 0; i < nread; i++) + { +#ifdef DEBUG + PrintDebug(L"user in group %d: %s", i, groups[i].lgrui0_name); +#endif + for (j = 0; j < count; j++) + { + if (wcscmp (groups[i].lgrui0_name, grp[j]) == 0) + { + ret = TRUE; + break; + } + } + if (ret) + break; + } +#ifdef DEBUG + PrintDebug(L"User is %s in an authorized gtoup", ret? L"" : L"not"); +#endif + +out: + if (groups) + NetApiBufferFree (groups); + + return ret; +} + +/* + * Run a command as admin using shell execute and return the exit code. + * If the command fails to execute, the return value is (DWORD) -1. + */ +static DWORD +RunAsAdmin(const WCHAR *cmd, const WCHAR *params) +{ + SHELLEXECUTEINFO shinfo; + DWORD status = -1; + + CLEAR (shinfo); + shinfo.cbSize = sizeof(shinfo); + shinfo.fMask = SEE_MASK_NOCLOSEPROCESS; + shinfo.hwnd = NULL; + shinfo.lpVerb = L"runas"; + shinfo.lpFile = cmd; + shinfo.lpDirectory = NULL; + shinfo.nShow = SW_HIDE; + shinfo.lpParameters = params; + + if (ShellExecuteEx(&shinfo) && shinfo.hProcess) + { + WaitForSingleObject(shinfo.hProcess, INFINITE); + GetExitCodeProcess(shinfo.hProcess, &status); + CloseHandle(shinfo.hProcess); + } + return status; +} + +/* + * The Administrators group may be localized or renamed by admins. + * Get the local name of the group using the SID. + */ +static BOOL +GetBuiltinAdminGroupName (WCHAR *name, DWORD nlen) +{ + BOOL b = FALSE; + PSID admin_sid = NULL; + DWORD sid_size = SECURITY_MAX_SID_SIZE; + SID_NAME_USE su; + + WCHAR domain[MAX_NAME]; + DWORD dlen = _countof(domain); + + admin_sid = malloc(sid_size); + if (!admin_sid) + return FALSE; + + b = CreateWellKnownSid(WinBuiltinAdministratorsSid, NULL, admin_sid, + &sid_size); + if(b) + { + b = LookupAccountSidW(NULL, admin_sid, name, &nlen, domain, &dlen, &su); + } +#ifdef DEBUG + PrintDebug (L"builtin admin group name = %s", name); +#endif + + free (admin_sid); + + return b; +} +/* + * Add current user to the specified group. Uses RunAsAdmin to elevate. + */ +static BOOL +AddUserToGroup (const WCHAR *group) +{ + WCHAR username[MAX_UNAME_LEN]; + WCHAR cmd[MAX_PATH] = L"C:\\windows\\system32\\cmd.exe"; + WCHAR netcmd[MAX_PATH] = L"C:\\windows\\system32\\net.exe"; + WCHAR syspath[MAX_PATH]; + WCHAR *params = NULL; + + /* command: cmd.exe, params: /c net.exe group /add & net.exe group user /add */ + const WCHAR *fmt = L"/c %s localgroup \"%s\" /add & %s localgroup \"%s\" \"%s\" /add"; + DWORD size; + DWORD status; + BOOL retval = FALSE; + + size = _countof(username); + if (!GetUserNameExW (NameSamCompatible, username, &size)) + return retval; + + size = _countof(syspath); + if (GetSystemDirectory (syspath, size)) + { + syspath[size-1] = L'\0'; + size = _countof(cmd); + _snwprintf(cmd, size, L"%s\\%s", syspath, L"cmd.exe"); + cmd[size-1] = L'\0'; + size = _countof(netcmd); + _snwprintf(netcmd, size, L"%s\\%s", syspath, L"net.exe"); + netcmd[size-1] = L'\0'; + } + size = (wcslen(fmt) + wcslen(username) + 2*wcslen(group) + 2*wcslen(netcmd)+ 1); + if ((params = malloc (size*sizeof(WCHAR))) == NULL) + return retval; + + _snwprintf(params, size, fmt, netcmd, group, netcmd, group, username); + params[size-1] = L'\0'; + + status = RunAsAdmin (cmd, params); + if (status == 0) + retval = TRUE; + +#ifdef DEBUG + if (status == (DWORD) -1) + PrintDebug(L"RunAsAdmin: failed to execute the command [%s %s] : error = 0x%x", + cmd, params, GetLastError()); + else if (status) + PrintDebug(L"RunAsAdmin: command [%s %s] returned exit_code = %lu", + cmd, params, status); +#endif + + free (params); + return retval; +} + +/* + * Check whether the config location is authorized for startup through + * interactive service. Either the user be a member of the specified groups, + * or the config_dir be inside the global_config_dir + */ +static BOOL +CheckConfigPath (const WCHAR *config_dir, DWORD ngrp, WCHAR *admin_group[]) +{ + BOOL ret = FALSE; + int size = wcslen(o.global_config_dir); + + /* if interactive service is not running, no access control: return TRUE */ + if (!CheckIServiceStatus(FALSE)) + ret = TRUE; + + /* if config is from the global location allow it */ + else if ( wcsncmp(config_dir, o.global_config_dir, size) == 0 && + wcsstr(config_dir + size, L"..") == NULL + ) + ret = TRUE; + + /* check user is in an authorized group */ + else + { + if (CheckGroupMember(ngrp, admin_group)) + ret = TRUE; + } + + return ret; +} + +/* + * If config_dir for a connection is not in an authorized location, + * show a dialog to add the user to the ovpn_admin_group. + */ +BOOL +AuthorizeConfig(const connection_t *c) +{ + DWORD res; + BOOL retval = FALSE; + WCHAR *admin_group[2]; + WCHAR sysadmin_group[MAX_NAME]; + + if (GetBuiltinAdminGroupName(sysadmin_group, _countof(sysadmin_group))) + admin_group[0] = sysadmin_group; + else + admin_group[0] = L"Administrators"; + + /* assuming TCHAR == WCHAR as we do not support non-unicode build */ + admin_group[1] = o.ovpn_admin_group; + +#ifdef DEBUG + PrintDebug (L"admin groups: %s, %s", admin_group[0], admin_group[1]); +#endif + + if (CheckConfigPath(c->config_dir, 2, admin_group)) + return TRUE; + /* do not attempt to add user to sysadmin_group or a no-name group */ + else if ( wcscmp(sysadmin_group, o.ovpn_admin_group) == 0 || + wcslen(o.ovpn_admin_group) == 0 || + !o.netcmd_semaphore ) + { + ShowLocalizedMsg(IDS_ERR_CONFIG_NOT_AUTHORIZED, c->config_name, sysadmin_group); + return FALSE; + } + + if (WaitForSingleObject (o.netcmd_semaphore, 0) != WAIT_OBJECT_0) + { + /* Could not lock semaphore -- auth dialog already running? */ + ShowLocalizedMsg(IDS_NFO_CONFIG_AUTH_PENDING, c->config_name, admin_group[1]); + return FALSE; + } + + /* semaphore locked -- relase before return */ + + res = ShowLocalizedMsgEx(MB_YESNO|MB_ICONWARNING, TEXT(PACKAGE_NAME), + IDS_ERR_CONFIG_TRY_AUTHORIZE, c->config_name, + o.ovpn_admin_group); + if (res == IDYES) + { + AddUserToGroup (o.ovpn_admin_group); + /* + * Check the success of above by testing the config path again. + */ + if (CheckConfigPath(c->config_dir, 2, admin_group)) + retval = TRUE; + else + ShowLocalizedMsg(IDS_ERR_ADD_USER_TO_ADMIN_GROUP, o.ovpn_admin_group); + SetForegroundWindow (o.hWnd); + } + ReleaseSemaphore (o.netcmd_semaphore, 1, NULL); + + return retval; +} diff --git a/access.h b/access.h new file mode 100644 index 0000000..51cb2fd --- /dev/null +++ b/access.h @@ -0,0 +1,29 @@ +/* + * This file is a part of OpenVPN-GUI -- A Windows GUI for OpenVPN. + * + * Copyright (C) 2016 Selva Nair + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program (see the file COPYING included with this + * distribution); if not, write to the Free Software Foundation, Inc., + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef ACCESS_H +#define ACCESS_H + +#include "options.h" + +BOOL AuthorizeConfig (const connection_t *c); + +#endif diff --git a/main.c b/main.c index 3177cde..5cc3911 100644 --- a/main.c +++ b/main.c @@ -23,6 +23,10 @@ #include #endif +#if !defined (UNICODE) +#error UNICODE and _UNICODE must be defined. This version only supports unicode builds. +#endif + #include #include #include @@ -120,7 +124,7 @@ int WINAPI _tWinMain (HINSTANCE hThisInstance, #ifdef DEBUG /* Open debug file for output */ - if (!(o.debug_fp = fopen(DEBUG_FILE, "w"))) + if (!(o.debug_fp = _wfopen(DEBUG_FILE, L"a+,ccs=UTF-8"))) { /* can't open debug file */ ShowLocalizedMsg(IDS_ERR_OPEN_DEBUG_FILE, DEBUG_FILE); @@ -183,7 +187,7 @@ int WINAPI _tWinMain (HINSTANCE hThisInstance, } if (!IsUserAdmin()) - CheckIServiceStatus(); + CheckIServiceStatus(TRUE); BuildFileList(); if (!VerifyAutoConnections()) { diff --git a/main.h b/main.h index 091e563..a75f193 100644 --- a/main.h +++ b/main.h @@ -29,7 +29,7 @@ /* Define this to enable DEBUG build */ //#define DEBUG -#define DEBUG_FILE "c:\\openvpngui_debug.txt" +#define DEBUG_FILE L"C:\\windows\\temp\\openvpngui_debug.txt" /* Define this to disable Change Password support */ //#define DISABLE_CHANGE_PASSWORD @@ -44,7 +44,8 @@ #define MAX_LOG_LINES 500 /* Max number of lines in LogWindow */ #define DEL_LOG_LINES 10 /* Number of lines to delete from LogWindow */ - +/* Authorized group who can use any options and config locations */ +#define OVPN_ADMIN_GROUP TEXT("OpenVPN Administrators") /* May be reset in registry */ /* bool definitions */ #define bool int @@ -103,11 +104,11 @@ __snprintf_0(char *buf, size_t size, char *format, ...) #ifdef DEBUG /* Print Debug Message */ #define PrintDebug(...) \ - { \ + do { \ TCHAR x_msg[256]; \ _sntprintf_0(x_msg, __VA_ARGS__); \ PrintDebugMsg(x_msg); \ - } + } while(0) void PrintDebugMsg(TCHAR *msg); #endif diff --git a/misc.c b/misc.c index 0777d03..5b8596c 100644 --- a/misc.c +++ b/misc.c @@ -32,6 +32,7 @@ #include "options.h" #include "manage.h" +#include "main.h" #include "misc.h" /* @@ -322,3 +323,18 @@ BOOL IsUserAdmin(VOID) return(b); } + +HANDLE +InitSemaphore (void) +{ + HANDLE semaphore = NULL; + semaphore = CreateSemaphore (NULL, 1, 1, NULL); + if (!semaphore) + { + MessageBoxW (NULL, L"PACKAGE_NAME", L"Error creating semaphore", MB_OK); +#ifdef DEBUG + PrintDebug (L"InitSemaphore: CreateSemaphore failed [error = %lu]", GetLastError()); +#endif + } + return semaphore; +} diff --git a/openvpn-gui-res.h b/openvpn-gui-res.h index 633d621..e163010 100644 --- a/openvpn-gui-res.h +++ b/openvpn-gui-res.h @@ -164,6 +164,10 @@ #define IDS_ERR_CONFIG_EXIST 1251 #define IDS_NFO_CONN_TIMEOUT 1252 #define IDS_NFO_NO_CONFIGS 1253 +#define IDS_ERR_CONFIG_NOT_AUTHORIZED 1254 +#define IDS_ERR_CONFIG_TRY_AUTHORIZE 1255 +#define IDS_NFO_CONFIG_AUTH_PENDING 1256 +#define IDS_ERR_ADD_USER_TO_ADMIN_GROUP 1257 /* Program Startup Related */ #define IDS_ERR_OPEN_DEBUG_FILE 1301 diff --git a/openvpn.c b/openvpn.c index e7da90d..2903b58 100644 --- a/openvpn.c +++ b/openvpn.c @@ -44,6 +44,7 @@ #include "passphrase.h" #include "localization.h" #include "misc.h" +#include "access.h" #define WM_OVPN_STOP (WM_APP + 10) #define WM_OVPN_SUSPEND (WM_APP + 11) @@ -747,6 +748,13 @@ StartOpenVPN(connection_t *c) DWORD size = _tcslen(c->config_dir) + _tcslen(options) + sizeof(c->manage.password) + 3; TCHAR startup_info[1024]; DWORD dwMode = PIPE_READMODE_MESSAGE; + + if ( !AuthorizeConfig(c)) + { + CloseHandle(c->exit_event); + goto out; + } + if (!SetNamedPipeHandleState(service, &dwMode, NULL, NULL)) { ShowLocalizedMsg (IDS_ERR_ACCESS_SERVICE_PIPE); diff --git a/options.c b/options.c index 59ab408..8325dc3 100644 --- a/options.c +++ b/options.c @@ -34,6 +34,7 @@ #include "main.h" #include "openvpn-gui-res.h" #include "localization.h" +#include "misc.h" #define streq(x, y) (_tcscmp((x), (y)) == 0) @@ -210,6 +211,7 @@ void InitOptions(options_t *opt) { CLEAR(*opt); + opt->netcmd_semaphore = InitSemaphore (); } diff --git a/options.h b/options.h index f8dda93..484e1b9 100644 --- a/options.h +++ b/options.h @@ -29,9 +29,12 @@ typedef struct connection connection_t; #include #include #include +#include #include "manage.h" +#define MAX_NAME (UNLEN + 1) + /* * Maximum number of parameters associated with an option, * including the option name itself. @@ -145,6 +148,7 @@ typedef struct { TCHAR connectscript_timeout_string[4]; TCHAR disconnectscript_timeout_string[4]; TCHAR preconnectscript_timeout_string[4]; + TCHAR ovpn_admin_group[MAX_NAME]; #ifdef DEBUG FILE *debug_fp; @@ -153,6 +157,7 @@ typedef struct { HWND hWnd; HINSTANCE hInstance; BOOL session_locked; + HANDLE netcmd_semaphore; } options_t; void InitOptions(options_t *); diff --git a/registry.c b/registry.c index e4dac2d..5802f41 100644 --- a/registry.c +++ b/registry.c @@ -68,6 +68,7 @@ GetRegistryKeys() { /* error reading registry value */ ShowLocalizedMsg(IDS_ERR_READING_REGISTRY); + RegCloseKey(regkey); return(false); } if (openvpn_path[_tcslen(openvpn_path) - 1] != _T('\\')) @@ -79,6 +80,11 @@ GetRegistryKeys() /* use default = openvpnpath\config */ _sntprintf_0(o.global_config_dir, _T("%sconfig"), openvpn_path); } + if (!GetRegistryValue(regkey, _T("ovpn_admin_group"), o.ovpn_admin_group, _countof(o.ovpn_admin_group))) + { + _tcsncpy(o.ovpn_admin_group, OVPN_ADMIN_GROUP, _countof(o.ovpn_admin_group)); + } + RegCloseKey(regkey); /* config_dir in user's profile by default */ _sntprintf_0(temp_path, _T("%s\\OpenVPN\\config"), profile_dir); diff --git a/res/openvpn-gui-res-en.rc b/res/openvpn-gui-res-en.rc index 58f2df2..a35f3a5 100644 --- a/res/openvpn-gui-res-en.rc +++ b/res/openvpn-gui-res-en.rc @@ -202,6 +202,16 @@ BEGIN /* OpenVPN */ IDS_ERR_MANY_CONFIGS "OpenVPN GUI does not support more than %d configs. Please contact the author if you have the need for more." IDS_NFO_NO_CONFIGS "No readable connection profiles (config files) found" + IDS_ERR_CONFIG_NOT_AUTHORIZED "Starting this connection (%s) requires membership in\n"\ + """%s"" group. Contact your system administrator.\n" + IDS_ERR_CONFIG_TRY_AUTHORIZE "Starting this connection (%s) requires membership in\n"\ + """%s"" group.\n\n"\ + "Do you want to add yourself to this group?\n"\ + "This action may prompt for administrtaive password or consent." + IDS_NFO_CONFIG_AUTH_PENDING "Starting this connection (%s) requires membership in\n"\ + """%s"" group.\n\n"\ + "Please complete the previous authorization dialog." + IDS_ERR_ADD_USER_TO_ADMIN_GROUP "Adding the user to ""%s"" group failed." IDS_ERR_ONE_CONN_OLD_VER "You can only have one connection running at the same time when using an older version on OpenVPN than 2.0-beta6." IDS_ERR_STOP_SERV_OLD_VER "You cannot use OpenVPN GUI to start a connection while the OpenVPN Service is running (with OpenVPN 1.5/1.6). Stop OpenVPN Service first if you want to use OpenVPN GUI." IDS_ERR_CREATE_EVENT "CreateEvent failed on exit event: %s" diff --git a/service.c b/service.c index fc7367f..66cca2d 100644 --- a/service.c +++ b/service.c @@ -242,7 +242,7 @@ int MyReStartService() } bool -CheckIServiceStatus() +CheckIServiceStatus(BOOL warn) { SC_HANDLE schSCManager; SC_HANDLE schService; @@ -260,7 +260,8 @@ CheckIServiceStatus() GetLastError() == ERROR_SERVICE_DOES_NOT_EXIST) { /* warn that iservice is not installed */ - ShowLocalizedMsg(IDS_ERR_INSTALL_ISERVICE); + if (warn) + ShowLocalizedMsg(IDS_ERR_INSTALL_ISERVICE); return(false); } @@ -270,7 +271,8 @@ CheckIServiceStatus() if (ssStatus.dwCurrentState != SERVICE_RUNNING) { /* warn that iservice is not started */ - ShowLocalizedMsg(IDS_ERR_NOTSTARTED_ISERVICE); + if (warn) + ShowLocalizedMsg(IDS_ERR_NOTSTARTED_ISERVICE); return(false); } return true; diff --git a/service.h b/service.h index dba8c83..85ac59a 100644 --- a/service.h +++ b/service.h @@ -23,4 +23,4 @@ int MyStartService(); int MyStopService(); int MyReStartService(); int CheckServiceStatus(); -BOOL CheckIServiceStatus(); +BOOL CheckIServiceStatus(BOOL warn); From c8ddab1f90fd86541c1116c43d1a29f523dced8b Mon Sep 17 00:00:00 2001 From: Selva Nair Date: Wed, 23 Mar 2016 00:10:36 -0400 Subject: [PATCH 2/3] Add InitSemaphore() to misc.h --- misc.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/misc.h b/misc.h index f463a6a..a48dba7 100644 --- a/misc.h +++ b/misc.h @@ -33,4 +33,6 @@ BOOL wcsbegins(LPCWSTR, LPCWSTR); BOOL ForceForegroundWindow(HWND); BOOL IsUserAdmin(VOID); +HANDLE InitSemaphore (void); + #endif From 6d9ab8122c9b9ba4d4f5f1946a3b606988ddc306 Mon Sep 17 00:00:00 2001 From: Selva Nair Date: Sun, 27 Mar 2016 23:56:24 -0400 Subject: [PATCH 3/3] Ensure group name in shell-execute cmdline is clean - Also fix typo in a comment. Signed-off-by: Selva Nair --- access.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/access.c b/access.c index 4fb6224..085dfe5 100644 --- a/access.c +++ b/access.c @@ -87,7 +87,7 @@ CheckGroupMember(DWORD count, WCHAR *grp[]) break; } #ifdef DEBUG - PrintDebug(L"User is %s in an authorized gtoup", ret? L"" : L"not"); + PrintDebug(L"User is%s in an authorized group", ret? L"" : L" not"); #endif out: @@ -161,6 +161,7 @@ GetBuiltinAdminGroupName (WCHAR *name, DWORD nlen) } /* * Add current user to the specified group. Uses RunAsAdmin to elevate. + * Reject if the group name contains certain illegal characters. */ static BOOL AddUserToGroup (const WCHAR *group) @@ -176,6 +177,20 @@ AddUserToGroup (const WCHAR *group) DWORD size; DWORD status; BOOL retval = FALSE; + WCHAR reject[] = L"\"\?\\/[]:;|=,+*<>\'&"; + + /* + * The only unknown content in the command line is the variable group. Ensure it + * does not contain any '"' character. Here we reject all characters not allowed + * in group names and special characters such as '&' as well. + */ + if (wcspbrk(group, reject) != NULL) + { +#ifdef DEBUG + PrintDebug (L"AddUSerToGroup: illegal characters in group name: '%s'.", group); +#endif + return retval; + } size = _countof(username); if (!GetUserNameExW (NameSamCompatible, username, &size))