Skip to content
This repository was archived by the owner on Feb 14, 2025. It is now read-only.

Commit 6d372e5

Browse files
committed
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.
1 parent 93eeb5f commit 6d372e5

File tree

2 files changed

+49
-36
lines changed

2 files changed

+49
-36
lines changed

src/DTF/Tools/SfxCA/SfxUtil.cpp

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -164,38 +164,18 @@ bool ExtractToTempDirectory(__in MSIHANDLE hSession, __in HMODULE hModule,
164164
StringCchCopy(szTempDir, cchTempDirBuf, szModule);
165165
StringCchCat(szTempDir, cchTempDirBuf, L"-");
166166

167+
BOOL fCreatedDirectory = FALSE;
167168
DWORD cchTempDir = (DWORD) wcslen(szTempDir);
168-
for (int i = 0; DirectoryExists(szTempDir); i++)
169+
for (int i = 0; i < 10000 && !fCreatedDirectory; i++)
169170
{
170171
swprintf_s(szTempDir + cchTempDir, cchTempDirBuf - cchTempDir, L"%d", i);
172+
fCreatedDirectory = ::CreateDirectory(szTempDir, NULL);
171173
}
172174

173-
if (!CreateDirectory(szTempDir, NULL))
175+
if (!fCreatedDirectory)
174176
{
175-
cchCopied = GetTempPath(cchTempDirBuf, szTempDir);
176-
if (cchCopied == 0 || cchCopied >= cchTempDirBuf)
177-
{
178-
Log(hSession, L"Failed to get temp directory. Error code %d", GetLastError());
179-
return false;
180-
}
181-
182-
wchar_t* szModuleName = wcsrchr(szModule, L'\\');
183-
if (szModuleName == NULL) szModuleName = szModule;
184-
else szModuleName = szModuleName + 1;
185-
StringCchCat(szTempDir, cchTempDirBuf, szModuleName);
186-
StringCchCat(szTempDir, cchTempDirBuf, L"-");
187-
188-
cchTempDir = (DWORD) wcslen(szTempDir);
189-
for (int i = 0; DirectoryExists(szTempDir); i++)
190-
{
191-
swprintf_s(szTempDir + cchTempDir, cchTempDirBuf - cchTempDir, L"%d", i);
192-
}
193-
194-
if (!CreateDirectory(szTempDir, NULL))
195-
{
196-
Log(hSession, L"Failed to create temp directory. Error code %d", GetLastError());
197-
return false;
198-
}
177+
Log(hSession, L"Failed to create temp directory. Error code %d", ::GetLastError());
178+
return false;
199179
}
200180

201181
Log(hSession, L"Extracting custom action to temporary directory: %s\\", szTempDir);

src/burn/engine/cache.cpp

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,15 @@ static BOOL vfInitializedCache = FALSE;
1313
static BOOL vfRunningFromCache = FALSE;
1414
static LPWSTR vsczSourceProcessPath = NULL;
1515
static LPWSTR vsczWorkingFolder = NULL;
16+
static BOOL vfWorkingFolderElevated = FALSE;
1617
static LPWSTR vsczDefaultUserPackageCache = NULL;
1718
static LPWSTR vsczDefaultMachinePackageCache = NULL;
1819
static LPWSTR vsczCurrentMachinePackageCache = NULL;
1920

2021
static HRESULT CalculateWorkingFolder(
2122
__in_z LPCWSTR wzBundleId,
22-
__deref_out_z LPWSTR* psczWorkingFolder
23+
__deref_out_z LPWSTR* psczWorkingFolder,
24+
__out_opt BOOL* pfWorkingFolderElevated
2325
);
2426
static HRESULT GetLastUsedSourceFolder(
2527
__in BURN_VARIABLES* pVariables,
@@ -195,11 +197,29 @@ extern "C" HRESULT CacheEnsureWorkingFolder(
195197
{
196198
HRESULT hr = S_OK;
197199
LPWSTR sczWorkingFolder = NULL;
200+
BOOL fElevatedWorkingFolder = FALSE;
201+
PSECURITY_DESCRIPTOR psd = NULL;
202+
LPSECURITY_ATTRIBUTES pWorkingFolderAcl = NULL;
198203

199-
hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder);
204+
hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder, &fElevatedWorkingFolder);
200205
ExitOnFailure(hr, "Failed to calculate working folder to ensure it exists.");
201206

202-
hr = DirEnsureExists(sczWorkingFolder, NULL);
207+
// If elevated, allocate the pWorkingFolderAcl to protect the working folder to only Admins and SYSTEM.
208+
if (fElevatedWorkingFolder)
209+
{
210+
LPCWSTR wzSddl = L"D:PAI(A;;FA;;;BA)(A;OICIIO;GA;;;BA)(A;;FA;;;SY)(A;OICIIO;GA;;;SY)";
211+
if (!::ConvertStringSecurityDescriptorToSecurityDescriptorW(wzSddl, SDDL_REVISION_1, &psd, NULL))
212+
{
213+
ExitWithLastError(hr, "Failed to create the security descriptor for the working folder.");
214+
}
215+
216+
pWorkingFolderAcl = reinterpret_cast<LPSECURITY_ATTRIBUTES>(MemAlloc(sizeof(SECURITY_ATTRIBUTES), TRUE));
217+
pWorkingFolderAcl->nLength = sizeof(SECURITY_ATTRIBUTES);
218+
pWorkingFolderAcl->lpSecurityDescriptor = psd;
219+
pWorkingFolderAcl->bInheritHandle = FALSE;
220+
}
221+
222+
hr = DirEnsureExists(sczWorkingFolder, pWorkingFolderAcl);
203223
ExitOnFailure(hr, "Failed create working folder.");
204224

205225
// Best effort to ensure our working folder is not encrypted.
@@ -212,6 +232,11 @@ extern "C" HRESULT CacheEnsureWorkingFolder(
212232
}
213233

214234
LExit:
235+
ReleaseMem(pWorkingFolderAcl);
236+
if (psd)
237+
{
238+
::LocalFree(psd);
239+
}
215240
ReleaseStr(sczWorkingFolder);
216241

217242
return hr;
@@ -237,7 +262,7 @@ extern "C" HRESULT CacheCalculateBundleWorkingPath(
237262
}
238263
else // Otherwise, use the real working folder.
239264
{
240-
hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder);
265+
hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder, NULL);
241266
ExitOnFailure(hr, "Failed to get working folder for bundle.");
242267

243268
hr = StrAllocFormatted(psczWorkingPath, L"%ls%ls\\%ls", sczWorkingFolder, BUNDLE_WORKING_FOLDER_NAME, wzExecutableName);
@@ -258,7 +283,7 @@ extern "C" HRESULT CacheCalculateBundleLayoutWorkingPath(
258283
HRESULT hr = S_OK;
259284
LPWSTR sczWorkingFolder = NULL;
260285

261-
hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath);
286+
hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath, NULL);
262287
ExitOnFailure(hr, "Failed to get working folder for bundle layout.");
263288

264289
hr = StrAllocConcat(psczWorkingPath, wzBundleId, 0);
@@ -278,7 +303,7 @@ extern "C" HRESULT CacheCalculatePayloadWorkingPath(
278303
{
279304
HRESULT hr = S_OK;
280305

281-
hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath);
306+
hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath, NULL);
282307
ExitOnFailure(hr, "Failed to get working folder for payload.");
283308

284309
hr = StrAllocConcat(psczWorkingPath, pPayload->sczKey, 0);
@@ -296,7 +321,7 @@ extern "C" HRESULT CacheCalculateContainerWorkingPath(
296321
{
297322
HRESULT hr = S_OK;
298323

299-
hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath);
324+
hr = CalculateWorkingFolder(wzBundleId, psczWorkingPath, NULL);
300325
ExitOnFailure(hr, "Failed to get working folder for container.");
301326

302327
hr = StrAllocConcat(psczWorkingPath, pContainer->sczHash, 0);
@@ -921,7 +946,7 @@ extern "C" HRESULT CacheRemoveWorkingFolder(
921946

922947
if (vfInitializedCache)
923948
{
924-
hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder);
949+
hr = CalculateWorkingFolder(wzBundleId, &sczWorkingFolder, NULL);
925950
ExitOnFailure(hr, "Failed to calculate the working folder to remove it.");
926951

927952
// Try to clean out everything in the working folder.
@@ -1035,7 +1060,7 @@ extern "C" void CacheCleanup(
10351060

10361061
if (!fPerMachine)
10371062
{
1038-
hr = CalculateWorkingFolder(wzBundleId, &sczFolder);
1063+
hr = CalculateWorkingFolder(wzBundleId, &sczFolder, NULL);
10391064
if (SUCCEEDED(hr))
10401065
{
10411066
hr = PathConcat(sczFolder, L"*.*", &sczFiles);
@@ -1099,7 +1124,8 @@ extern "C" void CacheUninitialize()
10991124

11001125
static HRESULT CalculateWorkingFolder(
11011126
__in_z LPCWSTR /*wzBundleId*/,
1102-
__deref_out_z LPWSTR* psczWorkingFolder
1127+
__deref_out_z LPWSTR* psczWorkingFolder,
1128+
__out_opt BOOL* pfWorkingFolderElevated
11031129
)
11041130
{
11051131
HRESULT hr = S_OK;
@@ -1143,11 +1169,18 @@ static HRESULT CalculateWorkingFolder(
11431169

11441170
hr = StrAllocFormatted(&vsczWorkingFolder, L"%ls%ls\\", wzTempPath, wzGuid);
11451171
ExitOnFailure(hr, "Failed to append bundle id on to temp path for working folder.");
1172+
1173+
vfWorkingFolderElevated = fElevated;
11461174
}
11471175

11481176
hr = StrAllocString(psczWorkingFolder, vsczWorkingFolder, 0);
11491177
ExitOnFailure(hr, "Failed to copy working folder path.");
11501178

1179+
if (pfWorkingFolderElevated)
1180+
{
1181+
*pfWorkingFolderElevated = vfWorkingFolderElevated;
1182+
}
1183+
11511184
LExit:
11521185
return hr;
11531186
}

0 commit comments

Comments
 (0)