installer: improve resource freeing flow

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

2 files changed, 44 insertions, 31 deletions

diff --git a/installer/installation.c b/installer/installation.c
index e31124b..cd93ffb 100644
--- a/installer/installation.c
+++ b/installer/installation.c
@@ -298,7 +298,7 @@ InstallWintun(BOOL UpdateExisting)
         Logger(LOG_WARN, TEXT("A reboot might be required, which really should not be the case"));
 
 cleanupDelete:
-    LastError = LastError ? LastError : GetLastError();
+    LastError = GetLastError();
     DeleteFile(CatPath);
     DeleteFile(SysPath);
     DeleteFile(InfPath);
@@ -410,8 +410,9 @@ ForceCloseWintunAdapterHandle(_In_ HDEVINFO DeviceInfoSet, _In_ SP_DEVINFO_DATA
     TCHAR *InstanceId = calloc(sizeof(*InstanceId), RequiredBytes);
     if (!InstanceId)
         return FALSE;
+    BOOL Ret = FALSE;
     if (!SetupDiGetDeviceInstanceId(DeviceInfoSet, DeviceInfo, InstanceId, RequiredBytes, &RequiredBytes))
-        return FALSE;
+        goto out;
     TCHAR *InterfaceList = NULL;
     for (;;)
     {
@@ -419,22 +420,22 @@ ForceCloseWintunAdapterHandle(_In_ HDEVINFO DeviceInfoSet, _In_ SP_DEVINFO_DATA
         if (CM_Get_Device_Interface_List_Size(
                 &RequiredBytes, (LPGUID)&GUID_DEVINTERFACE_NET, InstanceId, CM_GET_DEVICE_INTERFACE_LIST_PRESENT) !=
             CR_SUCCESS)
-            return FALSE;
+            goto out;
         InterfaceList = calloc(sizeof(*InterfaceList), RequiredBytes);
         if (!InterfaceList)
-            return FALSE;
-        CONFIGRET Ret = CM_Get_Device_Interface_List(
+            goto out;
+        CONFIGRET CRet = CM_Get_Device_Interface_List(
             (LPGUID)&GUID_DEVINTERFACE_NET,
             InstanceId,
             InterfaceList,
             RequiredBytes,
             CM_GET_DEVICE_INTERFACE_LIST_PRESENT);
-        if (Ret == CR_SUCCESS)
+        if (CRet == CR_SUCCESS)
             break;
-        if (Ret != CR_BUFFER_SMALL)
+        if (CRet != CR_BUFFER_SMALL)
         {
             free(InterfaceList);
-            return FALSE;
+            goto out;
         }
     }
 
@@ -448,11 +449,13 @@ ForceCloseWintunAdapterHandle(_In_ HDEVINFO DeviceInfoSet, _In_ SP_DEVINFO_DATA
         NULL);
     free(InterfaceList);
     if (NdisHandle == INVALID_HANDLE_VALUE)
-        return FALSE;
-    BOOL Ret = DeviceIoControl(NdisHandle, TUN_IOCTL_FORCE_CLOSE_HANDLES, NULL, 0, NULL, 0, &RequiredBytes, NULL);
+        goto out;
+    Ret = DeviceIoControl(NdisHandle, TUN_IOCTL_FORCE_CLOSE_HANDLES, NULL, 0, NULL, 0, &RequiredBytes, NULL);
     DWORD LastError = GetLastError();
     CloseHandle(NdisHandle);
     SetLastError(LastError);
+out:
+    free(InstanceId);
     return Ret;
 }
 
diff --git a/installer/rundll32.c b/installer/rundll32.c
index cc1a6d2..17295e0 100644
--- a/installer/rundll32.c
+++ b/installer/rundll32.c
@@ -42,15 +42,19 @@ static BOOL ElevateToSystem(VOID)
     TOKEN_PRIVILEGES Privileges = { .PrivilegeCount = 1, .Privileges = { { .Attributes = SE_PRIVILEGE_ENABLED } } };
     CHAR LocalSystemSid[0x400];
     DWORD RequiredBytes = sizeof(LocalSystemSid);
-
-    if (!CreateWellKnownSid(WinLocalSystemSid, NULL, &LocalSystemSid, &RequiredBytes))
-        goto cleanup;
     struct
     {
         TOKEN_USER MaybeLocalSystem;
         CHAR LargeEnoughForLocalSystem[0x400];
     } TokenUserBuffer;
-    if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &CurrentProcessToken))
+
+    Ret = CreateWellKnownSid(WinLocalSystemSid, NULL, &LocalSystemSid, &RequiredBytes);
+    LastError = GetLastError();
+    if (!Ret)
+        goto cleanup;
+    Ret = OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &CurrentProcessToken);
+    LastError = GetLastError();
+    if (!Ret)
         goto cleanup;
     Ret =
         GetTokenInformation(CurrentProcessToken, TokenUser, &TokenUserBuffer, sizeof(TokenUserBuffer), &RequiredBytes);
@@ -60,41 +64,47 @@ static BOOL ElevateToSystem(VOID)
         goto cleanup;
     if (EqualSid(TokenUserBuffer.MaybeLocalSystem.User.Sid, LocalSystemSid))
         return TRUE;
-    if (!LookupPrivilegeValue(NULL, SE_DEBUG_NAME, &Privileges.Privileges[0].Luid))
+    Ret = LookupPrivilegeValue(NULL, SE_DEBUG_NAME, &Privileges.Privileges[0].Luid);
+    LastError = GetLastError();
+    if (!Ret)
         goto cleanup;
     ProcessSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
+    LastError = GetLastError();
     if (ProcessSnapshot == INVALID_HANDLE_VALUE)
         goto cleanup;
-    for (Ret = Process32First(ProcessSnapshot, &ProcessEntry); Ret;
-         LastError = GetLastError(), Ret = Process32Next(ProcessSnapshot, &ProcessEntry))
+    for (Ret = Process32First(ProcessSnapshot, &ProcessEntry); Ret; Ret = Process32Next(ProcessSnapshot, &ProcessEntry))
     {
         if (_tcsicmp(ProcessEntry.szExeFile, TEXT("winlogon.exe")))
             continue;
         RevertToSelf();
-        if (!ImpersonateSelf(SecurityImpersonation))
-            continue;
-        if (!OpenThreadToken(GetCurrentThread(), TOKEN_ADJUST_PRIVILEGES, FALSE, &ThreadToken))
+        Ret = ImpersonateSelf(SecurityImpersonation);
+        LastError = GetLastError();
+        if (!Ret)
             continue;
-        if (!AdjustTokenPrivileges(ThreadToken, FALSE, &Privileges, sizeof(Privileges), NULL, NULL))
-        {
-            LastError = GetLastError();
-            CloseHandle(ThreadToken);
+        Ret = OpenThreadToken(GetCurrentThread(), TOKEN_ADJUST_PRIVILEGES, FALSE, &ThreadToken);
+        LastError = GetLastError();
+        if (!Ret)
             continue;
-        }
+        Ret = AdjustTokenPrivileges(ThreadToken, FALSE, &Privileges, sizeof(Privileges), NULL, NULL);
+        LastError = GetLastError();
         CloseHandle(ThreadToken);
+        if (!Ret)
+            continue;
 
         WinlogonProcess = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, ProcessEntry.th32ProcessID);
+        LastError = GetLastError();
         if (!WinlogonProcess)
             continue;
-        if (!OpenProcessToken(WinlogonProcess, TOKEN_IMPERSONATE | TOKEN_DUPLICATE, &WinlogonToken))
-            continue;
+        Ret = OpenProcessToken(WinlogonProcess, TOKEN_IMPERSONATE | TOKEN_DUPLICATE, &WinlogonToken);
+        LastError = GetLastError();
         CloseHandle(WinlogonProcess);
-        if (!DuplicateToken(WinlogonToken, SecurityImpersonation, &DuplicatedToken))
-        {
-            LastError = GetLastError();
+        if (!Ret)
             continue;
-        }
+        Ret = DuplicateToken(WinlogonToken, SecurityImpersonation, &DuplicatedToken);
+        LastError = GetLastError();
         CloseHandle(WinlogonToken);
+        if (!Ret)
+            continue;
         if (!GetTokenInformation(DuplicatedToken, TokenUser, &TokenUserBuffer, sizeof(TokenUserBuffer), &RequiredBytes))
             goto next;
         if (SetLastError(ERROR_ACCESS_DENIED), !EqualSid(TokenUserBuffer.MaybeLocalSystem.User.Sid, LocalSystemSid))