Properly validate bundle variable names
Fixes 6743
Rob Mensching committed
Jun 15, 2022 at 13:46 UTC
744cde09b8c4a5338a99cd4af24f81459c82713b
10 files changed
+122
-25
src/wix/WixToolset.Core/Common.cs
+29
@@ -583,6 +583,29 @@ namespace WixToolset.Core
583
return true;
584
}
585
586
+ /// <summary>
587
+ /// Verifies that a value is a legal Bundle Variable/@Name.
588
+ /// </summary>
589
+ /// <param name="value">The value to verify.</param>
590
+ /// <returns>true if the value is an valid Variable/@Name; false otherwise.</returns>
591
+ public static bool IsBundleVariableName(string value)
592
+ {
593
+ if (String.IsNullOrEmpty(value))
594
+ {
595
+ return false;
596
+ }
597
+
598
+ for (var i = 0; i < value.Length; ++i)
599
+ {
600
+ if (!ValidBundleVariableNameChar(value[i], i == 0))
601
+ {
602
+ return false;
603
+ }
604
+ }
605
+
606
+ return true;
607
+ }
608
+
609
/// <summary>
610
/// Get an identifier attribute value and displays an error for an illegal identifier value.
611
/// </summary>
@@ -812,5 +835,11 @@ namespace WixToolset.Core
835
return ('A' <= c && 'Z' >= c) || ('a' <= c && 'z' >= c) || '_' == c ||
836
(!firstChar && (Char.IsDigit(c) || '.' == c));
837
}
838
+
839
+ private static bool ValidBundleVariableNameChar(char c, bool firstChar)
840
+ {
841
+ return ('A' <= c && 'Z' >= c) || ('a' <= c && 'z' >= c) || '_' == c ||
842
+ (!firstChar && Char.IsDigit(c));
843
+ }
844
}
845
}
src/wix/WixToolset.Core/CompilerCore.cs
+11
-6
@@ -737,21 +737,26 @@ namespace WixToolset.Core
737
}
738
739
/// <summary>
740
- /// Gets a Bundle variable value and displays an error for an illegal value.
740
+ /// Gets a bundle variable value and displays an error for an illegal value.
741
/// </summary>
742
/// <param name="sourceLineNumbers">Source line information about the owner element.</param>
743
/// <param name="attribute">The attribute containing the value to get.</param>
744
/// <returns>The attribute's value.</returns>
745
- public string GetAttributeBundleVariableValue(SourceLineNumber sourceLineNumbers, XAttribute attribute)
745
+ public Identifier GetAttributeBundleVariableNameIdentifier(SourceLineNumber sourceLineNumbers, XAttribute attribute)
746
{
747
- string value = this.GetAttributeValue(sourceLineNumbers, attribute);
747
+ var variableName = this.GetAttributeIdentifier(sourceLineNumbers, attribute);
748
749
- if (!String.IsNullOrEmpty(value))
749
+ if (!String.IsNullOrEmpty(variableName?.Id))
750
{
751
- this.bundleValidator.ValidateBundleVariableName(sourceLineNumbers, attribute.Parent.Name.LocalName, attribute.Name.LocalName, value);
751
+ this.bundleValidator.ValidateBundleVariableName(sourceLineNumbers, attribute.Parent.Name.LocalName, attribute.Name.LocalName, variableName.Id);
752
+
753
+ if (variableName.Id.StartsWith("Wix", StringComparison.OrdinalIgnoreCase))
754
+ {
755
+ this.messaging.Write(ErrorMessages.ReservedNamespaceViolation(sourceLineNumbers, attribute.Parent.Name.LocalName, attribute.Name.LocalName, "Wix"));
756
+ }
757
}
758
754
- return value;
759
+ return variableName;
760
}
761
762
/// <summary>
src/wix/WixToolset.Core/CompilerErrors.cs
+6
@@ -16,6 +16,11 @@ namespace WixToolset.Core
16
return Message(sourceLineNumbers, Ids.ReservedValue, "The {0}/@{1} attribute value '{2}' is reserved and cannot be used here. Please choose a different value.", elementName, attributeName, attributeValue);
17
}
18
19
+ public static Message IllegalBundleVariableName(SourceLineNumber sourceLineNumbers, string elementName,string attributeName, string value)
20
+ {
21
+ return Message(sourceLineNumbers, Ids.IllegalBundleVariableName, "The {0}/@{1} attribute's value, '{2}', is not a legal bundle variable name. Identifiers may contain ASCII characters A-Z, a-z, digits, or underscores (_). Every identifier must begin with either a letter or an underscore.", elementName, attributeName, value);
22
+ }
23
+
24
public static Message IllegalName(SourceLineNumber sourceLineNumbers, string parentElement, string name)
25
{
26
return Message(sourceLineNumbers, Ids.IllegalName, "The Tag/@Name attribute value, '{1}', contains invalid filename identifiers. The Tag/@Name may have defaulted from the {0}/@Name attrbute. If so, use the Tag/@Name attribute to provide a valid filename. Any character except for the follow may be used: \\ ? | > < : / * \".", parentElement, name);
@@ -38,6 +43,7 @@ namespace WixToolset.Core
43
44
IllegalName = 6601,
45
ExampleRegid = 6602,
46
+ IllegalBundleVariableName = 6603,
47
} // 5400-5499 and 6600-6699 were the ranges for Dependency and Tag which are now in Core between CompilerWarnings and CompilerErrors.
48
}
49
}
src/wix/WixToolset.Core/Compiler_Bundle.cs
+3
-7
@@ -3659,7 +3659,7 @@ namespace WixToolset.Core
3659
{
3660
var sourceLineNumbers = Preprocessor.GetSourceLineNumbers(node);
3661
var hidden = false;
3662
- string name = null;
3662
+ Identifier name = null;
3663
var persisted = false;
3664
string value = null;
3665
string typeValue = null;
@@ -3677,7 +3677,7 @@ namespace WixToolset.Core
3677
}
3678
break;
3679
case "Name":
3680
- name = this.Core.GetAttributeBundleVariableValue(sourceLineNumbers, attrib);
3680
+ name = this.Core.GetAttributeBundleVariableNameIdentifier(sourceLineNumbers, attrib);
3681
break;
3682
case "Persisted":
3683
if (YesNoType.Yes == this.Core.GetAttributeYesNoValue(sourceLineNumbers, attrib))
@@ -3706,10 +3706,6 @@ namespace WixToolset.Core
3706
{
3707
this.Core.Write(ErrorMessages.ExpectedAttribute(sourceLineNumbers, node.Name.LocalName, "Name"));
3708
}
3709
- else if (name.StartsWith("Wix", StringComparison.OrdinalIgnoreCase))
3710
- {
3711
- this.Core.Write(ErrorMessages.ReservedNamespaceViolation(sourceLineNumbers, node.Name.LocalName, "Name", "Wix"));
3712
- }
3709
3710
if (hidden && persisted)
3711
{
@@ -3722,7 +3718,7 @@ namespace WixToolset.Core
3718
3719
if (!this.Core.EncounteredError)
3720
{
3725
- this.Core.AddSymbol(new WixBundleVariableSymbol(sourceLineNumbers, new Identifier(AccessModifier.Section, name))
3721
+ this.Core.AddSymbol(new WixBundleVariableSymbol(sourceLineNumbers, name)
3722
{
3723
Value = value,
3724
Type = type,
src/wix/WixToolset.Core/ExtensibilityServices/BundleValidator.cs
+6
@@ -153,6 +153,12 @@ namespace WixToolset.Core.ExtensibilityServices
153
154
return false;
155
}
156
+ else if (!Common.IsBundleVariableName(variableName))
157
+ {
158
+ this.Messaging.Write(CompilerErrors.IllegalBundleVariableName(sourceLineNumbers, elementName, attributeName, variableName));
159
+
160
+ return false;
161
+ }
162
else if (BuiltinBundleVariables.Contains(variableName))
163
{
164
var illegalValues = CreateValueList(ValueListKind.Or, BuiltinBundleVariables);
src/wix/test/WixToolsetTest.CoreIntegration/BadInputFixture.cs
+38
-5
@@ -154,6 +154,42 @@ namespace WixToolsetTest.CoreIntegration
154
}
155
}
156
157
+ [Fact]
158
+ public void CannotBuildBundleWithLocVariableNames()
159
+ {
160
+ var folder = TestData.Get(@"TestData");
161
+
162
+ using (var fs = new DisposableFileSystem())
163
+ {
164
+ var baseFolder = fs.GetFolder();
165
+ var intermediateFolder = Path.Combine(baseFolder, "obj");
166
+
167
+ var result = WixRunner.Execute(new[]
168
+ {
169
+ "build",
170
+ Path.Combine(folder, "BundleWithInvalid", "BundleWithInvalidLocVariableNames.wxs"),
171
+ "-loc", Path.Combine(folder, "BundleWithInvalid", "BundleWithInvalidLocValues.wxl"),
172
+ "-bindpath", Path.Combine(folder, ".Data"),
173
+ "-bindpath", Path.Combine(folder, "DecompileSingleFileCompressed"),
174
+ "-bindpath", Path.Combine(folder, "SimpleBundle", "data"),
175
+ "-intermediateFolder", intermediateFolder,
176
+ "-o", Path.Combine(baseFolder, @"bin\test.exe")
177
+ });
178
+
179
+ var messages = result.Messages.Select(m => m.ToString()).ToList();
180
+ messages.Sort();
181
+
182
+ WixAssert.CompareLineByLine(new[]
183
+ {
184
+ "The SetVariable/@Variable attribute's value, '!(loc.BuiltinBurnVariableName)', is not a legal bundle variable name. Identifiers may contain ASCII characters A-Z, a-z, digits, or underscores (_). Every identifier must begin with either a letter or an underscore.",
185
+ "The Variable/@Name attribute was not found; it is required.",
186
+ "The Variable/@Name attribute's value, '!(loc.BuiltinBurnVariableName)', is not a legal identifier. Identifiers may contain ASCII characters A-Z, a-z, digits, underscores (_), or periods (.). Every identifier must begin with either a letter or an underscore.",
187
+ }, messages.ToArray());
188
+
189
+ Assert.Equal(6603, result.ExitCode);
190
+ }
191
+ }
192
+
193
[Fact]
194
public void GuardsAgainstVariousBundleValuesFromLoc()
195
{
@@ -176,23 +212,20 @@ namespace WixToolsetTest.CoreIntegration
212
"-o", Path.Combine(baseFolder, @"bin\test.exe")
213
});
214
179
- Assert.InRange(result.ExitCode, 2, Int32.MaxValue);
180
-
215
var messages = result.Messages.Select(m => m.ToString()).ToList();
216
messages.Sort();
217
218
WixAssert.CompareLineByLine(new[]
219
{
186
- "*Search/@Condition contains the built-in Variable 'WixBundleAction', which is not available when it is evaluated. (Unavailable Variables are: 'WixBundleAction'.). Rewrite the condition to avoid Variables that are never valid during its evaluation.",
220
"Bundle/@Condition contains the built-in Variable 'WixBundleInstalled', which is not available when it is evaluated. (Unavailable Variables are: 'RebootPending', 'WixBundleAction', or 'WixBundleInstalled'.). Rewrite the condition to avoid Variables that are never valid during its evaluation.",
221
"ExePackage/@DetectCondition contains the built-in Variable 'WixBundleAction', which is not available when it is evaluated. (Unavailable Variables are: 'WixBundleAction'.). Rewrite the condition to avoid Variables that are never valid during its evaluation.",
189
- "The *Search/@Variable attribute's value, 'WixBundleInstalled', is one of the illegal options: 'AdminToolsFolder', 'AppDataFolder', 'CommonAppDataFolder', 'CommonFiles64Folder', 'CommonFilesFolder', 'CompatibilityMode', 'Date', 'DesktopFolder', 'FavoritesFolder', 'FontsFolder', 'InstallerName', 'InstallerVersion', 'LocalAppDataFolder', 'LogonUser', 'MyPicturesFolder', 'NativeMachine', 'NTProductType', 'NTSuiteBackOffice', 'NTSuiteDataCenter', 'NTSuiteEnterprise', 'NTSuitePersonal', 'NTSuiteSmallBusiness', 'NTSuiteSmallBusinessRestricted', 'NTSuiteWebServer', 'PersonalFolder', 'Privileged', 'ProgramFiles64Folder', 'ProgramFiles6432Folder', 'ProgramFilesFolder', 'ProgramMenuFolder', 'RebootPending', 'SendToFolder', 'ServicePackLevel', 'StartMenuFolder', 'StartupFolder', 'System64Folder', 'SystemFolder', 'TempFolder', 'TemplateFolder', 'TerminalServer', 'UserLanguageID', 'UserUILanguageID', 'VersionMsi', 'VersionNT', 'VersionNT64', 'WindowsFolder', 'WindowsVolume', 'WixBundleAction', 'WixBundleCommandLineAction', 'WixBundleForcedRestartPackage', 'WixBundleElevated', 'WixBundleInstalled', 'WixBundleProviderKey', 'WixBundleTag', or 'WixBundleVersion'.",
222
"The CommandLine/@Condition attribute's value '=' is not a valid bundle condition.",
223
"The MsiPackage/@InstallCondition attribute's value '=' is not a valid bundle condition.",
224
"The MsiProperty/@Condition attribute's value '=' is not a valid bundle condition.",
193
- //"The Variable/@Name attribute's value, 'WixBundleInstalled', is one of the illegal options: 'AdminToolsFolder', 'AppDataFolder', 'CommonAppDataFolder', 'CommonFiles64Folder', 'CommonFilesFolder', 'CompatibilityMode', 'Date', 'DesktopFolder', 'FavoritesFolder', 'FontsFolder', 'InstallerName', 'InstallerVersion', 'LocalAppDataFolder', 'LogonUser', 'MyPicturesFolder', 'NativeMachine', 'NTProductType', 'NTSuiteBackOffice', 'NTSuiteDataCenter', 'NTSuiteEnterprise', 'NTSuitePersonal', 'NTSuiteSmallBusiness', 'NTSuiteSmallBusinessRestricted', 'NTSuiteWebServer', 'PersonalFolder', 'Privileged', 'ProgramFiles64Folder', 'ProgramFiles6432Folder', 'ProgramFilesFolder', 'ProgramMenuFolder', 'RebootPending', 'SendToFolder', 'ServicePackLevel', 'StartMenuFolder', 'StartupFolder', 'System64Folder', 'SystemFolder', 'TempFolder', 'TemplateFolder', 'TerminalServer', 'UserLanguageID', 'UserUILanguageID', 'VersionMsi', 'VersionNT', 'VersionNT64', 'WindowsFolder', 'WindowsVolume', 'WixBundleAction', 'WixBundleCommandLineAction', 'WixBundleForcedRestartPackage', 'WixBundleElevated', 'WixBundleInstalled', 'WixBundleProviderKey', 'WixBundleTag', or 'WixBundleVersion'.",
225
"The 'REINSTALLMODE' MsiProperty is controlled by the bootstrapper and cannot be authored. (Illegal properties are: 'ACTION', 'ADDLOCAL', 'ADDSOURCE', 'ADDDEFAULT', 'ADVERTISE', 'ALLUSERS', 'REBOOT', 'REINSTALL', 'REINSTALLMODE', or 'REMOVE'.) Remove the MsiProperty element.",
226
}, messages.ToArray());
227
+
228
+ Assert.Equal(1159, result.ExitCode);
229
}
230
}
231
}
src/wix/test/WixToolsetTest.CoreIntegration/FeatureFixture.cs
+7
-5
@@ -32,13 +32,15 @@ namespace WixToolsetTest.CoreIntegration
32
"-o", msiPath
33
});
34
35
- Assert.Equal(267, result.ExitCode);
35
+ var messages = result.Messages.Select(m => m.ToString()).ToList();
36
+ messages.Sort();
37
37
- var errors = result.Messages.Where(m => m.Level == MessageLevel.Error);
38
- Assert.Equal(new[]
38
+ WixAssert.CompareLineByLine(new[]
39
{
40
- 267
41
- }, errors.Select(e => e.Id).ToArray());
40
+ "Found orphaned Component 'filit6MyH46zIGKsPPPXDZDfeNrfVY'. If this is a Package, every Component must have at least one parent Feature. To include a Component in a Module, you must include it directly as a Component element of the Module element or indirectly via ComponentRef, ComponentGroup, or ComponentGroupRef elements.",
41
+ }, messages.ToArray());
42
+
43
+ Assert.Equal(267, result.ExitCode);
44
}
45
}
46
src/wix/test/WixToolsetTest.CoreIntegration/TestData/BundleWithInvalid/BundleWithInvalidLocValues.wxs
+2
-2
@@ -12,7 +12,7 @@
12
<CommandLine Condition="!(loc.NonsenseExecuteCondition)" />
13
</ExePackage>
14
</Chain>
15
- <Variable Name="!(loc.BuiltinBurnVariableName)" Value="1" />
16
- <SetVariable Id="Builtin" Condition="!(loc.NonsenseDetectCondition)" Variable="!(loc.BuiltinBurnVariableName)" Value="1" />
15
+ <!--<Variable Name="FOO" Value="1" />
16
+ <SetVariable Id="Builtin" Condition="!(loc.NonsenseDetectCondition)" Variable="FOO" Value="1" />-->
17
</Bundle>
18
</Wix>
src/wix/test/WixToolsetTest.CoreIntegration/TestData/BundleWithInvalid/BundleWithInvalidLocVariableNames.wxs
new
+16
@@ -0,0 +1,16 @@
1
+<Wix xmlns="http://wixtoolset.org/schemas/v4/wxs">
2
+ <Bundle Name="BundleWithInvalidUpgradeCode" Condition="!(loc.NonsenseGlobalCondition)"
3
+ Version="1.0.0.0" Manufacturer="Example Corporation" UpgradeCode="{F2A56B65-2105-44C8-A532-A93A8C169D07}">
4
+ <BootstrapperApplication>
5
+ <BootstrapperApplicationDll SourceFile="fakeba.dll" />
6
+ </BootstrapperApplication>
7
+ <Chain>
8
+ <MsiPackage SourceFile="example.msi" >
9
+ <MsiProperty Name="!(loc.BuiltinMsiPropertyName)" Value="1" />
10
+ </MsiPackage>
11
+ <ExePackage DetectCondition="DoSomething" UninstallArguments="-uninstall" SourceFile="burn.exe" />
12
+ </Chain>
13
+ <Variable Name="!(loc.BuiltinBurnVariableName)" Value="1" />
14
+ <SetVariable Id="Builtin" Condition="!(loc.NonsenseDetectCondition)" Variable="!(loc.BuiltinBurnVariableName)" Value="1" />
15
+ </Bundle>
16
+</Wix>
src/wix/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj
+4
@@ -14,6 +14,10 @@
14
<Content Include="TestData\**" CopyToOutputDirectory="PreserveNewest" />
15
</ItemGroup>
16
17
+ <ItemGroup>
18
+ <None Remove="TestData\BundleWithInvalid\BundleWithInvalidLocVariableNames.wxs" />
19
+ </ItemGroup>
20
+
21
<ItemGroup>
22
<ProjectReference Include="..\..\WixToolset.Core\WixToolset.Core.csproj" />
23
<ProjectReference Include="..\..\WixToolset.Core.Burn\WixToolset.Core.Burn.csproj" />