CVE-2024-29188: Insufficient Information (NVD-CWE-noinfo)
value: WiX toolset lets developers create installers for Windows Installer, the Windows installation engine. The custom action behind WiX's `RemoveFolderEx` functionality could allow a standard user to delete protected directories. `RemoveFolderEx` deletes an entire directory tree during installation or uninstallation. It does so by recursing every subdirectory starting at a specified directory and adding each subdirectory to the list of directories Windows Installer should delete. If the setup author instructed `RemoveFolderEx` to delete a per-user folder from a per-machine installer, an attacker could create a directory junction in that per-user folder pointing to a per-machine, protected directory. Windows Installer, when executing the per-machine installer after approval by an administrator, would delete the target of the directory junction. 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/ext/ca/wixca/dll/RemoveFoldersEx.cpp
Commit date: 2024-03-22 01:03:57 -0700
Commit message: Don't follow junctions when recursing directories.

When deleting directories recursively, an elevated custom action
following junctions in a user-writable location could recurse into
any directory, including some that you might not want to be deleted.
Therefore, avoid recursing into directories that are actually
junctions (aka "reparse points").

This applies to:

- The RemoveFoldersEx custom action (which doesn't actually do deletions
but would instruct elevated MSI to delete on your behalf).
- DTF's custom action runner.

Diff: diff --git a/src/DTF/Tools/SfxCA/SfxUtil.cpp b/src/DTF/Tools/SfxCA/SfxUtil.cpp
index 1bf2c5b2..2e6b0555 100644
--- a/src/DTF/Tools/SfxCA/SfxUtil.cpp
+++ b/src/DTF/Tools/SfxCA/SfxUtil.cpp
@@ -91,11 +91,13 @@ bool DeleteDirectory(const wchar_t* szDir)
         while (hSearch != INVALID_HANDLE_VALUE)
         {
                 StringCchCopy(szPath + cchDir + 1, cchPathBuf - (cchDir + 1), fd.cFileName);
                 if ((fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0)
                 {
-                        if (wcscmp(fd.cFileName, L".") != 0 && wcscmp(fd.cFileName, L"..") != 0)
+                        if (wcscmp(fd.cFileName, L".") != 0
+                            && wcscmp(fd.cFileName, L"..") != 0
+                            && ((fd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0))
                         {
                                 DeleteDirectory(szPath);
                         }
                 }
                 else
diff --git a/src/ext/ca/wixca/dll/RemoveFoldersEx.cpp b/src/ext/ca/wixca/dll/RemoveFoldersEx.cpp
index dbfaaf73..a2a29113 100644
--- a/src/ext/ca/wixca/dll/RemoveFoldersEx.cpp
+++ b/src/ext/ca/wixca/dll/RemoveFoldersEx.cpp
@@ -22,10 +22,18 @@ static HRESULT RecursePath(
     LPWSTR sczProperty = NULL;
     HANDLE hFind = INVALID_HANDLE_VALUE;
     WIN32_FIND_DATAW wfd;
     LPWSTR sczNext = NULL;
 
+    // Do NOT follow junctions.
+    DWORD dwAttributes = ::GetFileAttributesW(wzPath);
+    if (dwAttributes & FILE_ATTRIBUTE_REPARSE_POINT)
+    {
+        WcaLog(LOGMSG_STANDARD, "Path is a junction; skipping: %ls", wzPath);
+        ExitFunction();
+    }
+
     // First recurse down to all the child directories.
     hr = StrAllocFormatted(&sczSearch, L"%s*", wzPath);
     ExitOnFailure1(hr, "Failed to allocate file search string in path: %S", wzPath);
 
     hFind = ::FindFirstFileW(sczSearch, &wfd);
@@ -157,14 +165,14 @@ extern "C" UINT WINAPI WixRemoveFoldersEx(
             ExitOnFailure2(hr = E_INVALIDARG, "Missing folder property: %S for row: %S", sczProperty, sczId);
         }
 
         hr = PathExpand(&sczExpandedPath, sczPath, PATH_EXPAND_ENVIRONMENT);
         ExitOnFailure2(hr, "Failed to expand path: %S for row: %S", sczPath, sczId);
-        
+
         hr = PathBackslashTerminate(&sczExpandedPath);
         ExitOnFailure1(hr, "Failed to backslash-terminate path: %S", sczExpandedPath);
-    
+
         WcaLog(LOGMSG_STANDARD, "Recursing path: %S for row: %S.", sczExpandedPath, sczId);
         hr = RecursePath(sczExpandedPath, sczId, sczComponent, sczProperty, iMode, &dwCounter, &hTable, &hColumns);
         ExitOnFailure2(hr, "Failed while navigating path: %S for row: %S", sczPath, sczId);
     }
 

Vulnerable code:                         if (wcscmp(fd.cFileName, L".") != 0 && wcscmp(fd.cFileName, L"..") != 0)
        
    
Fixed code:                         if (wcscmp(fd.cFileName, L".") != 0
                            && wcscmp(fd.cFileName, L"..") != 0
                            && ((fd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0))
    // Do NOT follow junctions.
    DWORD dwAttributes = ::GetFileAttributesW(wzPath);
    if (dwAttributes & FILE_ATTRIBUTE_REPARSE_POINT)
    {
        WcaLog(LOGMSG_STANDARD, "Path is a junction; skipping: %ls", wzPath);
        ExitFunction();
    }