CVE-2024-29187: Insufficient Information (NVD-CWE-noinfo)
value: WiX toolset lets developers create installers for Windows Installer, the Windows installation engine. When a bundle runs as SYSTEM user, Burn uses GetTempPathW which points to an insecure directory C:\Windows\Temp to drop and load multiple binaries. Standard users can hijack the binary before it's loaded in the application resulting in elevation of privileges. This vulnerability is fixed in 3.14.1 and 4.0.5.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-03-24T20:15Z
Repository: https://github.com/wixtoolset/wix3
Language: C++
Files changed: src/DTF/Tools/SfxCA/SfxUtil.cpp, src/burn/engine/cache.cpp
Commit date: 2024-03-22 01:04:33 -0700
Commit message: Protect elevated working folder from malicious data

When running elevated, Burn uses the Windows Temp folder as its working folder
to prevent normal processes from tampering with the files. Windows Temp does
allow non-elevated processes to write to the folder but they cannot see the
files there. Unfortunately, contrary to our belief, non-elevated processes
can read the files in Windows Temp by watching for directory changes. This
allows a malicious process to lie in wait, watching the Windows Temp folder
until a Burn process is launched elevated, then attack the working folder.
Mitigate that attack by protecting the working folder to only elevated users.

Managed custom actions also fall back to using the Windows Temp folder in
some cases and thus can be exposed in a similar fashion as an elevated Burn
process. Remove that possibility.

Diff: diff --git a/src/DTF/Tools/SfxCA/SfxUtil.cpp b/src/DTF/Tools/SfxCA/SfxUtil.cpp
index 2e6b0555..32dc6e04 100644
--- a/src/DTF/Tools/SfxCA/SfxUtil.cpp
+++ b/src/DTF/Tools/SfxCA/SfxUtil.cpp
@@ -162,42 +162,22 @@ bool ExtractToTempDirectory(__in MSIHANDLE hSession, __in HMODULE hModule,
                 return false;
         }
         StringCchCopy(szTempDir, cchTempDirBuf, szModule);
         StringCchCat(szTempDir, cchTempDirBuf, L"-");
 
+        BOOL fCreatedDirectory = FALSE;
         DWORD cchTempDir = (DWORD) wcslen(szTempDir);
-        for (int i = 0; DirectoryExists(szTempDir); i++)
+        for (int i = 0; i < 10000 && !fCreatedDirectory; i++)
         {
                 swprintf_s(szTempDir + cchTempDir, cchTempDirBuf - cchTempDir, L"%d", i);
+                fCreatedDirectory = ::CreateDirectory(szTempDir, NULL);
         }
 
-        if (!CreateDirectory(szTempDir, NULL))
+        if (!fCreatedDirectory)
         {
-                cchCopied = GetTempPath(cchTempDirBuf, szTempDir);
-                if (cchCopied == 0 || cchCopied >= cchTempDirBuf)
-                {
-                        Log(hSession, L"Failed to get temp directory. Error code %d", GetLastError());
-                        return false;
-                }
-
-                wchar_t* szModuleName = wcsrchr(szModule, L'\\');
-                if (szModuleName == NULL) szModuleName = szModule;
-                else szModuleName = szModuleName + 1;
-                StringCchCat(szTempDir, cchTempDirBuf, szModuleName);
-                StringCchCat(szTempDir, cchTempDirBuf, L"-");
-
-                cchTempDir = (DWORD) wcslen(szTempDir);
-                for (int i = 0; DirectoryExists(szTempDir); i++)
-                {
-                        swprintf_s(szTempDir + cchTempDir, cchTempDirBuf - cchTempDir, L"%d", i);
-                }
-
-                if (!CreateDirectory(szTempDir, NULL))
-                {
-                        Log(hSession, L"Failed to create temp directory. Error code %d", GetLastError());
-                        return false;
-                }
+                Log(hSession, L"Failed to create temp directory. Error code %d", ::GetLastError());
+                return false;
         }
 
         Log(hSession, L"Extracting custom action to temporary directory: %s\\", szTempDir);
         int err = ExtractCabinet(szModule, szTempDir);
         if (err != 0)
diff --git a/src/burn/engine/cache.cpp b/src/burn/engine/cache.cpp
index 46dc4146..7397eafb 100644
--- a/src/burn/engine/cache.cpp
+++ b/src/burn/engine/cache.cpp
@@ -11,17 +11,19 @@ static const DWORD FILE_OPERATION_RETRY_WAIT = 2000;
 
 static BOOL vfInitializedCache = FALSE;
 static BOOL vfRunningFromCache = FALSE;
 static LPWSTR vsczSourceProcessPath = NULL;
 static LPWSTR vsczWorkingFolder = NULL;
+static BOOL vfWorkingFolderElevated = FALSE;
 static LPWSTR vsczDefaultUserPackageCache = NULL;
 static LPWSTR vsczDefaultMachinePackageCache = NULL;
 static LPWSTR vsczCurrentMachinePackageCache = NULL;
 
 static HRESULT CalculateWorkingFolder(
     __in_z LPCWSTR wzBundleId,
-    __deref_out_z LPWSTR* psczWorkingFolder
+    __deref_out_z LPWSTR* psczWorkingFolder,
+    __out_opt BOOL* pfWorkingFolderElevated
     );
 static HRESULT GetLastUsedSourceFolder(
     __in BURN_VARIABLES* pVariables,
     __out_z LPWSTR* psczLastSource
     );
@@ -193,15 +195,33 @@ extern "C" HRESULT CacheEnsureWorkingFolder(
     __deref_out_z_opt LPWSTR* psczWorkingFolder
     )
 {
     HRESULT hr = S_OK;
     LPWSTR sczWorkingFolder = NULL;
+    BOOL fElevatedWorkingFolder = FALSE;
+    PSECURITY_DESCRIPTOR psd = NULL;
+    LPSECURITY_ATTRIBUTES pWorkingFolderAcl = NULL;
 
-    hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder);
+    hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder, &fElevatedWorkingFolder);
     ExitOnFailure(hr, "Failed to calculate working folder to ensure it exists.");
 
-    hr = DirEnsureExists(sczWorkingFolder, NULL);
+    // If elevated, allocate the pWorkingFolderAcl to protect the working folder to only Admins and SYSTEM.
+    if (fElevatedWorkingFolder)
+    {
+        LPCWSTR wzSddl = L"D:PAI(A;;FA;;;BA)(A;OICIIO;GA;;;BA)(A;;FA;;;SY)(A;OICIIO;GA;;;SY)";
+        if (!::ConvertStringSecurityDescriptorToSecurityDescriptorW(wzSddl, SDDL_REVISION_1, &psd, NULL))
+        {
+            ExitWithLastError(hr, "Failed to create the security descriptor for the working folder.");
+        }
+
+        pWorkingFolderAcl = reinterpret_cast<LPSECURITY_ATTRIBUTES>(MemAlloc(sizeof(SECURITY_ATTRIBUTES), TRUE));
+        pWorkingFolderAcl->nLength = sizeof(SECURITY_ATTRIBUTES);
+        pWorkingFolderAcl->lpSecurityDescriptor = psd;
+        pWorkingFolderAcl->bInheritHandle = FALSE;
+    }
+
+    hr = DirEnsureExists(sczWorkingFolder, pWorkingFolderAcl);
     ExitOnFailure(hr, "Failed create working folder.");
 
     // Best effort to ensure our working folder is not encrypted.
     ::DecryptFileW(sczWorkingFolder, 0);
 
@@ -210,10 +230,15 @@ extern "C" HRESULT CacheEnsureWorkingFolder(
         hr = StrAllocString(psczWorkingFolder, sczWorkingFolder, 0);
         ExitOnFailure(hr, "Failed to copy working folder.");
     }
 
 LExit:
+    ReleaseMem(pWorkingFolderAcl);
+    if (psd)
+    {
+        ::LocalFree(psd);
+    }
     ReleaseStr(sczWorkingFolder);
 
     return hr;
 }
 
@@ -235,11 +260,11 @@ extern "C" HRESULT CacheCalculateBundleWorkingPath(
         hr = PathForCurrentProcess(psczWorkingPath, NULL);
         ExitOnFailure(hr, "Failed to get current process path.");
     }
     else // Otherwise, use the real working folder.
     {
-        hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder);
+        hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder, NULL);
         ExitOnFailure(hr, "Failed to get working folder for bundle.");
 
         hr = StrAllocFormatted(psczWorkingPath, L"%ls%ls\\%ls", sczWorkingFolder, BUNDLE_WORKING_FOLDER_NAME, wzExecutableName);
         ExitOnFailure(hr, "Failed to calculate the bundle working path.");
     }
@@ -256,11 +281,11 @@ extern "C" HRESULT CacheCalculateBundleLayoutWorkingPath(
     )
 {
     HRESULT hr = S_OK;
     LPWSTR sczWorkingFolder = NULL;
 
-    hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath);
+    hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath, NULL);
     ExitOnFailure(hr, "Failed to get working folder for bundle layout.");
 
     hr = StrAllocConcat(psczWorkingPath, wzBundleId, 0);
     ExitOnFailure(hr, "Failed to append bundle id for bundle layout working path.");
 
@@ -276,11 +301,11 @@ extern "C" HRESULT CacheCalculatePayloadWorkingPath(
     __deref_out_z LPWSTR* psczWorkingPath
     )
 {
     HRESULT hr = S_OK;
 
-    hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath);
+    hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath, NULL);
     ExitOnFailure(hr, "Failed to get working folder for payload.");
 
     hr = StrAllocConcat(psczWorkingPath, pPayload->sczKey, 0);
     ExitOnFailure(hr, "Failed to append SHA1 hash as payload unverified path.");
 
@@ -294,11 +319,11 @@ extern "C" HRESULT CacheCalculateContainerWorkingPath(
     __deref_out_z LPWSTR* psczWorkingPath
     )
 {
     HRESULT hr = S_OK;
 
-    hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath);
+    hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath, NULL);
     ExitOnFailure(hr, "Failed to get working folder for container.");
 
     hr = StrAllocConcat(psczWorkingPath, pContainer->sczHash, 0);
     ExitOnFailure(hr, "Failed to append SHA1 hash as container unverified path.");
 
@@ -919,11 +944,11 @@ extern "C" HRESULT CacheRemoveWorkingFolder(
     HRESULT hr = S_OK;
     LPWSTR sczWorkingFolder = NULL;
 
     if (vfInitializedCache)
     {
-        hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder);
+        hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder, NULL);
         ExitOnFailure(hr, "Failed to calculate the working folder to remove it.");
 
         // Try to clean out everything in the working folder.
         hr = DirEnsureDeleteEx(sczWorkingFolder, DIR_DELETE_FILES | DIR_DELETE_RECURSE | DIR_DELETE_SCHEDULE);
         TraceError(hr, "Could not delete bundle engine working folder.");
@@ -1033,11 +1058,11 @@ extern "C" void CacheCleanup(
         hr = DirEnsureDeleteEx(sczFolder, DIR_DELETE_FILES | DIR_DELETE_RECURSE | DIR_DELETE_SCHEDULE);
     }
 
     if (!fPerMachine)
     {
-        hr = CalculateWorkingFolder(wzBundleId, &sczFolder);
+        hr = CalculateWorkingFolder(wzBundleId, &sczFolder, NULL);
         if (SUCCEEDED(hr))
         {
             hr = PathConcat(sczFolder, L"*.*", &sczFiles);
             if (SUCCEEDED(hr))
             {
@@ -1097,11 +1122,12 @@ extern "C" void CacheUninitialize()
 
 // Internal functions.
 
 static HRESULT CalculateWorkingFolder(
     __in_z LPCWSTR /*wzBundleId*/,
-    __deref_out_z LPWSTR* psczWorkingFolder
+    __deref_out_z LPWSTR* psczWorkingFolder,
+    __out_opt BOOL* pfWorkingFolderElevated
     )
 {
     HRESULT hr = S_OK;
     RPC_STATUS rs = RPC_S_OK;
     BOOL fElevated = FALSE;
@@ -1141,15 +1167,22 @@ static HRESULT CalculateWorkingFolder(
             ExitOnRootFailure(hr, "Failed to convert working folder guid into string.");
         }
 
         hr = StrAllocFormatted(&vsczWorkingFolder, L"%ls%ls\\", wzTempPath, wzGuid);
         ExitOnFailure(hr, "Failed to append bundle id on to temp path for working folder.");
+
+        vfWorkingFolderElevated = fElevated;
     }
 
     hr = StrAllocString(psczWorkingFolder, vsczWorkingFolder, 0);
     ExitOnFailure(hr, "Failed to copy working folder path.");
 
+    if (pfWorkingFolderElevated)
+    {
+        *pfWorkingFolderElevated = vfWorkingFolderElevated;
+    }
+
 LExit:
     return hr;
 }
 
 static HRESULT GetRootPath(

Vulnerable code:         for (int i = 0; DirectoryExists(szTempDir); i++)
        if (!CreateDirectory(szTempDir, NULL))
                cchCopied = GetTempPath(cchTempDirBuf, szTempDir);
                if (cchCopied == 0 || cchCopied >= cchTempDirBuf)
                {
                        Log(hSession, L"Failed to get temp directory. Error code %d", GetLastError());
                        return false;
                }

                wchar_t* szModuleName = wcsrchr(szModule, L'\\');
                if (szModuleName == NULL) szModuleName = szModule;
                else szModuleName = szModuleName + 1;
                StringCchCat(szTempDir, cchTempDirBuf, szModuleName);
                StringCchCat(szTempDir, cchTempDirBuf, L"-");

                cchTempDir = (DWORD) wcslen(szTempDir);
                for (int i = 0; DirectoryExists(szTempDir); i++)
                {
                        swprintf_s(szTempDir + cchTempDir, cchTempDirBuf - cchTempDir, L"%d", i);
                }

                if (!CreateDirectory(szTempDir, NULL))
                {
                        Log(hSession, L"Failed to create temp directory. Error code %d", GetLastError());
                        return false;
                }
    __deref_out_z LPWSTR* psczWorkingFolder
    hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder);
    hr = DirEnsureExists(sczWorkingFolder, NULL);
        hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder);
    hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath);
    hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath);
    hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath);
        hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder);
        hr = CalculateWorkingFolder(wzBundleId, &sczFolder);
    __deref_out_z LPWSTR* psczWorkingFolder
Fixed code:         BOOL fCreatedDirectory = FALSE;
        for (int i = 0; i < 10000 && !fCreatedDirectory; i++)
                fCreatedDirectory = ::CreateDirectory(szTempDir, NULL);
        if (!fCreatedDirectory)
                Log(hSession, L"Failed to create temp directory. Error code %d", ::GetLastError());
                return false;
static BOOL vfWorkingFolderElevated = FALSE;
    __deref_out_z LPWSTR* psczWorkingFolder,
    __out_opt BOOL* pfWorkingFolderElevated
    BOOL fElevatedWorkingFolder = FALSE;
    PSECURITY_DESCRIPTOR psd = NULL;
    LPSECURITY_ATTRIBUTES pWorkingFolderAcl = NULL;
    hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder, &fElevatedWorkingFolder);
    // If elevated, allocate the pWorkingFolderAcl to protect the working folder to only Admins and SYSTEM.
    if (fElevatedWorkingFolder)
    {
        LPCWSTR wzSddl = L"D:PAI(A;;FA;;;BA)(A;OICIIO;GA;;;BA)(A;;FA;;;SY)(A;OICIIO;GA;;;SY)";
        if (!::ConvertStringSecurityDescriptorToSecurityDescriptorW(wzSddl, SDDL_REVISION_1, &psd, NULL))
        {
            ExitWithLastError(hr, "Failed to create the security descriptor for the working folder.");
        }

        pWorkingFolderAcl = reinterpret_cast<LPSECURITY_ATTRIBUTES>(MemAlloc(sizeof(SECURITY_ATTRIBUTES), TRUE));
        pWorkingFolderAcl->nLength = sizeof(SECURITY_ATTRIBUTES);
        pWorkingFolderAcl->lpSecurityDescriptor = psd;
        pWorkingFolderAcl->bInheritHandle = FALSE;
    }

    hr = DirEnsureExists(sczWorkingFolder, pWorkingFolderAcl);
    ReleaseMem(pWorkingFolderAcl);
    if (psd)
    {
        ::LocalFree(psd);
    }
        hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder, NULL);
    hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath, NULL);
    hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath, NULL);
    hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath, NULL);
        hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder, NULL);
        hr = CalculateWorkingFolder(wzBundleId, &sczFolder, NULL);
    __deref_out_z LPWSTR* psczWorkingFolder,
    __out_opt BOOL* pfWorkingFolderElevated

        vfWorkingFolderElevated = fElevated;
    if (pfWorkingFolderElevated)
    {
        *pfWorkingFolderElevated = vfWorkingFolderElevated;
    }