Skip to content

Commit 9193a4f

Browse files
nirbarnirbar
committed
Fix mis-ordering of escaped command line arguments
1 parent 2575134 commit 9193a4f

6 files changed

Lines changed: 109 additions & 93 deletions

File tree

‎src/burn/engine/bundlepackageengine.cpp‎

Lines changed: 24 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -808,11 +808,8 @@ static HRESULT ExecuteBundle(
808808
LPWSTR sczCachedDirectory = NULL;
809809
LPWSTR sczExecutablePath = NULL;
810810
LPWSTR sczBaseCommand = NULL;
811-
LPWSTR sczUnformattedUserArgs = NULL;
812811
LPWSTR sczUserArgs = NULL;
813812
LPWSTR sczUserArgsObfuscated = NULL;
814-
LPWSTR sczUserArgsEscaped = NULL;
815-
LPWSTR sczUserArgsEscapedObfuscated = NULL;
816813
LPWSTR sczCommandObfuscated = NULL;
817814
LPWSTR sczArpUninstallString = NULL;
818815
int argcArp = 0;
@@ -924,8 +921,11 @@ static HRESULT ExecuteBundle(
924921
// now add optional arguments
925922
if (wzArguments && *wzArguments)
926923
{
927-
hr = StrAllocString(&sczUnformattedUserArgs, wzArguments, 0);
928-
ExitOnFailure(hr, "Failed to copy package arguments.");
924+
hr = ExeEngineAppendCommandLineArgument(wzArguments, pVariables, FALSE, &sczUserArgs);
925+
ExitOnFailure(hr, "Failed to get command-line arguments.");
926+
927+
hr = ExeEngineAppendCommandLineArgument(wzArguments, pVariables, TRUE, &sczUserArgsObfuscated);
928+
ExitOnFailure(hr, "Failed to get obfuscated command-line arguments.");
929929
}
930930

931931
for (DWORD i = 0; i < pPackage->Bundle.cCommandLineArguments; ++i)
@@ -941,59 +941,62 @@ static HRESULT ExecuteBundle(
941941

942942
if (fCondition)
943943
{
944-
if (sczUnformattedUserArgs && !commandLineArgument->fEscape)
945-
{
946-
hr = StrAllocConcat(&sczUnformattedUserArgs, L" ", 0);
947-
ExitOnFailure(hr, "Failed to separate command-line arguments.");
948-
}
949-
950944
switch (action)
951945
{
952946
case BOOTSTRAPPER_ACTION_STATE_INSTALL:
953947
if (commandLineArgument->fEscape)
954948
{
955-
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczInstallArgument, pVariables, FALSE, &sczUserArgsEscaped);
949+
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczInstallArgument, pVariables, FALSE, &sczUserArgs);
956950
ExitOnFailure(hr, "Failed to get command-line argument for install.");
957951

958-
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczInstallArgument, pVariables, TRUE, &sczUserArgsEscapedObfuscated);
952+
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczInstallArgument, pVariables, TRUE, &sczUserArgsObfuscated);
959953
ExitOnFailure(hr, "Failed to get obfuscated command-line argument for install.");
960954
}
961955
else
962956
{
963-
hr = StrAllocConcat(&sczUnformattedUserArgs, commandLineArgument->sczInstallArgument, 0);
957+
hr = ExeEngineAppendCommandLineArgument(commandLineArgument->sczInstallArgument, pVariables, FALSE, &sczUserArgs);
964958
ExitOnFailure(hr, "Failed to get command-line argument for install.");
959+
960+
hr = ExeEngineAppendCommandLineArgument(commandLineArgument->sczInstallArgument, pVariables, TRUE, &sczUserArgsObfuscated);
961+
ExitOnFailure(hr, "Failed to get obfuscated command-line argument for install.");
965962
}
966963
break;
967964

968965
case BOOTSTRAPPER_ACTION_STATE_UNINSTALL:
969966
if (commandLineArgument->fEscape)
970967
{
971-
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczUninstallArgument, pVariables, FALSE, &sczUserArgsEscaped);
968+
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczUninstallArgument, pVariables, FALSE, &sczUserArgs);
972969
ExitOnFailure(hr, "Failed to get command-line argument for uninstall.");
973970

974-
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczUninstallArgument, pVariables, TRUE, &sczUserArgsEscapedObfuscated);
971+
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczUninstallArgument, pVariables, TRUE, &sczUserArgsObfuscated);
975972
ExitOnFailure(hr, "Failed to get obfuscated command-line argument for uninstall.");
976973
}
977974
else
978975
{
979-
hr = StrAllocConcat(&sczUnformattedUserArgs, commandLineArgument->sczUninstallArgument, 0);
976+
hr = ExeEngineAppendCommandLineArgument(commandLineArgument->sczUninstallArgument, pVariables, FALSE, &sczUserArgs);
980977
ExitOnFailure(hr, "Failed to get command-line argument for uninstall.");
978+
979+
hr = ExeEngineAppendCommandLineArgument(commandLineArgument->sczUninstallArgument, pVariables, TRUE, &sczUserArgsObfuscated);
980+
ExitOnFailure(hr, "Failed to get obfuscated command-line argument for uninstall.");
981981
}
982982
break;
983983

984984
case BOOTSTRAPPER_ACTION_STATE_REPAIR:
985985
if (commandLineArgument->fEscape)
986986
{
987-
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczRepairArgument, pVariables, FALSE, &sczUserArgsEscaped);
987+
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczRepairArgument, pVariables, FALSE, &sczUserArgs);
988988
ExitOnFailure(hr, "Failed to get command-line argument for repair.");
989989

990-
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczRepairArgument, pVariables, TRUE, &sczUserArgsEscapedObfuscated);
990+
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczRepairArgument, pVariables, TRUE, &sczUserArgsObfuscated);
991991
ExitOnFailure(hr, "Failed to get obfuscated command-line argument for repair.");
992992
}
993993
else
994994
{
995-
hr = StrAllocConcat(&sczUnformattedUserArgs, commandLineArgument->sczRepairArgument, 0);
995+
hr = ExeEngineAppendCommandLineArgument(commandLineArgument->sczRepairArgument, pVariables, FALSE, &sczUserArgs);
996996
ExitOnFailure(hr, "Failed to get command-line argument for repair.");
997+
998+
hr = ExeEngineAppendCommandLineArgument(commandLineArgument->sczRepairArgument, pVariables, TRUE, &sczUserArgsObfuscated);
999+
ExitOnFailure(hr, "Failed to get obfuscated command-line argument for repair.");
9971000
}
9981001
break;
9991002

@@ -1080,33 +1083,12 @@ static HRESULT ExecuteBundle(
10801083
ExitOnFailure(hr, "Failed to append %ls", BURN_COMMANDLINE_SWITCH_FILEHANDLE_SELF);
10811084

10821085
// build user args
1083-
if (sczUnformattedUserArgs && *sczUnformattedUserArgs)
1086+
if (sczUserArgsObfuscated && *sczUserArgsObfuscated)
10841087
{
1085-
hr = VariableFormatString(pVariables, sczUnformattedUserArgs, &sczUserArgs, NULL);
1086-
ExitOnFailure(hr, "Failed to format argument string.");
1087-
1088-
hr = VariableFormatStringObfuscated(pVariables, sczUnformattedUserArgs, &sczUserArgsObfuscated, NULL);
1089-
ExitOnFailure(hr, "Failed to format obfuscated argument string.");
1090-
10911088
hr = StrAllocFormatted(&sczCommandObfuscated, L"%ls %ls", sczBaseCommand, sczUserArgsObfuscated);
10921089
ExitOnFailure(hr, "Failed to allocate obfuscated bundle command.");
10931090
}
10941091

1095-
if (sczUserArgsEscaped && *sczUserArgsEscaped)
1096-
{
1097-
hr = StrAllocConcat(&sczUserArgs, L" ", 0);
1098-
ExitOnFailure(hr, "Failed to concat exe command.");
1099-
1100-
hr = StrAllocConcat(&sczUserArgs, sczUserArgsEscaped, 0);
1101-
ExitOnFailure(hr, "Failed to concat exe command.");
1102-
1103-
hr = StrAllocConcat(&sczCommandObfuscated, L" ", 0);
1104-
ExitOnFailure(hr, "Failed to concat obfuscated exe command.");
1105-
1106-
hr = StrAllocConcat(&sczCommandObfuscated, sczUserArgsEscapedObfuscated, 0);
1107-
ExitOnFailure(hr, "Failed to concat obfuscated exe command.");
1108-
}
1109-
11101092
// Append logging to command line if it doesn't contain '-log'
11111093
CoreAppendLogToCommandLine(&sczBaseCommand, &sczCommandObfuscated, fRollback, pVariables, pPackage);
11121094

@@ -1144,11 +1126,8 @@ static HRESULT ExecuteBundle(
11441126
ReleaseStr(sczCachedDirectory);
11451127
ReleaseStr(sczExecutablePath);
11461128
ReleaseStr(sczBaseCommand);
1147-
ReleaseStr(sczUnformattedUserArgs);
11481129
StrSecureZeroFreeString(sczUserArgs);
11491130
ReleaseStr(sczUserArgsObfuscated);
1150-
ReleaseStr(sczUserArgsEscapedObfuscated);
1151-
ReleaseStr(sczUserArgsEscaped);
11521131
ReleaseStr(sczCommandObfuscated);
11531132
ReleaseStr(sczArpUninstallString);
11541133

‎src/burn/engine/exeengine.cpp‎

Lines changed: 63 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -473,10 +473,7 @@ extern "C" HRESULT ExeEngineExecutePackage(
473473
LPWSTR sczCachedDirectory = NULL;
474474
LPWSTR sczExecutablePath = NULL;
475475
LPWSTR sczBaseCommand = NULL;
476-
LPWSTR sczUnformattedUserArgs = NULL;
477476
LPWSTR sczUserArgs = NULL;
478-
LPWSTR sczUserArgsEscaped = NULL;
479-
LPWSTR sczUserArgsEscapedObfuscated = NULL;
480477
LPWSTR sczUserArgsObfuscated = NULL;
481478
LPWSTR sczCommandObfuscated = NULL;
482479
LPWSTR sczArpUninstallString = NULL;
@@ -589,8 +586,14 @@ extern "C" HRESULT ExeEngineExecutePackage(
589586
}
590587

591588
// now add optional arguments
592-
hr = StrAllocString(&sczUnformattedUserArgs, wzArguments && *wzArguments ? wzArguments : L"", 0);
593-
ExitOnFailure(hr, "Failed to copy package arguments.");
589+
if (wzArguments && *wzArguments)
590+
{
591+
hr = ExeEngineAppendCommandLineArgument(wzArguments, pVariables, FALSE, &sczUserArgs);
592+
ExitOnFailure(hr, "Failed to get command-line arguments.");
593+
594+
hr = ExeEngineAppendCommandLineArgument(wzArguments, pVariables, TRUE, &sczUserArgsObfuscated);
595+
ExitOnFailure(hr, "Failed to get obfuscated command-line arguments.");
596+
}
594597

595598
for (DWORD i = 0; i < pPackage->Exe.cCommandLineArguments; ++i)
596599
{
@@ -605,59 +608,62 @@ extern "C" HRESULT ExeEngineExecutePackage(
605608

606609
if (fCondition)
607610
{
608-
if (sczUnformattedUserArgs && !commandLineArgument->fEscape)
609-
{
610-
hr = StrAllocConcat(&sczUnformattedUserArgs, L" ", 0);
611-
ExitOnFailure(hr, "Failed to separate command-line arguments.");
612-
}
613-
614611
switch (pExecuteAction->exePackage.action)
615612
{
616613
case BOOTSTRAPPER_ACTION_STATE_INSTALL:
617614
if (commandLineArgument->fEscape)
618615
{
619-
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczInstallArgument, pVariables, FALSE, &sczUserArgsEscaped);
616+
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczInstallArgument, pVariables, FALSE, &sczUserArgs);
620617
ExitOnFailure(hr, "Failed to get command-line argument for install.");
621618

622-
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczInstallArgument, pVariables, TRUE, &sczUserArgsEscapedObfuscated);
619+
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczInstallArgument, pVariables, TRUE, &sczUserArgsObfuscated);
623620
ExitOnFailure(hr, "Failed to get obfuscated command-line argument for install.");
624621
}
625622
else
626623
{
627-
hr = StrAllocConcat(&sczUnformattedUserArgs, commandLineArgument->sczInstallArgument, 0);
624+
hr = ExeEngineAppendCommandLineArgument(commandLineArgument->sczInstallArgument, pVariables, FALSE, &sczUserArgs);
628625
ExitOnFailure(hr, "Failed to get command-line argument for install.");
626+
627+
hr = ExeEngineAppendCommandLineArgument(commandLineArgument->sczInstallArgument, pVariables, TRUE, &sczUserArgsObfuscated);
628+
ExitOnFailure(hr, "Failed to get obfuscated command-line argument for install.");
629629
}
630630
break;
631631

632632
case BOOTSTRAPPER_ACTION_STATE_UNINSTALL:
633633
if (commandLineArgument->fEscape)
634634
{
635-
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczUninstallArgument, pVariables, FALSE, &sczUserArgsEscaped);
635+
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczUninstallArgument, pVariables, FALSE, &sczUserArgs);
636636
ExitOnFailure(hr, "Failed to get command-line argument for uninstall.");
637637

638-
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczUninstallArgument, pVariables, TRUE, &sczUserArgsEscapedObfuscated);
638+
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczUninstallArgument, pVariables, TRUE, &sczUserArgsObfuscated);
639639
ExitOnFailure(hr, "Failed to get obfuscated command-line argument for uninstall.");
640640
}
641641
else
642642
{
643-
hr = StrAllocConcat(&sczUnformattedUserArgs, commandLineArgument->sczUninstallArgument, 0);
643+
hr = ExeEngineAppendCommandLineArgument(commandLineArgument->sczUninstallArgument, pVariables, FALSE, &sczUserArgs);
644644
ExitOnFailure(hr, "Failed to get command-line argument for uninstall.");
645+
646+
hr = ExeEngineAppendCommandLineArgument(commandLineArgument->sczUninstallArgument, pVariables, TRUE, &sczUserArgsObfuscated);
647+
ExitOnFailure(hr, "Failed to get obfuscated command-line argument for uninstall.");
645648
}
646649
break;
647650

648651
case BOOTSTRAPPER_ACTION_STATE_REPAIR:
649652
if (commandLineArgument->fEscape)
650653
{
651-
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczRepairArgument, pVariables, FALSE, &sczUserArgsEscaped);
654+
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczRepairArgument, pVariables, FALSE, &sczUserArgs);
652655
ExitOnFailure(hr, "Failed to get command-line argument for repair.");
653656

654-
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczRepairArgument, pVariables, TRUE, &sczUserArgsEscapedObfuscated);
657+
hr = ExeEngineAppendEscapeCommandLineArgument(commandLineArgument->sczRepairArgument, pVariables, TRUE, &sczUserArgsObfuscated);
655658
ExitOnFailure(hr, "Failed to get obfuscated command-line argument for repair.");
656659
}
657660
else
658661
{
659-
hr = StrAllocConcat(&sczUnformattedUserArgs, commandLineArgument->sczRepairArgument, 0);
662+
hr = ExeEngineAppendCommandLineArgument(commandLineArgument->sczRepairArgument, pVariables, FALSE, &sczUserArgs);
660663
ExitOnFailure(hr, "Failed to get command-line argument for repair.");
664+
665+
hr = ExeEngineAppendCommandLineArgument(commandLineArgument->sczRepairArgument, pVariables, TRUE, &sczUserArgsObfuscated);
666+
ExitOnFailure(hr, "Failed to get obfuscated command-line argument for repair.");
661667
}
662668
break;
663669

@@ -714,33 +720,12 @@ extern "C" HRESULT ExeEngineExecutePackage(
714720
}
715721

716722
// build user args
717-
if (sczUnformattedUserArgs && *sczUnformattedUserArgs)
723+
if (sczUserArgsObfuscated && *sczUserArgsObfuscated)
718724
{
719-
hr = VariableFormatString(pVariables, sczUnformattedUserArgs, &sczUserArgs, NULL);
720-
ExitOnFailure(hr, "Failed to format argument string.");
721-
722-
hr = VariableFormatStringObfuscated(pVariables, sczUnformattedUserArgs, &sczUserArgsObfuscated, NULL);
723-
ExitOnFailure(hr, "Failed to format obfuscated argument string.");
724-
725725
hr = StrAllocFormatted(&sczCommandObfuscated, L"%ls %ls", sczBaseCommand, sczUserArgsObfuscated);
726726
ExitOnFailure(hr, "Failed to allocate obfuscated exe command.");
727727
}
728728

729-
if (sczUserArgsEscaped && *sczUserArgsEscaped)
730-
{
731-
hr = StrAllocConcat(&sczUserArgs, L" ", 0);
732-
ExitOnFailure(hr, "Failed to concat exe command.");
733-
734-
hr = StrAllocConcat(&sczUserArgs, sczUserArgsEscaped, 0);
735-
ExitOnFailure(hr, "Failed to concat exe command.");
736-
737-
hr = StrAllocConcat(&sczCommandObfuscated, L" ", 0);
738-
ExitOnFailure(hr, "Failed to concat obfuscated exe command.");
739-
740-
hr = StrAllocConcat(&sczCommandObfuscated, sczUserArgsEscapedObfuscated, 0);
741-
ExitOnFailure(hr, "Failed to concat obfuscated exe command.");
742-
}
743-
744729
// Log obfuscated command, which won't include raw hidden variable values or protocol specific arguments to avoid exposing secrets.
745730
LogId(REPORT_STANDARD, MSG_APPLYING_PACKAGE, LoggingRollbackOrExecute(fRollback), pPackage->sczId, LoggingActionStateToString(pExecuteAction->exePackage.action), sczExecutablePath, sczCommandObfuscated ? sczCommandObfuscated : sczBaseCommand);
746731

@@ -767,11 +752,8 @@ extern "C" HRESULT ExeEngineExecutePackage(
767752
ReleaseStr(sczCachedDirectory);
768753
ReleaseStr(sczExecutablePath);
769754
ReleaseStr(sczBaseCommand);
770-
ReleaseStr(sczUnformattedUserArgs);
771755
StrSecureZeroFreeString(sczUserArgs);
772756
ReleaseStr(sczUserArgsObfuscated);
773-
ReleaseStr(sczUserArgsEscapedObfuscated);
774-
ReleaseStr(sczUserArgsEscaped);
775757
ReleaseStr(sczCommandObfuscated);
776758
ReleaseStr(sczArpUninstallString);
777759

@@ -1098,6 +1080,42 @@ extern "C" HRESULT ExeEngineAppendEscapeCommandLineArgument(
10981080
return hr;
10991081
}
11001082

1083+
extern "C" HRESULT ExeEngineAppendCommandLineArgument(
1084+
__in LPCWSTR szArgument,
1085+
__in BURN_VARIABLES* pVariables,
1086+
__in BOOL fObfuscateHiddenVariables,
1087+
__deref_out_z LPWSTR* psczArguments
1088+
)
1089+
{
1090+
HRESULT hr = S_OK;
1091+
LPWSTR sz = NULL;
1092+
1093+
if (fObfuscateHiddenVariables)
1094+
{
1095+
hr = VariableFormatStringObfuscated(pVariables, szArgument, &sz, NULL);
1096+
ExitOnFailure(hr, "Failed to format argument.");
1097+
}
1098+
else
1099+
{
1100+
hr = VariableFormatString(pVariables, szArgument, &sz, NULL);
1101+
ExitOnFailure(hr, "Failed to format argument.");
1102+
}
1103+
1104+
if (*psczArguments && **psczArguments)
1105+
{
1106+
StrAllocConcat(psczArguments, L" ", 0);
1107+
ExitOnFailure(hr, "Failed to append space.");
1108+
}
1109+
1110+
hr = StrAllocConcat(psczArguments, sz, 0);
1111+
ExitOnFailure(hr, "Failed to append argument.");
1112+
1113+
LExit:
1114+
ReleaseStr(sz);
1115+
1116+
return hr;
1117+
}
1118+
11011119
extern "C" HRESULT ExeEngineHandleExitCode(
11021120
__in BURN_EXE_EXIT_CODE* rgCustomExitCodes,
11031121
__in DWORD cCustomExitCodes,

‎src/burn/engine/exeengine.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ HRESULT ExeEngineAppendEscapeCommandLineArgument(
7272
__in BOOL fObfuscateHiddenVariables,
7373
__deref_out_z LPWSTR* psczEscapedArguments
7474
);
75+
HRESULT ExeEngineAppendCommandLineArgument(
76+
__in LPCWSTR szArgument,
77+
__in BURN_VARIABLES* pVariables,
78+
__in BOOL fObfuscateHiddenVariables,
79+
__deref_out_z LPWSTR* psczArguments
80+
);
7581
HRESULT ExeEngineHandleExitCode(
7682
__in BURN_EXE_EXIT_CODE* rgCustomExitCodes,
7783
__in DWORD cCustomExitCodes,

‎src/test/burn/TestData/BasicFunctionalityTests/BundleE/BundleE.wxs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
<!-- Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information. -->
22

33

4-
<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
4+
<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs" xmlns:bal="http://wixtoolset.org/schemas/v4/wxs/bal">
55
<Fragment>
6+
7+
<Variable Name="NotInstallFolder" Value="0" Type="formatted" Persisted="no" bal:Overridable="yes"/>
8+
<Variable Name="InstallFolder" Value="1" Type="formatted" Persisted="no" bal:Overridable="yes"/>
9+
<Variable Name="InstallFolder2" Value="2" Type="formatted" Persisted="no" bal:Overridable="yes"/>
10+
<Variable Name="UnquotedInstallFolder" Value="4" Type="formatted" Persisted="no" bal:Overridable="yes"/>
11+
612
<PackageGroup Id="BundlePackages">
713
<PackageGroupRef Id="PackageA"/>
814
<PackageGroupRef Id="PackageA_x64"/>

0 commit comments

Comments
 (0)