From f8d1a711625599ab94630242976a740a4790915e Mon Sep 17 00:00:00 2001 From: Bob Arnson Date: Fri, 27 Sep 2024 22:09:24 -0400 Subject: [PATCH] Drop CA path validation in WixUI by default. Add `WixUI/ExtendedPathValidation="yes"` to opt-in. Removed the WIXUI_DONTVALIDATEPATH runtime opt-out. Fixes https://github.com/wixtoolset/issues/issues/8718 Relies on https://github.com/wixtoolset/wix/pull/563 --- src/ext/UI/ca/DriveCheck.cpp | 42 ++++++++--------- .../TestData/InstallDir_NoLicense/Package.wxs | 15 +++--- .../InstallDir_SpecialDlg/Package.wxs | 11 ++--- .../TestData/WixUI_Advanced/Package.wxs | 4 +- .../TestData/WixUI_InstallDir/Package.wxs | 4 +- .../TestData/WixUI_Mondo/Package.wxs | 6 +-- .../WixToolsetTest.UI/UIExtensionFixture.cs | 46 ++++--------------- src/ext/UI/wixext/UICompiler.cs | 14 +++++- src/ext/UI/wixlib/BrowseDlg.wxs | 5 +- src/ext/UI/wixlib/WixUI_Advanced.wxs | 29 +++++++++--- src/ext/UI/wixlib/WixUI_InstallDir.wxs | 30 +++++++++--- src/ext/UI/wixlib/WixUI_Mondo.wxs | 16 ++++++- 12 files changed, 126 insertions(+), 96 deletions(-) diff --git a/src/ext/UI/ca/DriveCheck.cpp b/src/ext/UI/ca/DriveCheck.cpp index fafd73c27..75e6ffd1f 100644 --- a/src/ext/UI/ca/DriveCheck.cpp +++ b/src/ext/UI/ca/DriveCheck.cpp @@ -12,21 +12,21 @@ static HRESULT PathIsRemovable(__in LPCWSTR pTargetFolder, __inout BOOL* fPathRe UINT __stdcall ValidatePath(MSIHANDLE hInstall) { HRESULT hr = S_OK; - + LPWSTR pwszWixUIDir = NULL; LPWSTR pwszInstallPath = NULL; BOOL fInstallPathIsRemote = TRUE; BOOL fInstallPathIsRemoveable = TRUE; - + hr = WcaInitialize(hInstall, "ValidatePath"); ExitOnFailure(hr, "failed to initialize"); hr = WcaGetProperty(L"WIXUI_INSTALLDIR", &pwszWixUIDir); ExitOnFailure(hr, "failed to get WixUI Installation Directory"); - + hr = WcaGetProperty(pwszWixUIDir, &pwszInstallPath); ExitOnFailure(hr, "failed to get Installation Directory"); - + hr = PathIsRemote(pwszInstallPath, &fInstallPathIsRemote); if (FAILED(hr)) { @@ -34,7 +34,7 @@ UINT __stdcall ValidatePath(MSIHANDLE hInstall) //reset HR, as we need to continue and find out if is a UNC path hr = S_OK; } - + hr = PathIsRemovable(pwszInstallPath, &fInstallPathIsRemoveable); if (FAILED(hr)) { @@ -45,7 +45,7 @@ UINT __stdcall ValidatePath(MSIHANDLE hInstall) // If the path does not point to a network drive, mapped drive, or removable drive, // then set WIXUI_INSTALLDIR_VALID to "1" otherwise set it to 0 - BOOL fInstallPathIsUnc = PathIsUNCW(pwszInstallPath); + BOOL fInstallPathIsUnc = ::PathIsUNCW(pwszInstallPath); if (!fInstallPathIsUnc && !fInstallPathIsRemote && !fInstallPathIsRemoveable) { // path is valid @@ -61,7 +61,7 @@ UINT __stdcall ValidatePath(MSIHANDLE hInstall) hr = WcaSetProperty(L"WIXUI_INSTALLDIR_VALID", L"0"); ExitOnFailure(hr, "failed to set WIXUI_INSTALLDIR_VALID"); } - + LExit: ReleaseStr(pwszInstallPath); ReleaseStr(pwszWixUIDir); @@ -79,22 +79,22 @@ static HRESULT PathIsRemote(__in LPCWSTR pTargetFolder, __inout BOOL* fPathRemot LPWSTR pStrippedTargetFolder = NULL; hr = StrAllocString(&pStrippedTargetFolder, pTargetFolder, 0); - + // Terminate the path at the root - if(!::PathStripToRootW(pStrippedTargetFolder)) + if (!::PathStripToRootW(pStrippedTargetFolder)) { hr = HRESULT_FROM_WIN32(ERROR_INVALID_DRIVE); - ExitOnFailure(hr, "failed to parse target folder"); + ExitOnFailure(hr, "failed to parse target folder"); } - - UINT uResult = GetDriveTypeW(pStrippedTargetFolder); - - *fPathRemote = (DRIVE_REMOTE == uResult) ; + + UINT uResult = ::GetDriveTypeW(pStrippedTargetFolder); + + *fPathRemote = (DRIVE_REMOTE == uResult); LExit: ReleaseStr(pStrippedTargetFolder); - return hr; + return hr; } /******************************************************************** @@ -107,16 +107,16 @@ static HRESULT PathIsRemovable(__in LPCWSTR pTargetFolder, __inout BOOL* fPathRe LPWSTR pStrippedTargetFolder = NULL; hr = StrAllocString(&pStrippedTargetFolder, pTargetFolder, 0); - + // Terminate the path at the root - if(!::PathStripToRootW(pStrippedTargetFolder)) + if (!::PathStripToRootW(pStrippedTargetFolder)) { hr = HRESULT_FROM_WIN32(ERROR_INVALID_DRIVE); - ExitOnFailure(hr, "failed to parse target folder"); + ExitOnFailure(hr, "failed to parse target folder"); } - - UINT uResult = GetDriveTypeW(pStrippedTargetFolder); - + + UINT uResult = ::GetDriveTypeW(pStrippedTargetFolder); + *fPathRemovable = ((DRIVE_CDROM == uResult) || (DRIVE_REMOVABLE == uResult) || (DRIVE_RAMDISK == uResult) || (DRIVE_UNKNOWN == uResult)); LExit: diff --git a/src/ext/UI/test/WixToolsetTest.UI/TestData/InstallDir_NoLicense/Package.wxs b/src/ext/UI/test/WixToolsetTest.UI/TestData/InstallDir_NoLicense/Package.wxs index 86b7453e4..0c9fd195b 100644 --- a/src/ext/UI/test/WixToolsetTest.UI/TestData/InstallDir_NoLicense/Package.wxs +++ b/src/ext/UI/test/WixToolsetTest.UI/TestData/InstallDir_NoLicense/Package.wxs @@ -24,8 +24,8 @@ - - + + @@ -50,8 +50,6 @@ - - @@ -59,11 +57,14 @@ - - - + + + + + + diff --git a/src/ext/UI/test/WixToolsetTest.UI/TestData/InstallDir_SpecialDlg/Package.wxs b/src/ext/UI/test/WixToolsetTest.UI/TestData/InstallDir_SpecialDlg/Package.wxs index eeb448336..de1792405 100644 --- a/src/ext/UI/test/WixToolsetTest.UI/TestData/InstallDir_SpecialDlg/Package.wxs +++ b/src/ext/UI/test/WixToolsetTest.UI/TestData/InstallDir_SpecialDlg/Package.wxs @@ -42,10 +42,7 @@ - - - - + @@ -70,7 +67,7 @@ - + @@ -85,8 +82,8 @@ - - + + diff --git a/src/ext/UI/test/WixToolsetTest.UI/TestData/WixUI_Advanced/Package.wxs b/src/ext/UI/test/WixToolsetTest.UI/TestData/WixUI_Advanced/Package.wxs index c22328e69..095a4546d 100644 --- a/src/ext/UI/test/WixToolsetTest.UI/TestData/WixUI_Advanced/Package.wxs +++ b/src/ext/UI/test/WixToolsetTest.UI/TestData/WixUI_Advanced/Package.wxs @@ -1,12 +1,12 @@ - + - + diff --git a/src/ext/UI/test/WixToolsetTest.UI/TestData/WixUI_InstallDir/Package.wxs b/src/ext/UI/test/WixToolsetTest.UI/TestData/WixUI_InstallDir/Package.wxs index 68e1177db..8a6556a9b 100644 --- a/src/ext/UI/test/WixToolsetTest.UI/TestData/WixUI_InstallDir/Package.wxs +++ b/src/ext/UI/test/WixToolsetTest.UI/TestData/WixUI_InstallDir/Package.wxs @@ -1,5 +1,5 @@ - + @@ -12,7 +12,7 @@ - + diff --git a/src/ext/UI/test/WixToolsetTest.UI/TestData/WixUI_Mondo/Package.wxs b/src/ext/UI/test/WixToolsetTest.UI/TestData/WixUI_Mondo/Package.wxs index 5bb7fa1a8..24f18c1eb 100644 --- a/src/ext/UI/test/WixToolsetTest.UI/TestData/WixUI_Mondo/Package.wxs +++ b/src/ext/UI/test/WixToolsetTest.UI/TestData/WixUI_Mondo/Package.wxs @@ -1,8 +1,8 @@ - + - + @@ -12,7 +12,7 @@ - + diff --git a/src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs b/src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs index eb5e36875..bb20f0875 100644 --- a/src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs +++ b/src/ext/UI/test/WixToolsetTest.UI/UIExtensionFixture.cs @@ -29,7 +29,6 @@ public void CanBuildUsingWixUIAdvanced() "Binary:WixUI_Bmp_Up\t[Binary data]", "Binary:WixUI_Ico_Exclam\t[Binary data]", "Binary:WixUI_Ico_Info\t[Binary data]", - "Binary:WixUiCa_X86\t[Binary data]", }, results.Where(r => r.StartsWith("Binary:")).ToArray()); WixAssert.CompareLineByLine(new[] { @@ -37,13 +36,8 @@ public void CanBuildUsingWixUIAdvanced() "CustomAction:WixSetDefaultPerUserFolder\t51\tWixPerUserFolder\t[LocalAppDataFolder]Apps\\[ApplicationFolderName]\t", "CustomAction:WixSetPerMachineFolder\t51\tAPPLICATIONFOLDER\t[WixPerMachineFolder]\t", "CustomAction:WixSetPerUserFolder\t51\tAPPLICATIONFOLDER\t[WixPerUserFolder]\t", - "CustomAction:WixUIValidatePath_X86\t65\tWixUiCa_X86\tValidatePath\t", }, results.Where(r => r.StartsWith("CustomAction:")).ToArray()); - WixAssert.CompareLineByLine(new[] - { - "ControlEvent:BrowseDlg\tOK\tDoAction\tWixUIValidatePath_X86\tNOT WIXUI_DONTVALIDATEPATH\t1", - "ControlEvent:InstallDirDlg\tNext\tDoAction\tWixUIValidatePath_X86\tNOT WIXUI_DONTVALIDATEPATH\t2", - }, results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).ToArray()); + Assert.Empty(results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).ToArray()); WixAssert.CompareLineByLine(new[] { "InstallUISequence:AdvancedWelcomeEulaDlg\tNOT Installed\t1297", @@ -68,7 +62,6 @@ public void CanBuildUsingWixUIAdvancedX64() "Binary:WixUI_Bmp_Up\t[Binary data]", "Binary:WixUI_Ico_Exclam\t[Binary data]", "Binary:WixUI_Ico_Info\t[Binary data]", - "Binary:WixUiCa_X64\t[Binary data]", }, results.Where(r => r.StartsWith("Binary:")).ToArray()); WixAssert.CompareLineByLine(new[] { @@ -76,13 +69,8 @@ public void CanBuildUsingWixUIAdvancedX64() "CustomAction:WixSetDefaultPerUserFolder\t51\tWixPerUserFolder\t[LocalAppDataFolder]Apps\\[ApplicationFolderName]\t", "CustomAction:WixSetPerMachineFolder\t51\tAPPLICATIONFOLDER\t[WixPerMachineFolder]\t", "CustomAction:WixSetPerUserFolder\t51\tAPPLICATIONFOLDER\t[WixPerUserFolder]\t", - "CustomAction:WixUIValidatePath_X64\t65\tWixUiCa_X64\tValidatePath\t", }, results.Where(r => r.StartsWith("CustomAction:")).ToArray()); - WixAssert.CompareLineByLine(new[] - { - "ControlEvent:BrowseDlg\tOK\tDoAction\tWixUIValidatePath_X64\tNOT WIXUI_DONTVALIDATEPATH\t1", - "ControlEvent:InstallDirDlg\tNext\tDoAction\tWixUIValidatePath_X64\tNOT WIXUI_DONTVALIDATEPATH\t2", - }, results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).ToArray()); + Assert.Empty(results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).ToArray()); } [Fact] @@ -102,7 +90,6 @@ public void CanBuildUsingWixUIAdvancedARM64() "Binary:WixUI_Bmp_Up\t[Binary data]", "Binary:WixUI_Ico_Exclam\t[Binary data]", "Binary:WixUI_Ico_Info\t[Binary data]", - "Binary:WixUiCa_A64\t[Binary data]", }, results.Where(r => r.StartsWith("Binary:")).ToArray()); WixAssert.CompareLineByLine(new[] { @@ -110,13 +97,8 @@ public void CanBuildUsingWixUIAdvancedARM64() "CustomAction:WixSetDefaultPerUserFolder\t51\tWixPerUserFolder\t[LocalAppDataFolder]Apps\\[ApplicationFolderName]\t", "CustomAction:WixSetPerMachineFolder\t51\tAPPLICATIONFOLDER\t[WixPerMachineFolder]\t", "CustomAction:WixSetPerUserFolder\t51\tAPPLICATIONFOLDER\t[WixPerUserFolder]\t", - "CustomAction:WixUIValidatePath_A64\t65\tWixUiCa_A64\tValidatePath\t", }, results.Where(r => r.StartsWith("CustomAction:")).ToArray()); - WixAssert.CompareLineByLine(new[] - { - "ControlEvent:BrowseDlg\tOK\tDoAction\tWixUIValidatePath_A64\tNOT WIXUI_DONTVALIDATEPATH\t1", - "ControlEvent:InstallDirDlg\tNext\tDoAction\tWixUIValidatePath_A64\tNOT WIXUI_DONTVALIDATEPATH\t2", - }, results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).ToArray()); + Assert.Empty(results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).ToArray()); } [Fact] @@ -180,8 +162,8 @@ public void CanBuildWithWixUIInstallDirWithCustomizedEula() }, results.Where(r => r.StartsWith("Property:WIXUI")).ToArray()); WixAssert.CompareLineByLine(new[] { - "ControlEvent:BrowseDlg\tOK\tDoAction\tWixUIValidatePath_X86\tNOT WIXUI_DONTVALIDATEPATH\t3", - "ControlEvent:InstallDirDlg\tNext\tDoAction\tWixUIValidatePath_X86\tNOT WIXUI_DONTVALIDATEPATH\t2", + "ControlEvent:BrowseDlg\tOK\tDoAction\tWixUIValidatePath_X86\t1\t1", + "ControlEvent:InstallDirDlg\tNext\tDoAction\tWixUIValidatePath_X86\t1\t1", }, results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).OrderBy(s => s).ToArray()); WixAssert.CompareLineByLine(new[] { @@ -281,7 +263,7 @@ public void CanBuildUsingWixUIMondo() }, results.Where(r => r.StartsWith("CustomAction:")).ToArray()); WixAssert.CompareLineByLine(new[] { - "ControlEvent:BrowseDlg\tOK\tDoAction\tWixUIValidatePath_X86\tNOT WIXUI_DONTVALIDATEPATH\t3", + "ControlEvent:BrowseDlg\tOK\tDoAction\tWixUIValidatePath_X86\t1\t1", }, results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).ToArray()); WixAssert.CompareLineByLine(new[] { @@ -332,8 +314,8 @@ public void CanBuildWithInstallDirAndRemovedDialog() }, results.Where(r => r.StartsWith("Property:WIXUI")).ToArray()); WixAssert.CompareLineByLine(new[] { - "ControlEvent:BrowseDlg\tOK\tDoAction\tWixUIValidatePath_X86\tNOT WIXUI_DONTVALIDATEPATH\t3", - "ControlEvent:InstallDirDlg\tNext\tDoAction\tWixUIValidatePath_X86\tNOT WIXUI_DONTVALIDATEPATH\t2", + "ControlEvent:BrowseDlg\tOK\tDoAction\tWixUIValidatePath_X86\t1\t3", + "ControlEvent:InstallDirDlg\tNext\tDoAction\tWixUIValidatePath_X86\t1\t2", }, results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).OrderBy(s => s).ToArray()); Assert.Empty(results.Where(result => result.Contains("LicenseAgreementDlg")).ToArray()); @@ -361,21 +343,13 @@ public void CanBuildWithInstallDirAndAddedDialog() "Binary:WixUI_Bmp_Up\t[Binary data]", "Binary:WixUI_Ico_Exclam\t[Binary data]", "Binary:WixUI_Ico_Info\t[Binary data]", - "Binary:WixUiCa_X64\t[Binary data]", }, results.Where(r => r.StartsWith("Binary:")).ToArray()); - WixAssert.CompareLineByLine(new[] - { - "CustomAction:WixUIValidatePath_X64\t65\tWixUiCa_X64\tValidatePath\t", - }, results.Where(r => r.StartsWith("CustomAction:")).ToArray()); + Assert.Empty(results.Where(r => r.StartsWith("CustomAction:")).ToArray()); WixAssert.CompareLineByLine(new[] { "Property:WIXUI_INSTALLDIR\tINSTALLFOLDER", }, results.Where(r => r.StartsWith("Property:WIXUI")).ToArray()); - WixAssert.CompareLineByLine(new[] - { - "ControlEvent:BrowseDlg\tOK\tDoAction\tWixUIValidatePath_X64\tNOT WIXUI_DONTVALIDATEPATH\t3", - "ControlEvent:InstallDirDlg\tNext\tDoAction\tWixUIValidatePath_X64\tNOT WIXUI_DONTVALIDATEPATH\t2", - }, results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).OrderBy(s => s).ToArray()); + Assert.Empty(results.Where(result => result.StartsWith("ControlEvent:") && result.Contains("DoAction")).OrderBy(s => s).ToArray()); WixAssert.CompareLineByLine(new[] { "InstallUISequence:WelcomeDlg\tNOT Installed OR PATCH\t1297", diff --git a/src/ext/UI/wixext/UICompiler.cs b/src/ext/UI/wixext/UICompiler.cs index 5f2751c94..57a0dc169 100644 --- a/src/ext/UI/wixext/UICompiler.cs +++ b/src/ext/UI/wixext/UICompiler.cs @@ -56,6 +56,7 @@ private void ParseWixUIElement(Intermediate intermediate, IntermediateSection se var sourceLineNumbers = this.ParseHelper.GetSourceLineNumbers(element); string id = null; string installDirectory = null; + YesNoType extendedPathValidation = YesNoType.No; foreach (var attrib in element.Attributes()) { @@ -69,6 +70,9 @@ private void ParseWixUIElement(Intermediate intermediate, IntermediateSection se case "InstallDirectory": installDirectory = this.ParseHelper.GetAttributeIdentifierValue(sourceLineNumbers, attrib); break; + case "ExtendedPathValidation": + extendedPathValidation = this.ParseHelper.GetAttributeYesNoValue(sourceLineNumbers, attrib); + break; default: this.ParseHelper.UnexpectedAttribute(element, attrib); break; @@ -87,7 +91,15 @@ private void ParseWixUIElement(Intermediate intermediate, IntermediateSection se else { var platform = this.Context.Platform == Platform.ARM64 ? "A64" : this.Context.Platform.ToString(); - this.ParseHelper.CreateSimpleReference(section, sourceLineNumbers, SymbolDefinitions.WixUI, $"{id}_{platform}"); + + if (extendedPathValidation == YesNoType.No) + { + this.ParseHelper.CreateSimpleReference(section, sourceLineNumbers, SymbolDefinitions.WixUI, $"{id}_{platform}"); + } + else + { + this.ParseHelper.CreateSimpleReference(section, sourceLineNumbers, SymbolDefinitions.WixUI, $"{id}_ExtendedPathValidation_{platform}"); + } if (installDirectory != null) { diff --git a/src/ext/UI/wixlib/BrowseDlg.wxs b/src/ext/UI/wixlib/BrowseDlg.wxs index dbb104fe1..67b078640 100644 --- a/src/ext/UI/wixlib/BrowseDlg.wxs +++ b/src/ext/UI/wixlib/BrowseDlg.wxs @@ -6,10 +6,7 @@ - - - - + diff --git a/src/ext/UI/wixlib/WixUI_Advanced.wxs b/src/ext/UI/wixlib/WixUI_Advanced.wxs index 2050dc247..35955a1a9 100644 --- a/src/ext/UI/wixlib/WixUI_Advanced.wxs +++ b/src/ext/UI/wixlib/WixUI_Advanced.wxs @@ -23,8 +23,25 @@ Todo: - - + + + + + + + + + + + + + + + + + + + @@ -76,7 +93,6 @@ Todo: - @@ -94,12 +110,13 @@ Todo: - - - + + + + diff --git a/src/ext/UI/wixlib/WixUI_InstallDir.wxs b/src/ext/UI/wixlib/WixUI_InstallDir.wxs index bbe806bde..3bbe320fa 100644 --- a/src/ext/UI/wixlib/WixUI_InstallDir.wxs +++ b/src/ext/UI/wixlib/WixUI_InstallDir.wxs @@ -24,8 +24,25 @@ Patch dialog sequence: - - + + + + + + + + + + + + + + + + + + + @@ -50,7 +67,6 @@ Patch dialog sequence: - @@ -61,11 +77,13 @@ Patch dialog sequence: - - - + + + + + diff --git a/src/ext/UI/wixlib/WixUI_Mondo.wxs b/src/ext/UI/wixlib/WixUI_Mondo.wxs index 4dd1148df..f0cb67f29 100644 --- a/src/ext/UI/wixlib/WixUI_Mondo.wxs +++ b/src/ext/UI/wixlib/WixUI_Mondo.wxs @@ -25,7 +25,18 @@ Patch dialog sequence: - + + + + + + + + + + + + @@ -66,6 +77,9 @@ Patch dialog sequence: + + +