Merge pull request #162 from selvanair/close-handles

-Close token handle in GetProcessTokenGroups()
-Close open service handles after use
-Close registry keys after use

Acked-by: Gert Doering <gert@greenie.muc.de>

access.c

@@ -324,7 +324,7 @@ GetProcessTokenGroups(void)
     if (!groups)
     {
         PrintDebug(L"GetProcessTokenGroups: error = %lu", GetLastError());
-        return NULL;
+        goto out;
     }
     if (!GetTokenInformation(token, TokenGroups, groups, buf_size, &buf_size))
     {
@@ -332,6 +332,9 @@ GetProcessTokenGroups(void)
         free (groups);
         groups = NULL;
     }
+
+out:
+    CloseHandle(token);
     return groups;
 }
 

registry.c

@@ -174,7 +174,7 @@ GetRegistryKeys ()
             PrintDebug(L"from registry: %s = %lu", regkey_int[i].name, *regkey_int[i].var);
     }
 
-    if ( status != ERROR_SUCCESS)
+    if ( status == ERROR_SUCCESS)
         RegCloseKey (regkey);
 
     if ((o.disconnectscript_timeout == 0))
@@ -198,13 +198,14 @@ SaveRegistryKeys ()
     HKEY regkey;
     DWORD status;
     int i;
+    int ret = false;
 
     status = RegCreateKeyEx(HKEY_CURRENT_USER, GUI_REGKEY_HKCU, 0, NULL, REG_OPTION_NON_VOLATILE,
                             KEY_WRITE|KEY_READ, NULL, &regkey, NULL);
     if (status != ERROR_SUCCESS)
     {
         ShowLocalizedMsg (IDS_ERR_CREATE_REG_HKCU_KEY, GUI_REGKEY_HKCU);
-        return false;
+        goto out;
     }
     for (i = 0 ; i < (int) _countof (regkey_str); ++i)
     {
@@ -213,7 +214,7 @@ SaveRegistryKeys ()
              RegValueExists(regkey, regkey_str[i].name) )
         {
             if (!SetRegistryValue (regkey, regkey_str[i].name, regkey_str[i].var))
-                return false;
+                goto out;
         }
     }
 
@@ -223,13 +224,16 @@ SaveRegistryKeys ()
              RegValueExists(regkey, regkey_int[i].name) )
         {
             if (!SetRegistryValueNumeric (regkey, regkey_int[i].name, *regkey_int[i].var))
-                return false;
+                goto out;
         }
     }
+    ret = true;
 
-    RegCloseKey (regkey);
+out:
 
-    return true;
+    if (status == ERROR_SUCCESS)
+        RegCloseKey(regkey);
+    return ret;
 }
 
 static BOOL
@@ -260,8 +264,8 @@ SetRegistryVersion (const version_t *v)
                             KEY_WRITE, NULL, &regkey, NULL);
     if (status == ERROR_SUCCESS)
     {
-        ret = (RegSetValueEx(regkey, L"version", 0, REG_BINARY, (const BYTE*) v, sizeof(*v))
-                != ERROR_SUCCESS);
+        if (RegSetValueEx(regkey, L"version", 0, REG_BINARY, (const BYTE*) v, sizeof(*v)) == ERROR_SUCCESS)
+            ret = true;
         RegCloseKey(regkey);
     }
     else

service.c

@@ -40,8 +40,8 @@ extern options_t o;
 int MyStartService()
 {
 
-  SC_HANDLE schSCManager;
-  SC_HANDLE schService;
+  SC_HANDLE schSCManager = NULL;
+  SC_HANDLE schService = NULL;
   SERVICE_STATUS ssStatus; 
   DWORD dwOldCheckPoint; 
   DWORD dwStartTickCount;
@@ -145,9 +145,6 @@ int MyStartService()
         }
     } 
 
-    CloseServiceHandle(schService); 
-    CloseServiceHandle(schSCManager);
-
     if (ssStatus.dwCurrentState != SERVICE_RUNNING) 
     { 
         /* service hasn't started */
@@ -167,10 +164,15 @@ int MyStartService()
     /* Show "OpenVPN Service Started" Tray Balloon msg */
     ShowTrayBalloon(LoadLocalizedString(IDS_NFO_SERVICE_STARTED), _T(""));
 
+    CloseServiceHandle(schService);
+    CloseServiceHandle(schSCManager);
     return(true);
 
 failed:
-
+    if (schService)
+        CloseServiceHandle(schService);
+    if (schSCManager)
+        CloseServiceHandle(schSCManager);
     /* Set Service Status = Disconnecting */
     o.service_state = service_disconnected;
     SetServiceMenuStatus();
@@ -181,10 +183,11 @@ failed:
 int MyStopService()
 {
 
-  SC_HANDLE schSCManager;
-  SC_HANDLE schService;
+  SC_HANDLE schSCManager = NULL;
+  SC_HANDLE schService = NULL;
   SERVICE_STATUS ssStatus; 
   int i;
+  BOOL ret = false;
 
     // Open a handle to the SC Manager database. 
     schSCManager = OpenSCManager( 
@@ -206,7 +209,7 @@ int MyStopService()
     if (schService == NULL) {
       /* can't open vpn service */
       ShowLocalizedMsg(IDS_ERR_OPEN_VPN_SERVICE);
-      return(false);
+      goto out;
     }
 
     /* Run DisConnect script */
@@ -220,13 +223,20 @@ int MyStopService()
     {
       /* stop failed */
       ShowLocalizedMsg(IDS_ERR_STOP_SERVICE);
-      return(false);
+      goto out;
     }
 
     o.service_state = service_disconnected;
     SetServiceMenuStatus();
     CheckAndSetTrayIcon();
-    return(true);
+    ret = true;
+
+out:
+    if (schService)
+        CloseServiceHandle(schService);
+    if (schSCManager)
+        CloseServiceHandle(schSCManager);
+    return ret;
 }
 
 int MyReStartService()
@@ -244,9 +254,10 @@ int MyReStartService()
 bool
 CheckIServiceStatus(BOOL warn)
 {
-    SC_HANDLE schSCManager;
-    SC_HANDLE schService;
+    SC_HANDLE schSCManager = NULL;
+    SC_HANDLE schService = NULL;
     SERVICE_STATUS ssStatus;
+    BOOL ret = false;
 
     // Open a handle to the SC Manager database.
     schSCManager = OpenSCManager(NULL, NULL, SC_MANAGER_CONNECT);
@@ -262,28 +273,36 @@ CheckIServiceStatus(BOOL warn)
         /* warn that iservice is not installed */
         if (warn)
             ShowLocalizedMsg(IDS_ERR_INSTALL_ISERVICE);
-        return(false);
+        goto out;
     }
 
     if (!QueryServiceStatus(schService, &ssStatus))
-        return(false);
+        goto out;
 
     if (ssStatus.dwCurrentState != SERVICE_RUNNING)
     {
         /* warn that iservice is not started */
         if (warn)
             ShowLocalizedMsg(IDS_ERR_NOTSTARTED_ISERVICE);
-        return(false);
+        goto out;
     }
-    return true;
+    ret = true;
+
+out:
+    if (schService)
+        CloseServiceHandle(schService);
+    if (schSCManager)
+        CloseServiceHandle(schSCManager);
+    return ret;
 }
 
 int CheckServiceStatus()
 {
 
-  SC_HANDLE schSCManager;
-  SC_HANDLE schService;
+  SC_HANDLE schSCManager = NULL;
+  SC_HANDLE schService = NULL;
   SERVICE_STATUS ssStatus; 
+  BOOL ret = false;
 
     // Open a handle to the SC Manager database. 
     schSCManager = OpenSCManager( 
@@ -294,7 +313,7 @@ int CheckServiceStatus()
     if (NULL == schSCManager) {
       o.service_state = service_noaccess;
       SetServiceMenuStatus();
-      return(false);
+      goto out;
     }
 
     schService = OpenService( 
@@ -307,7 +326,7 @@ int CheckServiceStatus()
       ShowLocalizedMsg(IDS_ERR_OPEN_VPN_SERVICE);
       o.service_state = service_noaccess;
       SetServiceMenuStatus();
-      return(false);
+      goto out;
     }
 
     if (!QueryServiceStatus( 
@@ -316,7 +335,7 @@ int CheckServiceStatus()
     {
       /* query failed */
       ShowLocalizedMsg(IDS_ERR_QUERY_SERVICE);
-      return(false);
+      goto out;
     }
  
     if (ssStatus.dwCurrentState == SERVICE_RUNNING) 
@@ -324,13 +343,21 @@ int CheckServiceStatus()
         o.service_state = service_connected;
         SetServiceMenuStatus(); 
         SetTrayIcon(connected);
-        return(true);
+        ret = true;
+        goto out;
     }
     else 
     { 
         o.service_state = service_disconnected;
         SetServiceMenuStatus();
         SetTrayIcon(disconnected);
-        return(false);
+        goto out;
     } 
+
+out:
+    if (schService)
+        CloseServiceHandle(schService);
+    if (schSCManager)
+        CloseServiceHandle(schSCManager);
+    return ret;
 }