Skip to content

Commit fed3d69

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 e84b676 commit fed3d69

File tree

6 files changed

+43
-33
lines changed

6 files changed

+43
-33
lines changed

src/burn/engine/ba.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ HRESULT BootstrapperApplicationInterpretExecuteResult(
111111
);
112112

113113
HRESULT BootstrapperApplicationEnsureWorkingFolder(
114+
__in BOOL fElevated,
114115
__in BURN_CACHE* pCache,
115116
__deref_out_z LPWSTR* psczUserExperienceWorkingFolder
116117
);

src/burn/engine/bootstrapperapplication.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,14 +276,15 @@ EXTERN_C HRESULT BootstrapperApplicationInterpretExecuteResult(
276276
}
277277

278278
EXTERN_C HRESULT BootstrapperApplicationEnsureWorkingFolder(
279+
__in BOOL fElevated,
279280
__in BURN_CACHE* pCache,
280281
__deref_out_z LPWSTR* psczUserExperienceWorkingFolder
281282
)
282283
{
283284
HRESULT hr = S_OK;
284285
LPWSTR sczWorkingFolder = NULL;
285286

286-
hr = CacheEnsureBaseWorkingFolder(pCache, &sczWorkingFolder);
287+
hr = CacheEnsureBaseWorkingFolder(fElevated, pCache, &sczWorkingFolder);
287288
ExitOnFailure(hr, "Failed to create working folder.");
288289

289290
hr = StrAllocFormatted(psczUserExperienceWorkingFolder, L"%ls%ls\\", sczWorkingFolder, L".ba");

src/burn/engine/cache.cpp

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ static HRESULT SecurePath(
106106
__in LPCWSTR wzPath
107107
);
108108
static HRESULT CopyEngineToWorkingFolder(
109+
__in BOOL fElevated,
109110
__in BURN_CACHE* pCache,
110111
__in_z LPCWSTR wzSourcePath,
111112
__in_z LPCWSTR wzWorkingFolderName,
@@ -330,6 +331,7 @@ extern "C" HRESULT CacheEnsureAcquisitionFolder(
330331
}
331332

332333
extern "C" HRESULT CacheEnsureBaseWorkingFolder(
334+
__in BOOL fElevated,
333335
__in BURN_CACHE* pCache,
334336
__deref_out_z_opt LPWSTR* psczBaseWorkingFolder
335337
)
@@ -338,15 +340,32 @@ extern "C" HRESULT CacheEnsureBaseWorkingFolder(
338340

339341
HRESULT hr = S_OK;
340342
LPWSTR sczPotential = NULL;
343+
PSECURITY_DESCRIPTOR psd = NULL;
344+
LPSECURITY_ATTRIBUTES pWorkingFolderAcl = NULL;
341345

342346
if (!pCache->fInitializedBaseWorkingFolder)
343347
{
348+
// If elevated, allocate the pWorkingFolderAcl to protect the working folder to only SYSTEM and Admins.
349+
if (fElevated)
350+
{
351+
LPCWSTR wzSddl = L"D:PAI(A;;FA;;;BA)(A;OICIIO;GA;;;BA)(A;;FA;;;SY)(A;OICIIO;GA;;;SY)";
352+
if (!::ConvertStringSecurityDescriptorToSecurityDescriptorW(wzSddl, SDDL_REVISION_1, &psd, NULL))
353+
{
354+
ExitWithLastError(hr, "Failed to create the security descriptor for the working folder.");
355+
}
356+
357+
pWorkingFolderAcl = reinterpret_cast<LPSECURITY_ATTRIBUTES>(MemAlloc(sizeof(SECURITY_ATTRIBUTES), TRUE));
358+
pWorkingFolderAcl->nLength = sizeof(SECURITY_ATTRIBUTES);
359+
pWorkingFolderAcl->lpSecurityDescriptor = psd;
360+
pWorkingFolderAcl->bInheritHandle = FALSE;
361+
}
362+
344363
for (DWORD i = 0; i < pCache->cPotentialBaseWorkingFolders; ++i)
345364
{
346365
hr = PathConcatRelativeToFullyQualifiedBase(pCache->rgsczPotentialBaseWorkingFolders[i], pCache->wzGuid, &sczPotential);
347366
if (SUCCEEDED(hr))
348367
{
349-
hr = DirEnsureExists(sczPotential, NULL);
368+
hr = DirEnsureExists(sczPotential, pWorkingFolderAcl);
350369
if (SUCCEEDED(hr))
351370
{
352371
pCache->sczBaseWorkingFolder = sczPotential;
@@ -373,6 +392,11 @@ extern "C" HRESULT CacheEnsureBaseWorkingFolder(
373392
}
374393

375394
LExit:
395+
ReleaseMem(pWorkingFolderAcl);
396+
if (psd)
397+
{
398+
::LocalFree(psd);
399+
}
376400
ReleaseStr(sczPotential);
377401

378402
return hr;
@@ -888,6 +912,7 @@ extern "C" HRESULT CachePreparePackage(
888912
}
889913

890914
extern "C" HRESULT CacheBundleToWorkingDirectory(
915+
__in BOOL fElevated,
891916
__in BURN_CACHE* pCache,
892917
__in_z LPCWSTR wzExecutableName,
893918
__in BURN_SECTION* pSection,
@@ -912,7 +937,7 @@ extern "C" HRESULT CacheBundleToWorkingDirectory(
912937
}
913938
else // otherwise, carry on putting the bundle in the working folder.
914939
{
915-
hr = CopyEngineToWorkingFolder(pCache, sczSourcePath, BUNDLE_WORKING_FOLDER_NAME, wzExecutableName, pSection, psczEngineWorkingPath);
940+
hr = CopyEngineToWorkingFolder(fElevated, pCache, sczSourcePath, BUNDLE_WORKING_FOLDER_NAME, wzExecutableName, pSection, psczEngineWorkingPath);
916941
ExitOnFailure(hr, "Failed to copy engine to working folder.");
917942
}
918943

@@ -2063,6 +2088,7 @@ static HRESULT SecurePath(
20632088

20642089

20652090
static HRESULT CopyEngineToWorkingFolder(
2091+
__in BOOL fElevated,
20662092
__in BURN_CACHE* pCache,
20672093
__in_z LPCWSTR wzSourcePath,
20682094
__in_z LPCWSTR wzWorkingFolderName,
@@ -2079,7 +2105,7 @@ static HRESULT CopyEngineToWorkingFolder(
20792105
LPWSTR sczPayloadSourcePath = NULL;
20802106
LPWSTR sczPayloadTargetPath = NULL;
20812107

2082-
hr = CacheEnsureBaseWorkingFolder(pCache, &sczWorkingFolder);
2108+
hr = CacheEnsureBaseWorkingFolder(fElevated, pCache, &sczWorkingFolder);
20832109
ExitOnFailure(hr, "Failed to create working path to copy engine.");
20842110

20852111
hr = PathConcatRelativeToFullyQualifiedBase(sczWorkingFolder, wzWorkingFolderName, &sczTargetDirectory);

src/burn/engine/cache.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ HRESULT CacheEnsureAcquisitionFolder(
9696
__in BURN_CACHE* pCache
9797
);
9898
HRESULT CacheEnsureBaseWorkingFolder(
99+
__in BOOL fElevated,
99100
__in BURN_CACHE* pCache,
100101
__deref_out_z_opt LPWSTR* psczBaseWorkingFolder
101102
);
@@ -171,6 +172,7 @@ HRESULT CachePreparePackage(
171172
__in BURN_PACKAGE* pPackage
172173
);
173174
HRESULT CacheBundleToWorkingDirectory(
175+
__in BOOL fElvated,
174176
__in BURN_CACHE* pCache,
175177
__in_z LPCWSTR wzExecutableName,
176178
__in BURN_SECTION* pSection,

src/burn/engine/core.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ extern "C" HRESULT CoreInitialize(
165165
if (BURN_MODE_NORMAL == pEngineState->internalCommand.mode || BURN_MODE_EMBEDDED == pEngineState->internalCommand.mode)
166166
{
167167
// Extract all UX payloads to working folder.
168-
hr = BootstrapperApplicationEnsureWorkingFolder(&pEngineState->cache, &pEngineState->userExperience.sczTempDirectory);
168+
hr = BootstrapperApplicationEnsureWorkingFolder(pEngineState->internalCommand.fInitiallyElevated, &pEngineState->cache, &pEngineState->userExperience.sczTempDirectory);
169169
ExitOnFailure(hr, "Failed to get unique temporary folder for bootstrapper application.");
170170

171171
hr = PayloadExtractUXContainer(&pEngineState->userExperience.payloads, &containerContext, pEngineState->userExperience.sczTempDirectory);
@@ -588,7 +588,7 @@ extern "C" HRESULT CoreElevate(
588588
// If the elevated companion pipe isn't created yet, let's make that happen.
589589
if (!pEngineState->sczBundleEngineWorkingPath)
590590
{
591-
hr = CacheBundleToWorkingDirectory(&pEngineState->cache, pEngineState->registration.sczExecutableName, &pEngineState->section, &pEngineState->sczBundleEngineWorkingPath);
591+
hr = CacheBundleToWorkingDirectory(pEngineState->internalCommand.fInitiallyElevated, &pEngineState->cache, pEngineState->registration.sczExecutableName, &pEngineState->section, &pEngineState->sczBundleEngineWorkingPath);
592592
ExitOnFailure(hr, "Failed to cache engine to working directory.");
593593
}
594594

@@ -697,7 +697,7 @@ extern "C" HRESULT CoreApply(
697697
// Ensure the engine is cached to the working path.
698698
if (!pEngineState->sczBundleEngineWorkingPath)
699699
{
700-
hr = CacheBundleToWorkingDirectory(&pEngineState->cache, pEngineState->registration.sczExecutableName, &pEngineState->section, &pEngineState->sczBundleEngineWorkingPath);
700+
hr = CacheBundleToWorkingDirectory(pEngineState->internalCommand.fInitiallyElevated, &pEngineState->cache, pEngineState->registration.sczExecutableName, &pEngineState->section, &pEngineState->sczBundleEngineWorkingPath);
701701
ExitOnFailure(hr, "Failed to cache engine to working directory.");
702702
}
703703

src/dtf/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);

0 commit comments

Comments
 (0)