diff --git a/Rules/AvoidValidateRangeTypeMismatch.cs b/Rules/AvoidValidateRangeTypeMismatch.cs new file mode 100644 index 000000000..d23f51243 --- /dev/null +++ b/Rules/AvoidValidateRangeTypeMismatch.cs @@ -0,0 +1,163 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using System.Management.Automation.Language; +using System.Reflection; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +#if !CORECLR +using System.ComponentModel.Composition; +#endif + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ + /// + /// AvoidValidateRangeTypeMismatch: Detect integer ValidateRange bounds on floating-point and decimal parameters. + /// +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + public sealed class AvoidValidateRangeTypeMismatch : IScriptRule + { + private static readonly HashSet FloatingPointAndDecimalTypes = new HashSet + { + typeof(double), + typeof(float), + typeof(decimal) + }; + + private static readonly HashSet IntegralTypes = new HashSet + { + typeof(byte), + typeof(sbyte), + typeof(short), + typeof(ushort), + typeof(int), + typeof(uint), + typeof(long), + typeof(ulong) + }; + + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast is null) + { + throw new ArgumentNullException(Strings.NullAstErrorMessage); + } + + IEnumerable parameterAsts = ast.FindAll(testAst => testAst is ParameterAst, searchNestedScriptBlocks: true); + foreach (ParameterAst parameterAst in parameterAsts) + { + TypeConstraintAst typeConstraint = parameterAst.Attributes.OfType().FirstOrDefault(); + if (typeConstraint == null) + { + continue; + } + + Type parameterType = typeConstraint.TypeName.GetReflectionType(); + if (parameterType == null) + { + continue; + } + + if (parameterType.IsArray) + { + parameterType = parameterType.GetElementType(); + } + + parameterType = Nullable.GetUnderlyingType(parameterType) ?? parameterType; + + if (!FloatingPointAndDecimalTypes.Contains(parameterType)) + { + continue; + } + + foreach (AttributeAst attributeAst in parameterAst.Attributes.OfType()) + { + if (!IsValidateRangeAttribute(attributeAst) + || attributeAst.PositionalArguments.Count < 2 + || !attributeAst.PositionalArguments.Take(2).All(IsIntegralBoundary)) + { + continue; + } + + yield return new DiagnosticRecord( + String.Format(CultureInfo.CurrentCulture, Strings.AvoidValidateRangeTypeMismatchError, parameterAst.Name, typeConstraint.TypeName.FullName), + attributeAst.Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName); + } + } + } + + private static bool IsIntegralBoundary(ExpressionAst expressionAst) + { + if (expressionAst is ConstantExpressionAst constantExpressionAst) + { + return constantExpressionAst.Value != null && IntegralTypes.Contains(constantExpressionAst.Value.GetType()); + } + + if (expressionAst is ConvertExpressionAst convertExpressionAst) + { + return IntegralTypes.Contains(convertExpressionAst.Type.TypeName.GetReflectionType()); + } + + if (expressionAst is UnaryExpressionAst unaryExpressionAst + && (unaryExpressionAst.TokenKind == TokenKind.Minus || unaryExpressionAst.TokenKind == TokenKind.Plus)) + { + return IsIntegralBoundary(unaryExpressionAst.Child); + } + + if (expressionAst is MemberExpressionAst memberExpressionAst + && memberExpressionAst.Static + && memberExpressionAst.Expression is TypeExpressionAst typeExpressionAst + && memberExpressionAst.Member is StringConstantExpressionAst memberName) + { + Type type = typeExpressionAst.TypeName.GetReflectionType(); + FieldInfo field = type?.GetField(memberName.Value, BindingFlags.Public | BindingFlags.Static | BindingFlags.IgnoreCase | BindingFlags.FlattenHierarchy); + return field != null && field.IsLiteral && IntegralTypes.Contains(field.FieldType); + } + + return false; + } + + private static bool IsValidateRangeAttribute(AttributeAst attributeAst) + { + return attributeAst.TypeName.GetReflectionAttributeType() == typeof(System.Management.Automation.ValidateRangeAttribute); + } + + public string GetName() + { + return String.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.AvoidValidateRangeTypeMismatchName); + } + + public string GetCommonName() + { + return Strings.AvoidValidateRangeTypeMismatchCommonName; + } + + public string GetDescription() + { + return Strings.AvoidValidateRangeTypeMismatchDescription; + } + + public SourceType GetSourceType() + { + return SourceType.Builtin; + } + + public RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + public string GetSourceName() + { + return String.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + } +} diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 2f2b1e1a9..43d173a90 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1221,6 +1221,18 @@ AvoidMultipleTypeAttributes + + Avoid mismatched ValidateRange boundary types. + + + Integer ValidateRange boundaries can allow fractional values for floating-point and decimal parameters. Use floating-point or decimal boundaries that match the parameter type. + + + ValidateRange boundaries should match the type of parameter '{0}' ({1}) to correctly restrict fractional values. + + + AvoidValidateRangeTypeMismatch + Avoid Using Broken Hash Algorithms diff --git a/Tests/Rules/AvoidValidateRangeTypeMismatch.tests.ps1 b/Tests/Rules/AvoidValidateRangeTypeMismatch.tests.ps1 new file mode 100644 index 000000000..767da3add --- /dev/null +++ b/Tests/Rules/AvoidValidateRangeTypeMismatch.tests.ps1 @@ -0,0 +1,95 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +Describe 'AvoidValidateRangeTypeMismatch' { + BeforeAll { + $ruleName = 'PSAvoidValidateRangeTypeMismatch' + Add-Type -TypeDefinition @' +public class ValidateRangeBoundsBaseForScriptAnalyzer +{ + public const int Max = 100; +} + +public class ValidateRangeBoundsForScriptAnalyzer : ValidateRangeBoundsBaseForScriptAnalyzer +{ + public static readonly int ReadonlyIntegral = 100; + public const double FloatingPoint = 100.0; + public static int ThrowingGetterInvocationCount; + + public static int ThrowingProperty + { + get + { + ThrowingGetterInvocationCount++; + throw new System.InvalidOperationException(); + } + } +} + +namespace Contoso +{ + [System.AttributeUsage(System.AttributeTargets.All)] + public class ValidateRangeAttribute : System.Attribute + { + public ValidateRangeAttribute(int minRange, int maxRange) + { + } + } +} +'@ + } + + It 'reports integral boundaries for floating-point and decimal parameter types' { + $scripts = @( + 'function Test { param([double][ValidateRange(1, 2)]$Value) }' + 'function Test { param([single][ValidateRange(-1, 2)]$Value) }' + 'function Test { param([decimal][ValidateRange(1, 2)]$Value) }' + 'function Test { param([Nullable[double]][ValidateRange(1, 2)]$Value) }' + 'function Test { param([Nullable[single]][ValidateRange(1, 2)]$Value) }' + 'function Test { param([Nullable[decimal]][ValidateRange(1, 2)]$Value) }' + 'function Test { param([double[]][ValidateRange(1, 2)]$Values) }' + 'function Test { param([double][System.Management.Automation.ValidateRangeAttribute(1, 2)]$Value) }' + 'function Test { param([double][ValidateRange([int]0.0, [int]100.0)]$Value) }' + 'function Test { param([double][ValidateRange([int32]0.0, [int32]100.0)]$Value) }' + 'function Test { param([double][ValidateRange(0, [int]::MaxValue)]$Value) }' + 'function Test { param([double][ValidateRange(0, [ValidateRangeBoundsForScriptAnalyzer]::Max)]$Value) }' + 'function Test { param([double][System.Management.Automation.ValidateRange(0, 100)]$Value) }' + ) + + foreach ($scriptDefinition in $scripts) { + $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $diagnostics.Count | Should -Be 1 + $diagnostics[0].Extent.Text | Should -Match 'ValidateRange' + } + } + + It 'ignores matching floating-point boundaries and non-floating-point parameters' { + $scripts = @( + 'function Test { param([double][ValidateRange(1.0, 2.0)]$Value) }' + 'function Test { param([single][ValidateRange(1.0, 2.0)]$Value) }' + 'function Test { param([decimal][ValidateRange(1.0, 2.0)]$Value) }' + 'function Test { param([double][ValidateRange(1, 2.0)]$Value) }' + 'function Test { param([double][Contoso.ValidateRange(0, 100)]$Value) }' + 'function Test { param([int][ValidateRange(1, 2)]$Value) }' + ) + + foreach ($scriptDefinition in $scripts) { + Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName | Should -BeNullOrEmpty + } + } + + It 'ignores non-constant static member expressions without invoking getters' { + $scripts = @( + 'function Test { param([double][ValidateRange(0, [ValidateRangeBoundsForScriptAnalyzer]::ThrowingProperty)]$Value) }' + 'function Test { param([double][ValidateRange(0, [ValidateRangeBoundsForScriptAnalyzer]::ReadonlyIntegral)]$Value) }' + 'function Test { param([double][ValidateRange(0, [ValidateRangeBoundsForScriptAnalyzer]::FloatingPoint)]$Value) }' + ) + + foreach ($scriptDefinition in $scripts) { + Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName | + Where-Object RuleName -eq $ruleName | Should -BeNullOrEmpty + } + + [ValidateRangeBoundsForScriptAnalyzer]::ThrowingGetterInvocationCount | Should -Be 0 + } +} diff --git a/docs/Rules/AvoidValidateRangeTypeMismatch.md b/docs/Rules/AvoidValidateRangeTypeMismatch.md new file mode 100644 index 000000000..cd7b265db --- /dev/null +++ b/docs/Rules/AvoidValidateRangeTypeMismatch.md @@ -0,0 +1,49 @@ +--- +description: Detect integer ValidateRange boundaries on floating-point and decimal parameters +ms.topic: reference +title: AvoidValidateRangeTypeMismatch +--- +# AvoidValidateRangeTypeMismatch + +**Severity Level: Warning** + +**Default state: Always enabled** + +## Description + +Integer boundaries in a `ValidateRange` attribute can select integer validation behavior for +floating-point and decimal parameters. This can allow fractional values outside the declared range. +Use boundaries with the same numeric type as the parameter. + +## Example + +### Noncompliant + +```powershell +function Set-Percentage +{ + param([double][ValidateRange(0, 100)]$Value) +} +``` + +### Compliant + +```powershell +function Set-Percentage +{ + param([double][ValidateRange(0.0, 100.0)]$Value) +} +``` + +## Configure rule + +This rule is always enabled and isn't configurable. Use one of the following methods to avoid using +this rule: + +- Create a custom rule configuration file to include only the rules you want or exclude the rules + you don't want. +- Add the appropriate rule suppression attributes to your code to suppress the rule for specific + code blocks. For more information, see the _Suppressing rules_ section of [Using PSScriptAnalyzer][01]. + + +[01]: ../using-scriptanalyzer.md#suppressing-rules diff --git a/docs/Rules/README.md b/docs/Rules/README.md index c35d646d0..b02342821 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -52,6 +52,7 @@ title: List of PSScriptAnalyzer rules | [AvoidUsingUsernameAndPasswordParams][32] | Error | Always enabled | | | [AvoidUsingWMICmdlet][33] | Warning | Always enabled | | | [AvoidUsingWriteHost][34] | Warning | Always enabled | | +| [AvoidValidateRangeTypeMismatch][81] | Warning | Always enabled | | | [DSCDscExamplesPresent][35] | Information | Always enabled | | | [DSCDscTestsPresent][36] | Information | Always enabled | | | [DSCReturnCorrectTypesForDSCFunctions][37] | Information | Always enabled | | @@ -180,3 +181,4 @@ title: List of PSScriptAnalyzer rules [78]: UseToExportFieldsInManifest.md [79]: UseUsingScopeModifierInNewRunspaces.md [80]: UseUTF8EncodingForHelpFile.md +[81]: AvoidValidateRangeTypeMismatch.md