Don't serialize specific variables to the elevated Burn process #290

Merged
merged 2 commits into from Aug 21, 2015

Projects

None yet

3 participants

@rseanhall
Member

#228 allowed the unelevated Burn process to overwrite built in variables to fix 4630, but the elevated Burn process was using the ProgramFiles variables to make security decisions for ApprovedExeForElevation. So mark those variables as not to be serialized to the elevated process.

@heaths heaths commented on an outdated diff Aug 18, 2015
src/burn/engine/variable.cpp
#endif
- {L"ProgramFiles6432Folder", InitializeVariable6432Folder, CSIDL_PROGRAM_FILES},
- {L"ProgramMenuFolder", InitializeVariableCsidlFolder, CSIDL_PROGRAMS},
+ {L"ProgramFiles6432Folder", InitializeVariable6432Folder, CSIDL_PROGRAM_FILES, FALSE, TRUE},
+ {L"ProgramMenuFolder", InitializeVariableCsidlFolder, CSIDL_PROGRAMS, FALSE, TRUE},
{L"RebootPending", InitializeVariableRebootPending, 0},
{L"SendToFolder", InitializeVariableCsidlFolder, CSIDL_SENDTO},
@heaths
heaths Aug 18, 2015 Contributor

What about this one?

@heaths heaths commented on an outdated diff Aug 18, 2015
src/burn/engine/variable.cpp
{L"RebootPending", InitializeVariableRebootPending, 0},
{L"SendToFolder", InitializeVariableCsidlFolder, CSIDL_SENDTO},
{L"ServicePackLevel", InitializeVariableVersionNT, OS_INFO_VARIABLE_ServicePackLevel},
- {L"StartMenuFolder", InitializeVariableCsidlFolder, CSIDL_STARTMENU},
- {L"StartupFolder", InitializeVariableCsidlFolder, CSIDL_STARTUP},
- {L"SystemFolder", InitializeVariableSystemFolder, FALSE},
- {L"System64Folder", InitializeVariableSystemFolder, TRUE},
+ {L"StartMenuFolder", InitializeVariableCsidlFolder, CSIDL_STARTMENU, FALSE, TRUE},
+ {L"StartupFolder", InitializeVariableCsidlFolder, CSIDL_STARTUP, FALSE, TRUE},
+ {L"SystemFolder", InitializeVariableSystemFolder, FALSE, FALSE, TRUE},
+ {L"System64Folder", InitializeVariableSystemFolder, TRUE, FALSE, TRUE},
{L"SystemLanguageID", InitializeSystemLanguageID, 0},
{L"TempFolder", InitializeVariableTempFolder, 0},
@heaths
heaths Aug 18, 2015 Contributor

This one.

@heaths heaths commented on an outdated diff Aug 18, 2015
src/burn/engine/variable.cpp
{L"TerminalServer", InitializeVariableOsInfo, OS_INFO_VARIABLE_TerminalServer},
{L"UserLanguageID", InitializeUserLanguageID, 0},
{L"VersionMsi", InitializeVariableVersionMsi, 0},
{L"VersionNT", InitializeVariableVersionNT, OS_INFO_VARIABLE_VersionNT},
{L"VersionNT64", InitializeVariableVersionNT, OS_INFO_VARIABLE_VersionNT64},
- {L"WindowsFolder", InitializeVariableCsidlFolder, CSIDL_WINDOWS},
+ {L"WindowsFolder", InitializeVariableCsidlFolder, CSIDL_WINDOWS, FALSE, TRUE},
{L"WindowsVolume", InitializeVariableWindowsVolumeFolder, 0},
@heaths
heaths Aug 18, 2015 Contributor

This one.

@barnson barnson added this to the v3.10 milestone Aug 18, 2015
@rseanhall rseanhall Convert fBuiltin into an enum
Convert fBuiltin into an enum to use a whitelist instead of a blacklist for the builtin variables that the unelevated process can serialize to the elevated process.

Delete dead code.
0807371
@rseanhall
Member

Updated based on yesterday's feedback. Also deleted property.h since it wasn't being used.

@heaths heaths commented on the diff Aug 20, 2015
src/burn/engine/variable.cpp
@@ -240,7 +242,7 @@ extern "C" HRESULT VariableInitialize(
{L"ProgramFilesFolder", InitializeVariableCsidlFolder, CSIDL_PROGRAM_FILESX86},
#else
{L"ProgramFiles64Folder", InitializeVariableRegistryFolder, CSIDL_PROGRAM_FILES},
- {L"ProgramFilesFolder", InitializeVariableCsidlFolder, CSIDL_PROGRAM_FILES},
+ {L"ProgramFilesFolder", InitializeVariableCsidlFolder, CSIDL_PROGRAM_FILES },
@heaths
heaths Aug 20, 2015 Contributor

Not a big deal, but cleaner if you get rid of the now-added space. Probably just if you have any other changes to make.

@heaths
Contributor
heaths commented Aug 20, 2015

Looks good.

@barnson barnson merged commit 0807371 into wixtoolset:develop Aug 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment