From e29c25090e26c8cca52232d580528840d1161b73 Mon Sep 17 00:00:00 2001
From: Bob Arnson <bob@firegiant.com>
Date: Fri, 8 Nov 2019 14:54:15 -0500
Subject: Ensure upgrade action properties are secure.

---
 .../Bind/CreateSpecialPropertiesCommand.cs          |  4 ++++
 src/WixToolset.Core/Compiler_2.cs                   |  3 ---
 .../WixToolsetTest.CoreIntegration/MsiFixture.cs    | 21 ++++++++++++++++-----
 .../TestData/ManualUpgrade/Package.wxs              |  1 +
 4 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/src/WixToolset.Core.WindowsInstaller/Bind/CreateSpecialPropertiesCommand.cs b/src/WixToolset.Core.WindowsInstaller/Bind/CreateSpecialPropertiesCommand.cs
index 8f769904..0d165f80 100644
--- a/src/WixToolset.Core.WindowsInstaller/Bind/CreateSpecialPropertiesCommand.cs
+++ b/src/WixToolset.Core.WindowsInstaller/Bind/CreateSpecialPropertiesCommand.cs
@@ -51,6 +51,10 @@ namespace WixToolset.Core.WindowsInstaller.Bind
                 .Select(ca => ca.Id.Id);
             hiddenProperties.UnionWith(hideTargetCustomActions);
 
+            // Ensure upgrade action properties are secure.
+            var actionProperties = this.Section.Tuples.OfType<UpgradeTuple>().Select(u => u.ActionProperty);
+            secureProperties.UnionWith(actionProperties);
+
             if (0 < adminProperties.Count)
             {
                 var tuple = new PropertyTuple(null, new Identifier(AccessModifier.Private, "AdminProperties"));
diff --git a/src/WixToolset.Core/Compiler_2.cs b/src/WixToolset.Core/Compiler_2.cs
index 2f578e61..3e50a32d 100644
--- a/src/WixToolset.Core/Compiler_2.cs
+++ b/src/WixToolset.Core/Compiler_2.cs
@@ -4919,9 +4919,6 @@ namespace WixToolset.Core
 
                 this.Core.AddTuple(tuple);
 
-                // Ensure the action property is secure.
-                this.AddWixPropertyRow(sourceLineNumbers, new Identifier(AccessModifier.Private, actionProperty), false, true, false);
-
                 // Ensure that RemoveExistingProducts is authored in InstallExecuteSequence
                 // if at least one row in Upgrade table lacks the OnlyDetect attribute.
                 if (!onlyDetect)
diff --git a/src/test/WixToolsetTest.CoreIntegration/MsiFixture.cs b/src/test/WixToolsetTest.CoreIntegration/MsiFixture.cs
index 21b6e9ce..4d1e35f9 100644
--- a/src/test/WixToolsetTest.CoreIntegration/MsiFixture.cs
+++ b/src/test/WixToolsetTest.CoreIntegration/MsiFixture.cs
@@ -372,16 +372,27 @@ namespace WixToolsetTest.CoreIntegration
 
                 Assert.Equal(0, result);
 
+                var pdbPath = Path.Combine(intermediateFolder, @"bin\test.wixpdb");
                 Assert.True(File.Exists(Path.Combine(intermediateFolder, @"bin\test.msi")));
-                Assert.True(File.Exists(Path.Combine(intermediateFolder, @"bin\test.wixpdb")));
+                Assert.True(File.Exists(pdbPath));
                 Assert.True(File.Exists(Path.Combine(intermediateFolder, @"bin\MsiPackage\test.txt")));
 
-                var intermediate = Intermediate.Load(Path.Combine(intermediateFolder, @"bin\test.wixpdb"));
+                var intermediate = Intermediate.Load(pdbPath);
                 var section = intermediate.Sections.Single();
 
-                var fileTuple = section.Tuples.OfType<FileTuple>().Single();
-                Assert.Equal(Path.Combine(folder, @"data\test.txt"), fileTuple[FileTupleFields.Source].AsPath().Path);
-                Assert.Equal(@"test.txt", fileTuple[FileTupleFields.Source].PreviousValue.AsPath().Path);
+                var upgradeTuple = section.Tuples.OfType<UpgradeTuple>().Single();
+                Assert.False(upgradeTuple.ExcludeLanguages);
+                Assert.True(upgradeTuple.IgnoreRemoveFailures);
+                Assert.False(upgradeTuple.VersionMaxInclusive);
+                Assert.True(upgradeTuple.VersionMinInclusive);
+                Assert.Equal("13.0.0", upgradeTuple.VersionMax);
+                Assert.Equal("12.0.0", upgradeTuple.VersionMin);
+                Assert.False(upgradeTuple.OnlyDetect);
+                Assert.Equal("BLAHBLAHBLAH", upgradeTuple.ActionProperty);
+
+                var pdb = WindowsInstallerData.Load(pdbPath, suppressVersionCheck: false);
+                var secureProperties = pdb.Tables["Property"].Rows.Where(row => row.GetKey() == "SecureCustomProperties").Single();
+                Assert.Contains("BLAHBLAHBLAH", secureProperties.FieldAsString(1));
             }
         }
 
diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/ManualUpgrade/Package.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/ManualUpgrade/Package.wxs
index d674eb59..38125b57 100644
--- a/src/test/WixToolsetTest.CoreIntegration/TestData/ManualUpgrade/Package.wxs
+++ b/src/test/WixToolsetTest.CoreIntegration/TestData/ManualUpgrade/Package.wxs
@@ -6,6 +6,7 @@
     <Upgrade Id="01120000-00E0-0000-0000-0000000FF1CE">
       <UpgradeVersion ExcludeLanguages="no" IgnoreRemoveFailure="yes" IncludeMaximum="no" IncludeMinimum="yes" Maximum="13.0.0" Minimum="12.0.0" OnlyDetect="no" Property="BLAHBLAHBLAH" />
     </Upgrade>
+    <!--<Property Id="BLAHBLAHBLAH" Secure="yes" />-->
 
     <InstallExecuteSequence>
       <RemoveExistingProducts After="InstallValidate" />
-- 
cgit v1.2.3-55-g6feb