From c583705ddd99f249308d68e126df6d698edcc945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Sun, 20 Dec 2020 23:20:23 +0100 Subject: [PATCH 1/9] Rewrite DoNotUseInsecureXSLTScriptExecutionAnalyzer --- ...harpDoNotUseInsecureXSLTScriptExecution.cs | 28 -- .../Core/AnalyzerReleases.Unshipped.md | 1 + .../DoNotUseInsecureXSLTScriptExecution.cs | 393 ++++++++++-------- .../Helpers/SecurityDiagnosticHelpers.cs | 24 ++ .../Helpers/SyntaxNodeHelper.cs | 40 +- ...ormLoadInsecureConstructedSettingsTests.cs | 10 +- ...TransformLoadInsecureInputSettingsTests.cs | 10 +- ...asicDoNotUseInsecureXSLTScriptExecution.vb | 25 -- 8 files changed, 253 insertions(+), 278 deletions(-) delete mode 100644 src/NetAnalyzers/CSharp/Microsoft.NetFramework.Analyzers/CSharpDoNotUseInsecureXSLTScriptExecution.cs delete mode 100644 src/NetAnalyzers/VisualBasic/Microsoft.NetFramework.Analyzers/BasicDoNotUseInsecureXSLTScriptExecution.vb diff --git a/src/NetAnalyzers/CSharp/Microsoft.NetFramework.Analyzers/CSharpDoNotUseInsecureXSLTScriptExecution.cs b/src/NetAnalyzers/CSharp/Microsoft.NetFramework.Analyzers/CSharpDoNotUseInsecureXSLTScriptExecution.cs deleted file mode 100644 index ab9364cfc4..0000000000 --- a/src/NetAnalyzers/CSharp/Microsoft.NetFramework.Analyzers/CSharpDoNotUseInsecureXSLTScriptExecution.cs +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.NetFramework.Analyzers; -using Microsoft.NetFramework.Analyzers.Helpers; -using Microsoft.NetFramework.CSharp.Analyzers.Helpers; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.Diagnostics; - -namespace Microsoft.NetFramework.CSharp.Analyzers -{ - [DiagnosticAnalyzer(LanguageNames.CSharp)] - public sealed class CSharpDoNotUseInsecureXSLTScriptExecutionAnalyzer : DoNotUseInsecureXSLTScriptExecutionAnalyzer - { - protected override SyntaxNodeAnalyzer GetAnalyzer(CodeBlockStartAnalysisContext context, CompilationSecurityTypes types) - { - SyntaxNodeAnalyzer analyzer = new SyntaxNodeAnalyzer(types, CSharpSyntaxNodeHelper.Default); - context.RegisterSyntaxNodeAction( - analyzer.AnalyzeNode, - SyntaxKind.InvocationExpression, - SyntaxKind.ObjectCreationExpression, - SyntaxKind.SimpleAssignmentExpression, - SyntaxKind.VariableDeclarator); - - return analyzer; - } - } -} diff --git a/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md b/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md index 6f195fc591..e4bf6f7089 100644 --- a/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md +++ b/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md @@ -10,3 +10,4 @@ CA2355 | Security | Disabled | DataSetDataTableInSerializableObjectGraphAnalyzer CA2356 | Security | Disabled | DataSetDataTableInWebSerializableObjectGraphAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2356) CA2361 | Security | Disabled | DoNotUseDataSetReadXml, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2361) CA2362 | Security | Disabled | DataSetDataTableInSerializableTypeAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca2362) +CA3076 | Security | Info | DoNotUseInsecureXSLTScriptExecutionAnalyzer, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca3076) diff --git a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs index 7813cae366..990729fe7f 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs @@ -1,23 +1,32 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -// TODO(dotpaul): Enable nullable analysis when rewriting this to use DFA. -#nullable disable - -using System.Collections.Generic; +using System; +using System.Collections.Concurrent; using System.Collections.Immutable; -using System.Linq; using Analyzer.Utilities; +using Analyzer.Utilities.Extensions; using Microsoft.NetFramework.Analyzers.Helpers; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; namespace Microsoft.NetFramework.Analyzers { - public abstract class DoNotUseInsecureXSLTScriptExecutionAnalyzer : DiagnosticAnalyzer where TLanguageKindEnum : struct + [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] + public sealed class DoNotUseInsecureXSLTScriptExecutionAnalyzer : DiagnosticAnalyzer { internal const string RuleId = "CA3076"; - internal static DiagnosticDescriptor RuleDoNotUseInsecureXSLTScriptExecution = CreateDiagnosticDescriptor(SecurityDiagnosticHelpers.GetLocalizableResourceString(nameof(MicrosoftNetFrameworkAnalyzersResources.DoNotUseInsecureDtdProcessingGenericMessage)), - SecurityDiagnosticHelpers.GetLocalizableResourceString(nameof(MicrosoftNetFrameworkAnalyzersResources.DoNotUseInsecureXSLTScriptExecutionDescription))); + + internal static DiagnosticDescriptor RuleDoNotUseInsecureXSLTScriptExecution = + DiagnosticDescriptorHelper.Create( + RuleId, + SecurityDiagnosticHelpers.GetLocalizableResourceString(nameof(MicrosoftNetFrameworkAnalyzersResources.InsecureXsltScriptProcessingMessage)), + SecurityDiagnosticHelpers.GetLocalizableResourceString(nameof(MicrosoftNetFrameworkAnalyzersResources.DoNotUseInsecureDtdProcessingGenericMessage)), + DiagnosticCategory.Security, + RuleLevel.IdeSuggestion, + SecurityDiagnosticHelpers.GetLocalizableResourceString(nameof(MicrosoftNetFrameworkAnalyzersResources.DoNotUseInsecureXSLTScriptExecutionDescription)), + isPortedFxCopRule: false, + isDataflowRule: false); public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(RuleDoNotUseInsecureXSLTScriptExecution); @@ -25,160 +34,171 @@ public abstract class DoNotUseInsecureXSLTScriptExecutionAnalyzer + context => { Compilation compilation = context.Compilation; var xmlTypes = new CompilationSecurityTypes(compilation); - if (xmlTypes.XslCompiledTransform != null) + if (xmlTypes.XslCompiledTransform == null) { - context.RegisterCodeBlockStartAction( - (c) => - { - GetAnalyzer(c, xmlTypes); - }); + return; } - }); - } + context.RegisterOperationBlockStartAction(context => + { + var analyzer = new OperationBlockAnalyzer(xmlTypes, context.OwningSymbol); + + context.RegisterOperationAction(context => + { + var assignment = (IAssignmentOperation)context.Operation; + analyzer.AnalyzeNodeForXsltSettings(assignment.Target, assignment.Value); + }, OperationKind.SimpleAssignment); - private static DiagnosticDescriptor CreateDiagnosticDescriptor(LocalizableResourceString messageFormat, LocalizableResourceString description) - { - return DiagnosticDescriptorHelper.Create(RuleId, - SecurityDiagnosticHelpers.GetLocalizableResourceString(nameof(MicrosoftNetFrameworkAnalyzersResources.InsecureXsltScriptProcessingMessage)), - messageFormat, - DiagnosticCategory.Security, - RuleLevel.BuildWarning, - description, - isPortedFxCopRule: false, - isDataflowRule: false); - } + context.RegisterOperationAction(context => + { + var variableDeclarator = (IVariableDeclaratorOperation)context.Operation; + analyzer.AnalyzeNodeForXsltSettings(variableDeclarator, variableDeclarator.GetVariableInitializer().Value); + }, OperationKind.VariableDeclarator); - protected abstract SyntaxNodeAnalyzer GetAnalyzer(CodeBlockStartAnalysisContext context, CompilationSecurityTypes types); + context.RegisterOperationAction(context => + { + var invocation = (IInvocationOperation)context.Operation; + analyzer.AnalyzeNodeForXslCompiledTransformLoad(invocation.TargetMethod, invocation.Arguments, + context.ReportDiagnostic); + }, OperationKind.Invocation); + + context.RegisterOperationAction(context => + { + var objectCreation = (IObjectCreationOperation)context.Operation; + analyzer.AnalyzeNodeForXslCompiledTransformLoad( + objectCreation.Constructor, objectCreation.Arguments, context.ReportDiagnostic); + }, OperationKind.ObjectCreation); + }); + }); + } - protected sealed class SyntaxNodeAnalyzer + private sealed class OperationBlockAnalyzer { private readonly CompilationSecurityTypes _xmlTypes; - private readonly SyntaxNodeHelper _syntaxNodeHelper; + private readonly ISymbol _enclosingSymbol; - private readonly Dictionary _xsltSettingsEnvironments = new(); + private readonly ConcurrentDictionary _xsltSettingsEnvironments = new(); - public SyntaxNodeAnalyzer(CompilationSecurityTypes xmlTypes, SyntaxNodeHelper helper) + public OperationBlockAnalyzer(CompilationSecurityTypes xmlTypes, ISymbol enclosingSymbol) { _xmlTypes = xmlTypes; - _syntaxNodeHelper = helper; + _enclosingSymbol = enclosingSymbol; } - public void AnalyzeNode(SyntaxNodeAnalysisContext context) + public void AnalyzeNodeForXslCompiledTransformLoad(IMethodSymbol methodSymbol, ImmutableArray arguments, + Action reportDiagnostic) { - AnalyzeNodeForXsltSettings(context); - AnalyzeNodeForXslCompiledTransformLoad(context); - } + if (!SecurityDiagnosticHelpers.IsXslCompiledTransformLoad(methodSymbol, _xmlTypes)) + { + return; + } - private void AnalyzeNodeForXslCompiledTransformLoad(SyntaxNodeAnalysisContext context) - { - SyntaxNode node = context.Node; - SemanticModel model = context.SemanticModel; - IMethodSymbol methodSymbol = _syntaxNodeHelper.GetCalleeMethodSymbol(node, model); + int xmlResolverIndex = SecurityDiagnosticHelpers.GetXmlResolverParameterIndex(methodSymbol, _xmlTypes); + int xsltSettingsIndex = SecurityDiagnosticHelpers.GetXsltSettingsParameterIndex(methodSymbol, _xmlTypes); - if (SecurityDiagnosticHelpers.IsXslCompiledTransformLoad(methodSymbol, _xmlTypes)) + // Overloads with no XmlResolver and XstlSettings specified are secure since they all have following behavior: + // 1. An XmlUrlResolver with no user credentials is used to process any xsl:import or xsl:include elements. + // 2. The document() function is disabled. + // 3. Embedded scripts are not supported. + if (xmlResolverIndex < 0 || xsltSettingsIndex < 0) { - bool isSecureResolver; - bool isSecureSettings; - bool isSetInBlock; - - int xmlResolverIndex = SecurityDiagnosticHelpers.GetXmlResolverParameterIndex(methodSymbol, _xmlTypes); - int xsltSettingsIndex = SecurityDiagnosticHelpers.GetXsltSettingsParameterIndex(methodSymbol, _xmlTypes); - - // Overloads with no XmlResolver and XstlSettings specified are secure since they all have folowing behavior: - // 1. An XmlUrlResolver with no user credentials is used to process any xsl:import or xsl:include elements. - // 2. The document() function is disabled. - // 3. Embedded scripts are not supported. - if (xmlResolverIndex >= 0 && - xsltSettingsIndex >= 0) - { - IEnumerable argumentExpressionNodes = _syntaxNodeHelper.GetInvocationArgumentExpressionNodes(node); - SyntaxNode resolverNode = argumentExpressionNodes.ElementAt(xmlResolverIndex); - - isSecureResolver = SyntaxNodeHelper.NodeHasConstantValueNull(resolverNode, model) || - SecurityDiagnosticHelpers.IsXmlSecureResolverType(model.GetTypeInfo(resolverNode).Type, _xmlTypes); + return; + } + var resolverArgumentOperation = arguments[xmlResolverIndex].Value; + if (resolverArgumentOperation is IConversionOperation conversion) + { + resolverArgumentOperation = conversion.Operand; + } - SyntaxNode settingsNode = argumentExpressionNodes.ElementAt(xsltSettingsIndex); - ISymbol settingsSymbol = SyntaxNodeHelper.GetSymbol(settingsNode, model); + var isSecureResolver = resolverArgumentOperation.HasNullConstantValue() + || SecurityDiagnosticHelpers.IsXmlSecureResolverType(resolverArgumentOperation.Type, _xmlTypes); - // 1. pass null or XsltSettings.Default as XsltSetting : secure - if (settingsSymbol == null || SecurityDiagnosticHelpers.IsXsltSettingsDefaultProperty(settingsSymbol as IPropertySymbol, _xmlTypes)) - { - isSetInBlock = true; - isSecureSettings = true; - } - // 2. XsltSettings.TrustedXslt : insecure - else if (SecurityDiagnosticHelpers.IsXsltSettingsTrustedXsltProperty(settingsSymbol as IPropertySymbol, _xmlTypes)) - { - isSetInBlock = true; - isSecureSettings = false; - } - // 3. check xsltSettingsEnvironments, if IsScriptDisabled && IsDocumentFunctionDisabled then secure, else insecure - else if (_xsltSettingsEnvironments.TryGetValue(settingsSymbol, out XsltSettingsEnvironment env)) - { - isSetInBlock = false; - isSecureSettings = env.IsDocumentFunctionDisabled && env.IsScriptDisabled; - } - //4. symbol for settings is not found => passed in without any change => assume insecure - else - { - isSetInBlock = true; - isSecureSettings = false; - } + var settingsOperation = arguments[xsltSettingsIndex].Value; + ISymbol? settingsSymbol = settingsOperation.Kind switch + { + OperationKind.ParameterReference => ((IParameterReferenceOperation)settingsOperation).Parameter, + OperationKind.LocalReference => ((ILocalReferenceOperation)settingsOperation).Local, + OperationKind.FieldReference => ((IFieldReferenceOperation)settingsOperation).Field, + OperationKind.PropertyReference => ((IPropertyReferenceOperation)settingsOperation).Property, + _ => null, + }; + + bool isSecureSettings; + bool isSetInBlock; + + // 1. pass null or XsltSettings.Default as XsltSetting : secure + if (settingsSymbol == null || SecurityDiagnosticHelpers.IsXsltSettingsDefaultProperty(settingsSymbol as IPropertySymbol, _xmlTypes)) + { + isSetInBlock = true; + isSecureSettings = true; + } + // 2. XsltSettings.TrustedXslt : insecure + else if (SecurityDiagnosticHelpers.IsXsltSettingsTrustedXsltProperty(settingsSymbol as IPropertySymbol, _xmlTypes)) + { + isSetInBlock = true; + isSecureSettings = false; + } + // 3. check xsltSettingsEnvironments, if IsScriptDisabled && IsDocumentFunctionDisabled then secure, else insecure + else if (_xsltSettingsEnvironments.TryGetValue(settingsSymbol, out XsltSettingsEnvironment env)) + { + isSetInBlock = false; + isSecureSettings = env.IsDocumentFunctionDisabled && env.IsScriptDisabled; + } + //4. symbol for settings is not found => passed in without any change => assume insecure + else + { + isSetInBlock = true; + isSecureSettings = false; + } - if (!isSecureSettings && !isSecureResolver) - { - LocalizableResourceString message = SecurityDiagnosticHelpers.GetLocalizableResourceString( - isSetInBlock ? nameof(MicrosoftNetFrameworkAnalyzersResources.XslCompiledTransformLoadInsecureConstructedMessage) : - nameof(MicrosoftNetFrameworkAnalyzersResources.XslCompiledTransformLoadInsecureInputMessage), - SecurityDiagnosticHelpers.GetNonEmptyParentName(node, model, context.CancellationToken) - ); - - context.ReportDiagnostic( - Diagnostic.Create( - RuleDoNotUseInsecureXSLTScriptExecution, - node.GetLocation(), - message - ) - ); - } - } + if (!isSecureSettings && !isSecureResolver) + { + LocalizableResourceString message = SecurityDiagnosticHelpers.GetLocalizableResourceString( + isSetInBlock + ? nameof(MicrosoftNetFrameworkAnalyzersResources.XslCompiledTransformLoadInsecureConstructedMessage) + : nameof(MicrosoftNetFrameworkAnalyzersResources.XslCompiledTransformLoadInsecureInputMessage), + SecurityDiagnosticHelpers.GetNonEmptyParentName(_enclosingSymbol) + ); + + var invocationOrObjCreation = settingsOperation.Parent.Parent; + reportDiagnostic(invocationOrObjCreation.CreateDiagnostic(RuleDoNotUseInsecureXSLTScriptExecution, message)); } } - private void AnalyzeNodeForXsltSettings(SyntaxNodeAnalysisContext context) + public void AnalyzeNodeForXsltSettings(IOperation lhs, IOperation rhs) { - SyntaxNode node = context.Node; - SemanticModel model = context.SemanticModel; - - SyntaxNode lhs = _syntaxNodeHelper.GetAssignmentLeftNode(node); - SyntaxNode rhs = _syntaxNodeHelper.GetAssignmentRightNode(node); - - if (lhs == null || rhs == null) + ISymbol? lhsSymbol = lhs.Kind switch { - return; - } - - ISymbol lhsSymbol = SyntaxNodeHelper.GetSymbol(lhs, model); - if (lhsSymbol == null) + OperationKind.VariableDeclarator => ((IVariableDeclaratorOperation)lhs).Symbol, + OperationKind.PropertyReference => ((IPropertyReferenceOperation)lhs).Property, + OperationKind.ParameterReference => ((IParameterReferenceOperation)lhs).Parameter, + OperationKind.LocalReference => ((ILocalReferenceOperation)lhs).Local, + OperationKind.FieldReference => ((IFieldReferenceOperation)lhs).Field, + _ => null, + }; + if (lhsSymbol is null) { return; } - IMethodSymbol rhsMethodSymbol = _syntaxNodeHelper.GetCalleeMethodSymbol(rhs, model); - IPropertySymbol rhsPropertySymbol = SyntaxNodeHelper.GetCalleePropertySymbol(rhs, model); + IMethodSymbol? rhsMethodSymbol = rhs.Kind switch + { + OperationKind.Invocation => ((IInvocationOperation)rhs).TargetMethod, + OperationKind.ObjectCreation => ((IObjectCreationOperation)rhs).Constructor, + _ => null, + }; + IPropertySymbol? rhsPropertySymbol = (rhs as IPropertyReferenceOperation)?.Property; if (SecurityDiagnosticHelpers.IsXsltSettingsCtor(rhsMethodSymbol, _xmlTypes)) { @@ -187,35 +207,49 @@ private void AnalyzeNodeForXsltSettings(SyntaxNodeAnalysisContext context) env.XsltSettingsSymbol = lhsSymbol; env.XsltSettingsDefinitionSymbol = rhsMethodSymbol; - env.XsltSettingsDefinition = node; - env.EnclosingConstructSymbol = _syntaxNodeHelper.GetEnclosingConstructSymbol(node, model); - //default both properties are disbled + env.XsltSettingsDefinition = lhs.Parent; + env.EnclosingConstructSymbol = _enclosingSymbol; + // default both properties are disabled env.IsDocumentFunctionDisabled = true; env.IsScriptDisabled = true; // XsltSettings Constructor (Boolean, Boolean) - if (rhsMethodSymbol.Parameters.Any()) + if (!rhsMethodSymbol.Parameters.IsEmpty) { - IEnumerable argumentExpressionNodes = _syntaxNodeHelper.GetObjectCreationArgumentExpressionNodes(rhs); - env.IsDocumentFunctionDisabled = SyntaxNodeHelper.NodeHasConstantValueBoolFalse(argumentExpressionNodes.ElementAt(0), model); - env.IsScriptDisabled = SyntaxNodeHelper.NodeHasConstantValueBoolFalse(argumentExpressionNodes.ElementAt(1), model); + var arguments = rhs.Kind switch + { + OperationKind.Invocation => ((IInvocationOperation)rhs).Arguments, + OperationKind.ObjectCreation => ((IObjectCreationOperation)rhs).Arguments, + _ => ImmutableArray.Empty, + }; + env.IsDocumentFunctionDisabled = arguments[0].Value.TryGetBoolConstantValue(out var value) && !value; + env.IsScriptDisabled = arguments[1].Value.TryGetBoolConstantValue(out value) && !value; } - foreach (SyntaxNode arg in _syntaxNodeHelper.GetObjectInitializerExpressionNodes(rhs)) + var initializers = rhs.Kind switch + { + OperationKind.ObjectCreation => ((IObjectCreationOperation)rhs).Initializer?.Initializers, + _ => null, + } ?? ImmutableArray.Empty; + + foreach (var arg in initializers) { - SyntaxNode argLhs = _syntaxNodeHelper.GetAssignmentLeftNode(arg); - SyntaxNode argRhs = _syntaxNodeHelper.GetAssignmentRightNode(arg); + if (arg is not ISimpleAssignmentOperation assignment) + { + continue; + } - ISymbol argLhsSymbol = SyntaxNodeHelper.GetSymbol(argLhs, model); + var argLhsPropertySymbol = (assignment.Target as IPropertyReferenceOperation)?.Property; + var argRhs = assignment.Value; // anything other than a constant false is treated as true - if (SecurityDiagnosticHelpers.IsXsltSettingsEnableDocumentFunctionProperty(argLhsSymbol as IPropertySymbol, _xmlTypes)) + if (SecurityDiagnosticHelpers.IsXsltSettingsEnableDocumentFunctionProperty(argLhsPropertySymbol, _xmlTypes)) { - env.IsDocumentFunctionDisabled = SyntaxNodeHelper.NodeHasConstantValueBoolFalse(argRhs, model); + env.IsDocumentFunctionDisabled = argRhs.TryGetBoolConstantValue(out var value) && !value; } - else if (SecurityDiagnosticHelpers.IsXsltSettingsEnableScriptProperty(argLhsSymbol as IPropertySymbol, _xmlTypes)) + else if (SecurityDiagnosticHelpers.IsXsltSettingsEnableScriptProperty(argLhsPropertySymbol, _xmlTypes)) { - env.IsScriptDisabled = SyntaxNodeHelper.NodeHasConstantValueBoolFalse(argRhs, model); + env.IsScriptDisabled = argRhs.TryGetBoolConstantValue(out var value) && !value; } } } @@ -226,8 +260,8 @@ private void AnalyzeNodeForXsltSettings(SyntaxNodeAnalysisContext context) env.XsltSettingsSymbol = lhsSymbol; env.XsltSettingsDefinitionSymbol = rhsPropertySymbol; - env.XsltSettingsDefinition = node; - env.EnclosingConstructSymbol = _syntaxNodeHelper.GetEnclosingConstructSymbol(node, model); + env.XsltSettingsDefinition = lhs.Parent; + env.EnclosingConstructSymbol = _enclosingSymbol; env.IsDocumentFunctionDisabled = true; env.IsScriptDisabled = true; } @@ -238,57 +272,68 @@ private void AnalyzeNodeForXsltSettings(SyntaxNodeAnalysisContext context) env.XsltSettingsSymbol = lhsSymbol; env.XsltSettingsDefinitionSymbol = rhsPropertySymbol; - env.XsltSettingsDefinition = node; - env.EnclosingConstructSymbol = _syntaxNodeHelper.GetEnclosingConstructSymbol(node, model); + env.XsltSettingsDefinition = lhs.Parent; + env.EnclosingConstructSymbol = _enclosingSymbol; } else { - bool isXlstSettingsEnableDocumentFunctionProperty = SecurityDiagnosticHelpers.IsXsltSettingsEnableDocumentFunctionProperty(lhsSymbol as IPropertySymbol, _xmlTypes); - bool isXlstSettingsEnableScriptProperty = SecurityDiagnosticHelpers.IsXsltSettingsEnableScriptProperty(lhsSymbol as IPropertySymbol, _xmlTypes); + var lhsPropertySymbol = lhsSymbol as IPropertySymbol; + bool isXlstSettingsEnableDocumentFunctionProperty = SecurityDiagnosticHelpers.IsXsltSettingsEnableDocumentFunctionProperty(lhsPropertySymbol, _xmlTypes); + bool isXlstSettingsEnableScriptProperty = SecurityDiagnosticHelpers.IsXsltSettingsEnableScriptProperty(lhsPropertySymbol, _xmlTypes); + if (!isXlstSettingsEnableDocumentFunctionProperty && + !isXlstSettingsEnableScriptProperty) + { + return; + } - if (isXlstSettingsEnableDocumentFunctionProperty || - isXlstSettingsEnableScriptProperty) + IOperation? lhsExpression = lhs.Kind switch { - SyntaxNode lhsExpressionNode = _syntaxNodeHelper.GetMemberAccessExpressionNode(lhs); - if (lhsExpressionNode == null) - { - return; - } + OperationKind.FieldReference => ((IFieldReferenceOperation)lhs).Instance, + OperationKind.PropertyReference => ((IPropertyReferenceOperation)lhs).Instance, + _ => null, + }; - ISymbol lhsExpressionSymbol = SyntaxNodeHelper.GetSymbol(lhsExpressionNode, model); - if (lhsExpressionSymbol == null) - { - return; - } + ISymbol? lhsExpressionSymbol = lhsExpression?.Kind switch + { + OperationKind.ParameterReference => ((IParameterReferenceOperation)lhsExpression).Parameter, + OperationKind.LocalReference => ((ILocalReferenceOperation)lhsExpression).Local, + OperationKind.FieldReference => ((IFieldReferenceOperation)lhsExpression).Field, + OperationKind.PropertyReference => ((IPropertyReferenceOperation)lhsExpression).Property, + _ => null, + }; + + if (lhsExpressionSymbol is null) + { + return; + } - if (!_xsltSettingsEnvironments.TryGetValue(lhsExpressionSymbol, out XsltSettingsEnvironment env)) + if (!_xsltSettingsEnvironments.TryGetValue(lhsExpressionSymbol, out var env)) + { + env = new XsltSettingsEnvironment { - env = new XsltSettingsEnvironment - { - XsltSettingsSymbol = lhsExpressionSymbol - }; - _xsltSettingsEnvironments[lhsExpressionSymbol] = env; - } + XsltSettingsSymbol = lhsExpressionSymbol, + }; + _xsltSettingsEnvironments[lhsExpressionSymbol] = env; + } - if (isXlstSettingsEnableDocumentFunctionProperty) - { - env.IsDocumentFunctionDisabled = SyntaxNodeHelper.NodeHasConstantValueBoolFalse(rhs, model); - } - else if (isXlstSettingsEnableScriptProperty) - { - env.IsScriptDisabled = SyntaxNodeHelper.NodeHasConstantValueBoolFalse(rhs, model); - } + if (isXlstSettingsEnableDocumentFunctionProperty) + { + env.IsDocumentFunctionDisabled = rhs.TryGetBoolConstantValue(out var value) && !value; + } + else if (isXlstSettingsEnableScriptProperty) + { + env.IsScriptDisabled = rhs.TryGetBoolConstantValue(out var value) && !value; } } } private class XsltSettingsEnvironment { - internal ISymbol XsltSettingsSymbol { get; set; } - internal ISymbol XsltSettingsDefinitionSymbol { get; set; } - internal SyntaxNode XsltSettingsDefinition { get; set; } - internal ISymbol EnclosingConstructSymbol { get; set; } + internal ISymbol? XsltSettingsSymbol { get; set; } + internal ISymbol? XsltSettingsDefinitionSymbol { get; set; } + internal IOperation? XsltSettingsDefinition { get; set; } + internal ISymbol? EnclosingConstructSymbol { get; set; } internal bool IsScriptDisabled { get; set; } internal bool IsDocumentFunctionDisabled { get; set; } } diff --git a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs index 532094d332..d680061a43 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs @@ -263,6 +263,30 @@ public static string GetNonEmptyParentName(SyntaxNode current, SemanticModel mod return string.Empty; } + /// + /// Get non-empty class or method name which encloses the current syntax node + /// + public static string GetNonEmptyParentName(ISymbol symbol) + { + var current = symbol; + while (current.ContainingSymbol != null) + { + switch (current) + { + case IMethodSymbol method: + return method.MethodKind == MethodKind.Ordinary + ? method.Name + : method.ContainingType.Name; + case INamedTypeSymbol namedType: + return namedType.Name; + } + + current = symbol.ContainingSymbol; + } + + return string.Empty; + } + /// /// Gets the version of the target .NET framework of the compilation. /// diff --git a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SyntaxNodeHelper.cs b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SyntaxNodeHelper.cs index efda2960b9..ec0330cedb 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SyntaxNodeHelper.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SyntaxNodeHelper.cs @@ -36,7 +36,7 @@ protected enum CallKinds // returns true if node is an ObjectCreationExpression and is under a FieldDeclaration node public abstract bool IsObjectCreationExpressionUnderFieldDeclaration(SyntaxNode? node); - // returns the ancestor VariableDeclarator node for an ObjectCreationExpression if + // returns the ancestor VariableDeclarator node for an ObjectCreationExpression if // IsObjectCreationExpressionUnderFieldDeclaration(node) returns true, return null otherwise. public abstract SyntaxNode? GetVariableDeclaratorOfAFieldDeclarationNode(SyntaxNode? objectCreationExpression); @@ -57,11 +57,6 @@ protected enum CallKinds return symbol; } - public IEnumerable GetCallArgumentExpressionNodes(SyntaxNode node) - { - return GetCallArgumentExpressionNodes(node, CallKinds.AnyCall); - } - public IEnumerable GetInvocationArgumentExpressionNodes(SyntaxNode node) { return GetCallArgumentExpressionNodes(node, CallKinds.Invocation); @@ -85,17 +80,6 @@ public static IEnumerable GetCandidateCalleeMethodSymbols(SyntaxN } } - public IEnumerable GetCalleeMethodSymbols(SyntaxNode? node, SemanticModel semanticModel) - { - IMethodSymbol? symbol = GetCalleeMethodSymbol(node, semanticModel); - if (symbol != null) - { - return new List() { symbol }; - } - - return GetCandidateCalleeMethodSymbols(node, semanticModel); - } - public static IPropertySymbol? GetCalleePropertySymbol(SyntaxNode? node, SemanticModel semanticModel) { ISymbol? symbol = GetReferencedSymbol(node, semanticModel); @@ -107,17 +91,6 @@ public IEnumerable GetCalleeMethodSymbols(SyntaxNode? node, Seman return null; } - public static IFieldSymbol? GetCalleeFieldSymbol(SyntaxNode? node, SemanticModel semanticModel) - { - ISymbol? symbol = GetReferencedSymbol(node, semanticModel); - if (symbol != null && symbol.Kind == SymbolKind.Field) - { - return (IFieldSymbol)symbol; - } - - return null; - } - public static ISymbol? GetSymbol(SyntaxNode? node, SemanticModel semanticModel) { return GetDeclaredSymbol(node, semanticModel) ?? GetReferencedSymbol(node, semanticModel); @@ -163,17 +136,6 @@ public static bool NodeHasConstantValueNull(SyntaxNode? node, SemanticModel? mod return value.HasValue && value.Value == null; } - public static bool NodeHasConstantValueIntZero(SyntaxNode? node, SemanticModel? model) - { - if (node == null || model == null) - { - return false; - } - Optional value = model.GetConstantValue(node); - return value.HasValue && - value.Value is 0; - } - public static bool NodeHasConstantValueBoolFalse(SyntaxNode? node, SemanticModel? model) { if (node == null || model == null) diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecutionXslCompiledTransformLoadInsecureConstructedSettingsTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecutionXslCompiledTransformLoadInsecureConstructedSettingsTests.cs index a912826189..d5869e875d 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecutionXslCompiledTransformLoadInsecureConstructedSettingsTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecutionXslCompiledTransformLoadInsecureConstructedSettingsTests.cs @@ -2,14 +2,12 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Testing; -using Microsoft.NetFramework.CSharp.Analyzers; -using Microsoft.NetFramework.VisualBasic.Analyzers; using Xunit; using VerifyCS = Test.Utilities.CSharpSecurityCodeFixVerifier< - Microsoft.NetFramework.CSharp.Analyzers.CSharpDoNotUseInsecureXSLTScriptExecutionAnalyzer, + Microsoft.NetFramework.Analyzers.DoNotUseInsecureXSLTScriptExecutionAnalyzer, Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; using VerifyVB = Test.Utilities.VisualBasicSecurityCodeFixVerifier< - Microsoft.NetFramework.VisualBasic.Analyzers.BasicDoNotUseInsecureXSLTScriptExecutionAnalyzer, + Microsoft.NetFramework.Analyzers.DoNotUseInsecureXSLTScriptExecutionAnalyzer, Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; namespace Microsoft.NetFramework.Analyzers.UnitTests @@ -18,12 +16,12 @@ public partial class DoNotUseInsecureXSLTScriptExecutionAnalyzerTests { private static DiagnosticResult GetCA3076LoadCSharpResultAt(int line, int column, string name) { - return new DiagnosticResult(CSharpDoNotUseInsecureXSLTScriptExecutionAnalyzer.RuleDoNotUseInsecureXSLTScriptExecution).WithLocation(line, column).WithArguments(string.Format(MicrosoftNetFrameworkAnalyzersResources.XslCompiledTransformLoadInsecureInputMessage, name)); + return new DiagnosticResult(DoNotUseInsecureXSLTScriptExecutionAnalyzer.RuleDoNotUseInsecureXSLTScriptExecution).WithLocation(line, column).WithArguments(string.Format(MicrosoftNetFrameworkAnalyzersResources.XslCompiledTransformLoadInsecureInputMessage, name)); } private static DiagnosticResult GetCA3076LoadBasicResultAt(int line, int column, string name) { - return new DiagnosticResult(BasicDoNotUseInsecureXSLTScriptExecutionAnalyzer.RuleDoNotUseInsecureXSLTScriptExecution).WithLocation(line, column).WithArguments(string.Format(MicrosoftNetFrameworkAnalyzersResources.XslCompiledTransformLoadInsecureInputMessage, name)); + return new DiagnosticResult(DoNotUseInsecureXSLTScriptExecutionAnalyzer.RuleDoNotUseInsecureXSLTScriptExecution).WithLocation(line, column).WithArguments(string.Format(MicrosoftNetFrameworkAnalyzersResources.XslCompiledTransformLoadInsecureInputMessage, name)); } [Fact] diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecutionXslCompiledTransformLoadInsecureInputSettingsTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecutionXslCompiledTransformLoadInsecureInputSettingsTests.cs index 91342be6ed..1d15e9aa6e 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecutionXslCompiledTransformLoadInsecureInputSettingsTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecutionXslCompiledTransformLoadInsecureInputSettingsTests.cs @@ -2,14 +2,12 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Testing; -using Microsoft.NetFramework.CSharp.Analyzers; -using Microsoft.NetFramework.VisualBasic.Analyzers; using Xunit; using VerifyCS = Test.Utilities.CSharpSecurityCodeFixVerifier< - Microsoft.NetFramework.CSharp.Analyzers.CSharpDoNotUseInsecureXSLTScriptExecutionAnalyzer, + Microsoft.NetFramework.Analyzers.DoNotUseInsecureXSLTScriptExecutionAnalyzer, Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; using VerifyVB = Test.Utilities.VisualBasicSecurityCodeFixVerifier< - Microsoft.NetFramework.VisualBasic.Analyzers.BasicDoNotUseInsecureXSLTScriptExecutionAnalyzer, + Microsoft.NetFramework.Analyzers.DoNotUseInsecureXSLTScriptExecutionAnalyzer, Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; namespace Microsoft.NetFramework.Analyzers.UnitTests @@ -18,12 +16,12 @@ public partial class DoNotUseInsecureXSLTScriptExecutionAnalyzerTests { private static DiagnosticResult GetCA3076LoadInsecureConstructedCSharpResultAt(int line, int column, string name) { - return new DiagnosticResult(CSharpDoNotUseInsecureXSLTScriptExecutionAnalyzer.RuleDoNotUseInsecureXSLTScriptExecution).WithLocation(line, column).WithArguments(string.Format(MicrosoftNetFrameworkAnalyzersResources.XslCompiledTransformLoadInsecureConstructedMessage, name)); + return new DiagnosticResult(DoNotUseInsecureXSLTScriptExecutionAnalyzer.RuleDoNotUseInsecureXSLTScriptExecution).WithLocation(line, column).WithArguments(string.Format(MicrosoftNetFrameworkAnalyzersResources.XslCompiledTransformLoadInsecureConstructedMessage, name)); } private static DiagnosticResult GetCA3076LoadInsecureConstructedBasicResultAt(int line, int column, string name) { - return new DiagnosticResult(BasicDoNotUseInsecureXSLTScriptExecutionAnalyzer.RuleDoNotUseInsecureXSLTScriptExecution).WithLocation(line, column).WithArguments(string.Format(MicrosoftNetFrameworkAnalyzersResources.XslCompiledTransformLoadInsecureConstructedMessage, name)); + return new DiagnosticResult(DoNotUseInsecureXSLTScriptExecutionAnalyzer.RuleDoNotUseInsecureXSLTScriptExecution).WithLocation(line, column).WithArguments(string.Format(MicrosoftNetFrameworkAnalyzersResources.XslCompiledTransformLoadInsecureConstructedMessage, name)); } [Fact] diff --git a/src/NetAnalyzers/VisualBasic/Microsoft.NetFramework.Analyzers/BasicDoNotUseInsecureXSLTScriptExecution.vb b/src/NetAnalyzers/VisualBasic/Microsoft.NetFramework.Analyzers/BasicDoNotUseInsecureXSLTScriptExecution.vb deleted file mode 100644 index e6177965f9..0000000000 --- a/src/NetAnalyzers/VisualBasic/Microsoft.NetFramework.Analyzers/BasicDoNotUseInsecureXSLTScriptExecution.vb +++ /dev/null @@ -1,25 +0,0 @@ -' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - - -Imports Microsoft.NetFramework.Analyzers -Imports Microsoft.CodeAnalysis -Imports Microsoft.CodeAnalysis.Diagnostics -Imports Microsoft.CodeAnalysis.VisualBasic -Imports Microsoft.NetFramework.Analyzers.Helpers -Imports Microsoft.NetFramework.VisualBasic.Analyzers.Helpers - -Namespace Microsoft.NetFramework.VisualBasic.Analyzers - - Public NotInheritable Class BasicDoNotUseInsecureXSLTScriptExecutionAnalyzer - Inherits DoNotUseInsecureXSLTScriptExecutionAnalyzer(Of SyntaxKind) - Protected Overrides Function GetAnalyzer(context As CodeBlockStartAnalysisContext(Of SyntaxKind), types As CompilationSecurityTypes) As SyntaxNodeAnalyzer - Dim analyzer As New SyntaxNodeAnalyzer(types, BasicSyntaxNodeHelper.DefaultInstance) - context.RegisterSyntaxNodeAction(AddressOf analyzer.AnalyzeNode, SyntaxKind.InvocationExpression, - SyntaxKind.ObjectCreationExpression, - SyntaxKind.SimpleAssignmentStatement, - SyntaxKind.VariableDeclarator, - SyntaxKind.NamedFieldInitializer) - Return analyzer - End Function - End Class -End Namespace From e886a01f7bcf783d6931f961946844bf8b5680df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Sun, 20 Dec 2020 23:47:21 +0100 Subject: [PATCH 2/9] Use helpers + use extension methods --- .../DoNotUseInsecureDtdProcessing.cs | 8 +- .../DoNotUseInsecureXSLTScriptExecution.cs | 92 ++++++++----------- .../Helpers/SecurityDiagnosticHelpers.cs | 62 +++++-------- 3 files changed, 62 insertions(+), 100 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureDtdProcessing.cs b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureDtdProcessing.cs index 29835b3fb1..7df034700a 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureDtdProcessing.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureDtdProcessing.cs @@ -257,7 +257,7 @@ private void AnalyzeMethodOverloads(OperationAnalysisContext context, IMethodSym } else if (method.MatchMethodDerivedByName(_xmlTypes.XmlReader, SecurityMemberNames.Create)) { - int xmlReaderSettingsIndex = SecurityDiagnosticHelpers.GetXmlReaderSettingsParameterIndex(method, _xmlTypes); + int xmlReaderSettingsIndex = method.GetXmlReaderSettingsParameterIndex(_xmlTypes); if (xmlReaderSettingsIndex < 0) { @@ -305,15 +305,15 @@ private void AnalyzeObjectCreationInternal(OperationAnalysisContext context, ISy _objectCreationOperationsAnalyzed.Add(objCreation); } - if (SecurityDiagnosticHelpers.IsXmlDocumentCtorDerived(objCreation.Constructor, _xmlTypes)) + if (objCreation.Constructor.IsXmlDocumentCtorDerived(_xmlTypes)) { AnalyzeObjectCreationForXmlDocument(context, variable, objCreation); } - else if (SecurityDiagnosticHelpers.IsXmlTextReaderCtorDerived(objCreation.Constructor, _xmlTypes)) + else if (objCreation.Constructor.IsXmlTextReaderCtorDerived(_xmlTypes)) { AnalyzeObjectCreationForXmlTextReader(context, variable, objCreation); } - else if (SecurityDiagnosticHelpers.IsXmlReaderSettingsCtor(objCreation.Constructor, _xmlTypes)) + else if (objCreation.Constructor.IsXmlReaderSettingsCtor(_xmlTypes)) { AnalyzeObjectCreationForXmlReaderSettings(variable, objCreation); } diff --git a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs index 990729fe7f..a9f298de9a 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs @@ -95,19 +95,21 @@ public OperationBlockAnalyzer(CompilationSecurityTypes xmlTypes, ISymbol enclosi _enclosingSymbol = enclosingSymbol; } - public void AnalyzeNodeForXslCompiledTransformLoad(IMethodSymbol methodSymbol, ImmutableArray arguments, - Action reportDiagnostic) + public void AnalyzeNodeForXslCompiledTransformLoad(IMethodSymbol methodSymbol, + ImmutableArray arguments, Action reportDiagnostic) { - if (!SecurityDiagnosticHelpers.IsXslCompiledTransformLoad(methodSymbol, _xmlTypes)) + if (!methodSymbol.IsXslCompiledTransformLoad(_xmlTypes)) { return; } - int xmlResolverIndex = SecurityDiagnosticHelpers.GetXmlResolverParameterIndex(methodSymbol, _xmlTypes); - int xsltSettingsIndex = SecurityDiagnosticHelpers.GetXsltSettingsParameterIndex(methodSymbol, _xmlTypes); + int xmlResolverIndex = methodSymbol.GetXmlResolverParameterIndex(_xmlTypes); + int xsltSettingsIndex = methodSymbol.GetXsltSettingsParameterIndex(_xmlTypes); - // Overloads with no XmlResolver and XstlSettings specified are secure since they all have following behavior: - // 1. An XmlUrlResolver with no user credentials is used to process any xsl:import or xsl:include elements. + // Overloads with no XmlResolver and XstlSettings specified are secure since they all have + // following behavior: + // 1. An XmlUrlResolver with no user credentials is used to process any xsl:import + // or xsl:include elements. // 2. The document() function is disabled. // 3. Embedded scripts are not supported. if (xmlResolverIndex < 0 || xsltSettingsIndex < 0) @@ -122,34 +124,28 @@ public void AnalyzeNodeForXslCompiledTransformLoad(IMethodSymbol methodSymbol, I } var isSecureResolver = resolverArgumentOperation.HasNullConstantValue() - || SecurityDiagnosticHelpers.IsXmlSecureResolverType(resolverArgumentOperation.Type, _xmlTypes); + || resolverArgumentOperation.Type.IsXmlSecureResolverType(_xmlTypes); var settingsOperation = arguments[xsltSettingsIndex].Value; - ISymbol? settingsSymbol = settingsOperation.Kind switch - { - OperationKind.ParameterReference => ((IParameterReferenceOperation)settingsOperation).Parameter, - OperationKind.LocalReference => ((ILocalReferenceOperation)settingsOperation).Local, - OperationKind.FieldReference => ((IFieldReferenceOperation)settingsOperation).Field, - OperationKind.PropertyReference => ((IPropertyReferenceOperation)settingsOperation).Property, - _ => null, - }; + var settingsSymbol = settingsOperation.GetReferencedMemberOrLocalOrParameter(); bool isSecureSettings; bool isSetInBlock; // 1. pass null or XsltSettings.Default as XsltSetting : secure - if (settingsSymbol == null || SecurityDiagnosticHelpers.IsXsltSettingsDefaultProperty(settingsSymbol as IPropertySymbol, _xmlTypes)) + if (settingsSymbol is null || (settingsSymbol as IPropertySymbol).IsXsltSettingsDefaultProperty(_xmlTypes)) { isSetInBlock = true; isSecureSettings = true; } // 2. XsltSettings.TrustedXslt : insecure - else if (SecurityDiagnosticHelpers.IsXsltSettingsTrustedXsltProperty(settingsSymbol as IPropertySymbol, _xmlTypes)) + else if ((settingsSymbol as IPropertySymbol).IsXsltSettingsTrustedXsltProperty(_xmlTypes)) { isSetInBlock = true; isSecureSettings = false; } - // 3. check xsltSettingsEnvironments, if IsScriptDisabled && IsDocumentFunctionDisabled then secure, else insecure + // 3. check xsltSettingsEnvironments, if IsScriptDisabled && IsDocumentFunctionDisabled then secure, + // else insecure else if (_xsltSettingsEnvironments.TryGetValue(settingsSymbol, out XsltSettingsEnvironment env)) { isSetInBlock = false; @@ -172,20 +168,19 @@ public void AnalyzeNodeForXslCompiledTransformLoad(IMethodSymbol methodSymbol, I ); var invocationOrObjCreation = settingsOperation.Parent.Parent; - reportDiagnostic(invocationOrObjCreation.CreateDiagnostic(RuleDoNotUseInsecureXSLTScriptExecution, message)); + RoslynDebug.Assert(invocationOrObjCreation.Kind is OperationKind.Invocation + or OperationKind.ObjectCreation); + reportDiagnostic( + invocationOrObjCreation.CreateDiagnostic(RuleDoNotUseInsecureXSLTScriptExecution, message)); } } public void AnalyzeNodeForXsltSettings(IOperation lhs, IOperation rhs) { - ISymbol? lhsSymbol = lhs.Kind switch + var lhsSymbol = lhs.Kind switch { OperationKind.VariableDeclarator => ((IVariableDeclaratorOperation)lhs).Symbol, - OperationKind.PropertyReference => ((IPropertyReferenceOperation)lhs).Property, - OperationKind.ParameterReference => ((IParameterReferenceOperation)lhs).Parameter, - OperationKind.LocalReference => ((ILocalReferenceOperation)lhs).Local, - OperationKind.FieldReference => ((IFieldReferenceOperation)lhs).Field, - _ => null, + _ => lhs.GetReferencedMemberOrLocalOrParameter(), }; if (lhsSymbol is null) { @@ -200,7 +195,7 @@ public void AnalyzeNodeForXsltSettings(IOperation lhs, IOperation rhs) }; IPropertySymbol? rhsPropertySymbol = (rhs as IPropertyReferenceOperation)?.Property; - if (SecurityDiagnosticHelpers.IsXsltSettingsCtor(rhsMethodSymbol, _xmlTypes)) + if (rhsMethodSymbol.IsXsltSettingsCtor(_xmlTypes)) { XsltSettingsEnvironment env = new XsltSettingsEnvironment(); _xsltSettingsEnvironments[lhsSymbol] = env; @@ -222,16 +217,13 @@ public void AnalyzeNodeForXsltSettings(IOperation lhs, IOperation rhs) OperationKind.ObjectCreation => ((IObjectCreationOperation)rhs).Arguments, _ => ImmutableArray.Empty, }; - env.IsDocumentFunctionDisabled = arguments[0].Value.TryGetBoolConstantValue(out var value) && !value; + env.IsDocumentFunctionDisabled = arguments[0].Value.TryGetBoolConstantValue(out var value) + && !value; env.IsScriptDisabled = arguments[1].Value.TryGetBoolConstantValue(out value) && !value; } - var initializers = rhs.Kind switch - { - OperationKind.ObjectCreation => ((IObjectCreationOperation)rhs).Initializer?.Initializers, - _ => null, - } ?? ImmutableArray.Empty; - + var initializers = (rhs as IObjectCreationOperation)?.Initializer?.Initializers + ?? ImmutableArray.Empty; foreach (var arg in initializers) { if (arg is not ISimpleAssignmentOperation assignment) @@ -243,17 +235,17 @@ public void AnalyzeNodeForXsltSettings(IOperation lhs, IOperation rhs) var argRhs = assignment.Value; // anything other than a constant false is treated as true - if (SecurityDiagnosticHelpers.IsXsltSettingsEnableDocumentFunctionProperty(argLhsPropertySymbol, _xmlTypes)) + if (argLhsPropertySymbol.IsXsltSettingsEnableDocumentFunctionProperty(_xmlTypes)) { env.IsDocumentFunctionDisabled = argRhs.TryGetBoolConstantValue(out var value) && !value; } - else if (SecurityDiagnosticHelpers.IsXsltSettingsEnableScriptProperty(argLhsPropertySymbol, _xmlTypes)) + else if (argLhsPropertySymbol.IsXsltSettingsEnableScriptProperty(_xmlTypes)) { env.IsScriptDisabled = argRhs.TryGetBoolConstantValue(out var value) && !value; } } } - else if (SecurityDiagnosticHelpers.IsXsltSettingsDefaultProperty(rhsPropertySymbol, _xmlTypes)) + else if (rhsPropertySymbol.IsXsltSettingsDefaultProperty(_xmlTypes)) { XsltSettingsEnvironment env = new XsltSettingsEnvironment(); _xsltSettingsEnvironments[lhsSymbol] = env; @@ -265,7 +257,7 @@ public void AnalyzeNodeForXsltSettings(IOperation lhs, IOperation rhs) env.IsDocumentFunctionDisabled = true; env.IsScriptDisabled = true; } - else if (SecurityDiagnosticHelpers.IsXsltSettingsTrustedXsltProperty(rhsPropertySymbol, _xmlTypes)) + else if (rhsPropertySymbol.IsXsltSettingsTrustedXsltProperty(_xmlTypes)) { XsltSettingsEnvironment env = new XsltSettingsEnvironment(); _xsltSettingsEnvironments[lhsSymbol] = env; @@ -278,8 +270,10 @@ public void AnalyzeNodeForXsltSettings(IOperation lhs, IOperation rhs) else { var lhsPropertySymbol = lhsSymbol as IPropertySymbol; - bool isXlstSettingsEnableDocumentFunctionProperty = SecurityDiagnosticHelpers.IsXsltSettingsEnableDocumentFunctionProperty(lhsPropertySymbol, _xmlTypes); - bool isXlstSettingsEnableScriptProperty = SecurityDiagnosticHelpers.IsXsltSettingsEnableScriptProperty(lhsPropertySymbol, _xmlTypes); + bool isXlstSettingsEnableDocumentFunctionProperty = + lhsPropertySymbol.IsXsltSettingsEnableDocumentFunctionProperty(_xmlTypes); + bool isXlstSettingsEnableScriptProperty = + lhsPropertySymbol.IsXsltSettingsEnableScriptProperty(_xmlTypes); if (!isXlstSettingsEnableDocumentFunctionProperty && !isXlstSettingsEnableScriptProperty) @@ -287,22 +281,8 @@ public void AnalyzeNodeForXsltSettings(IOperation lhs, IOperation rhs) return; } - IOperation? lhsExpression = lhs.Kind switch - { - OperationKind.FieldReference => ((IFieldReferenceOperation)lhs).Instance, - OperationKind.PropertyReference => ((IPropertyReferenceOperation)lhs).Instance, - _ => null, - }; - - ISymbol? lhsExpressionSymbol = lhsExpression?.Kind switch - { - OperationKind.ParameterReference => ((IParameterReferenceOperation)lhsExpression).Parameter, - OperationKind.LocalReference => ((ILocalReferenceOperation)lhsExpression).Local, - OperationKind.FieldReference => ((IFieldReferenceOperation)lhsExpression).Field, - OperationKind.PropertyReference => ((IPropertyReferenceOperation)lhsExpression).Property, - _ => null, - }; - + IOperation? lhsExpression = (lhs as IMemberReferenceOperation)?.Instance; + ISymbol? lhsExpressionSymbol = lhsExpression?.GetReferencedMemberOrLocalOrParameter(); if (lhsExpressionSymbol is null) { return; diff --git a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs index d680061a43..4877480d23 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs @@ -12,14 +12,12 @@ namespace Microsoft.NetFramework.Analyzers.Helpers { public static class SecurityDiagnosticHelpers { - public static bool IsXslCompiledTransformLoad([NotNullWhen(returnValue: true)] IMethodSymbol? method, CompilationSecurityTypes xmlTypes) - { - return method != null + public static bool IsXslCompiledTransformLoad([NotNullWhen(returnValue: true)] this IMethodSymbol? method, CompilationSecurityTypes xmlTypes) + => method != null && xmlTypes.XslCompiledTransform != null && method.MatchMethodByName(xmlTypes.XslCompiledTransform, SecurityMemberNames.Load); - } - public static bool IsXmlDocumentCtorDerived([NotNullWhen(returnValue: true)] IMethodSymbol? method, CompilationSecurityTypes xmlTypes) + public static bool IsXmlDocumentCtorDerived([NotNullWhen(returnValue: true)] this IMethodSymbol? method, CompilationSecurityTypes xmlTypes) { return method != null && xmlTypes.XmlDocument != null @@ -31,7 +29,7 @@ public static bool IsXmlDocumentXmlResolverProperty([NotNullWhen(returnValue: tr return IsSpecifiedProperty(symbol, xmlTypes.XmlDocument, SecurityMemberNames.XmlResolver); } - public static bool IsXmlTextReaderCtorDerived([NotNullWhen(returnValue: true)] IMethodSymbol? method, CompilationSecurityTypes xmlTypes) + public static bool IsXmlTextReaderCtorDerived([NotNullWhen(returnValue: true)] this IMethodSymbol? method, CompilationSecurityTypes xmlTypes) { return method != null && xmlTypes.XmlTextReader != null @@ -58,7 +56,7 @@ public static bool IsXmlTextReaderDtdProcessingProperty([NotNullWhen(returnValue return IsSpecifiedProperty(symbol, xmlTypes.XmlTextReader, SecurityMemberNames.DtdProcessing); } - public static bool IsXmlReaderSettingsCtor([NotNullWhen(returnValue: true)] IMethodSymbol? method, CompilationSecurityTypes xmlTypes) + public static bool IsXmlReaderSettingsCtor([NotNullWhen(returnValue: true)] this IMethodSymbol? method, CompilationSecurityTypes xmlTypes) { return method != null && xmlTypes.XmlReaderSettings != null @@ -80,32 +78,22 @@ public static bool IsXmlReaderSettingsMaxCharactersFromEntitiesProperty([NotNull return IsSpecifiedProperty(symbol, xmlTypes.XmlReaderSettings, SecurityMemberNames.MaxCharactersFromEntities); } - public static bool IsXsltSettingsCtor([NotNullWhen(returnValue: true)] IMethodSymbol? method, CompilationSecurityTypes xmlTypes) - { - return method != null + public static bool IsXsltSettingsCtor([NotNullWhen(returnValue: true)] this IMethodSymbol? method, CompilationSecurityTypes xmlTypes) + => method != null && xmlTypes.XsltSettings != null && method.MatchMethodByName(xmlTypes.XsltSettings, WellKnownMemberNames.InstanceConstructorName); - } - public static bool IsXsltSettingsTrustedXsltProperty([NotNullWhen(returnValue: true)] IPropertySymbol? symbol, CompilationSecurityTypes xmlTypes) - { - return IsSpecifiedProperty(symbol, xmlTypes.XsltSettings, SecurityMemberNames.TrustedXslt); - } + public static bool IsXsltSettingsTrustedXsltProperty([NotNullWhen(returnValue: true)] this IPropertySymbol? symbol, CompilationSecurityTypes xmlTypes) + => IsSpecifiedProperty(symbol, xmlTypes.XsltSettings, SecurityMemberNames.TrustedXslt); - public static bool IsXsltSettingsDefaultProperty([NotNullWhen(returnValue: true)] IPropertySymbol? symbol, CompilationSecurityTypes xmlTypes) - { - return IsSpecifiedProperty(symbol, xmlTypes.XsltSettings, SecurityMemberNames.Default); - } + public static bool IsXsltSettingsDefaultProperty([NotNullWhen(returnValue: true)] this IPropertySymbol? symbol, CompilationSecurityTypes xmlTypes) + => IsSpecifiedProperty(symbol, xmlTypes.XsltSettings, SecurityMemberNames.Default); - public static bool IsXsltSettingsEnableDocumentFunctionProperty([NotNullWhen(returnValue: true)] IPropertySymbol? symbol, CompilationSecurityTypes xmlTypes) - { - return IsSpecifiedProperty(symbol, xmlTypes.XsltSettings, SecurityMemberNames.EnableDocumentFunction); - } + public static bool IsXsltSettingsEnableDocumentFunctionProperty([NotNullWhen(returnValue: true)] this IPropertySymbol? symbol, CompilationSecurityTypes xmlTypes) + => IsSpecifiedProperty(symbol, xmlTypes.XsltSettings, SecurityMemberNames.EnableDocumentFunction); - public static bool IsXsltSettingsEnableScriptProperty([NotNullWhen(returnValue: true)] IPropertySymbol? symbol, CompilationSecurityTypes xmlTypes) - { - return IsSpecifiedProperty(symbol, xmlTypes.XsltSettings, SecurityMemberNames.EnableScript); - } + public static bool IsXsltSettingsEnableScriptProperty([NotNullWhen(returnValue: true)] this IPropertySymbol? symbol, CompilationSecurityTypes xmlTypes) + => IsSpecifiedProperty(symbol, xmlTypes.XsltSettings, SecurityMemberNames.EnableScript); public static bool IsXmlResolverType([NotNullWhen(returnValue: true)] ITypeSymbol? symbol, CompilationSecurityTypes xmlTypes) { @@ -113,11 +101,9 @@ public static bool IsXmlResolverType([NotNullWhen(returnValue: true)] ITypeSymbo && symbol.DerivesFrom(xmlTypes.XmlResolver, baseTypesOnly: true); } - public static bool IsXmlSecureResolverType([NotNullWhen(returnValue: true)] ITypeSymbol? symbol, CompilationSecurityTypes xmlTypes) - { - return symbol != null + public static bool IsXmlSecureResolverType([NotNullWhen(returnValue: true)] this ITypeSymbol? symbol, CompilationSecurityTypes xmlTypes) + => symbol != null && symbol.DerivesFrom(xmlTypes.XmlSecureResolver, baseTypesOnly: true); - } public static bool IsXsltSettingsType([NotNullWhen(returnValue: true)] ITypeSymbol? symbol, CompilationSecurityTypes xmlTypes) { @@ -129,17 +115,13 @@ public static bool IsXmlReaderSettingsType([NotNullWhen(returnValue: true)] ITyp return Equals(symbol, xmlTypes.XmlReaderSettings); } - public static int GetXmlResolverParameterIndex(IMethodSymbol? method, CompilationSecurityTypes xmlTypes) - { - return GetSpecifiedParameterIndex(method, xmlTypes, IsXmlResolverType); - } + public static int GetXmlResolverParameterIndex(this IMethodSymbol? method, CompilationSecurityTypes xmlTypes) + => GetSpecifiedParameterIndex(method, xmlTypes, IsXmlResolverType); - public static int GetXsltSettingsParameterIndex(IMethodSymbol? method, CompilationSecurityTypes xmlTypes) - { - return GetSpecifiedParameterIndex(method, xmlTypes, IsXsltSettingsType); - } + public static int GetXsltSettingsParameterIndex(this IMethodSymbol? method, CompilationSecurityTypes xmlTypes) + => GetSpecifiedParameterIndex(method, xmlTypes, IsXsltSettingsType); - public static int GetXmlReaderSettingsParameterIndex(IMethodSymbol? method, CompilationSecurityTypes xmlTypes) + public static int GetXmlReaderSettingsParameterIndex(this IMethodSymbol? method, CompilationSecurityTypes xmlTypes) { return GetSpecifiedParameterIndex(method, xmlTypes, IsXmlReaderSettingsType); } From 6abf4dadc4ef8c3f60881ec6b0e516d2aafbeed6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Mon, 21 Dec 2020 10:02:20 +0100 Subject: [PATCH 3/9] Trigger CI From c45476d4882e2f0f56e4612e02f0c61e9acd0c1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Wed, 6 Jan 2021 09:01:11 +0100 Subject: [PATCH 4/9] Update src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs Co-authored-by: Paul Ming --- .../Helpers/SecurityDiagnosticHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs index 4877480d23..bfc26324ea 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/Helpers/SecurityDiagnosticHelpers.cs @@ -246,7 +246,7 @@ public static string GetNonEmptyParentName(SyntaxNode current, SemanticModel mod } /// - /// Get non-empty class or method name which encloses the current syntax node + /// Get class or method name which encloses the current symbol node /// public static string GetNonEmptyParentName(ISymbol symbol) { From b5be5576eb24954fdaca83d69fb0d57ff7f6d045 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Fri, 8 Jan 2021 19:35:35 +0100 Subject: [PATCH 5/9] Fix AnalyzerReleases.Unshipped.md --- src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md b/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md index cdf4f1397e..0c17fc7850 100644 --- a/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md +++ b/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md @@ -1 +1,5 @@ ; Please do not edit this file manually, it should only be updated through code fix application. +### Changed Rules +Rule ID | New Category | New Severity | Old Category | Old Severity | Notes +--------|--------------|--------------|--------------|--------------|------- +CA3076 | Security | Info | Security | Hidden | DoNotUseInsecureXSLTScriptExecutionAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3076) \ No newline at end of file From 56606bdaf0e2686aaeb5b84f44676e45376dcf58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Sat, 9 Jan 2021 00:11:38 +0100 Subject: [PATCH 6/9] Run pack --- .../Microsoft.CodeAnalysis.NetAnalyzers.md | 2 +- .../Microsoft.CodeAnalysis.NetAnalyzers.sarif | 58 +++++++------------ 2 files changed, 21 insertions(+), 39 deletions(-) diff --git a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md index 1dff4237dc..a232efa756 100644 --- a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md +++ b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md @@ -2324,7 +2324,7 @@ Providing an insecure XsltSettings instance and an insecure XmlResolver instance |-|-| |Category|Security| |Enabled|True| -|Severity|Hidden| +|Severity|Info| |CodeFix|False| --- diff --git a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif index 3f418be3a1..6e8821dac7 100644 --- a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif +++ b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif @@ -420,25 +420,6 @@ ] } }, - "CA3076": { - "id": "CA3076", - "shortDescription": "Insecure XSLT script processing.", - "fullDescription": "Providing an insecure XsltSettings instance and an insecure XmlResolver instance to XslCompiledTransform.Load method is potentially unsafe as it allows processing script within XSL, which on an untrusted XSL input may lead to malicious code execution. Either replace the insecure XsltSettings argument with XsltSettings.Default or an instance that has disabled document function and script execution, or replace the XmlResolver argument with null or an XmlSecureResolver instance. This message may be suppressed if the input is known to be from a trusted source and external resource resolution from locations that are not known in advance must be supported.", - "defaultLevel": "hidden", - "helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3076", - "properties": { - "category": "Security", - "isEnabledByDefault": true, - "typeName": "CSharpDoNotUseInsecureXSLTScriptExecutionAnalyzer", - "languages": [ - "C#" - ], - "tags": [ - "Telemetry", - "EnabledRuleInAggressiveMode" - ] - } - }, "CA3077": { "id": "CA3077", "shortDescription": "Insecure Processing in API Design, XmlDocument and XmlTextReader", @@ -4001,6 +3982,26 @@ ] } }, + "CA3076": { + "id": "CA3076", + "shortDescription": "Insecure XSLT script processing.", + "fullDescription": "Providing an insecure XsltSettings instance and an insecure XmlResolver instance to XslCompiledTransform.Load method is potentially unsafe as it allows processing script within XSL, which on an untrusted XSL input may lead to malicious code execution. Either replace the insecure XsltSettings argument with XsltSettings.Default or an instance that has disabled document function and script execution, or replace the XmlResolver argument with null or an XmlSecureResolver instance. This message may be suppressed if the input is known to be from a trusted source and external resource resolution from locations that are not known in advance must be supported.", + "defaultLevel": "note", + "helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3076", + "properties": { + "category": "Security", + "isEnabledByDefault": true, + "typeName": "DoNotUseInsecureXSLTScriptExecutionAnalyzer", + "languages": [ + "C#", + "Visual Basic" + ], + "tags": [ + "Telemetry", + "EnabledRuleInAggressiveMode" + ] + } + }, "CA3147": { "id": "CA3147", "shortDescription": "Mark Verb Handlers With Validate Antiforgery Token", @@ -5496,25 +5497,6 @@ ] } }, - "CA3076": { - "id": "CA3076", - "shortDescription": "Insecure XSLT script processing.", - "fullDescription": "Providing an insecure XsltSettings instance and an insecure XmlResolver instance to XslCompiledTransform.Load method is potentially unsafe as it allows processing script within XSL, which on an untrusted XSL input may lead to malicious code execution. Either replace the insecure XsltSettings argument with XsltSettings.Default or an instance that has disabled document function and script execution, or replace the XmlResolver argument with null or an XmlSecureResolver instance. This message may be suppressed if the input is known to be from a trusted source and external resource resolution from locations that are not known in advance must be supported.", - "defaultLevel": "hidden", - "helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3076", - "properties": { - "category": "Security", - "isEnabledByDefault": true, - "typeName": "BasicDoNotUseInsecureXSLTScriptExecutionAnalyzer", - "languages": [ - "Visual Basic" - ], - "tags": [ - "Telemetry", - "EnabledRuleInAggressiveMode" - ] - } - }, "CA3077": { "id": "CA3077", "shortDescription": "Insecure Processing in API Design, XmlDocument and XmlTextReader", From cbed397ec9c1efd0636c06e412072707a25fe143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 12 Jan 2021 10:30:54 +0100 Subject: [PATCH 7/9] Update src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs Co-authored-by: Paul Ming --- .../DoNotUseInsecureXSLTScriptExecution.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs index a9f298de9a..3a5c329428 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetFramework.Analyzers/DoNotUseInsecureXSLTScriptExecution.cs @@ -23,7 +23,7 @@ public sealed class DoNotUseInsecureXSLTScriptExecutionAnalyzer : DiagnosticAnal SecurityDiagnosticHelpers.GetLocalizableResourceString(nameof(MicrosoftNetFrameworkAnalyzersResources.InsecureXsltScriptProcessingMessage)), SecurityDiagnosticHelpers.GetLocalizableResourceString(nameof(MicrosoftNetFrameworkAnalyzersResources.DoNotUseInsecureDtdProcessingGenericMessage)), DiagnosticCategory.Security, - RuleLevel.IdeSuggestion, + RuleLevel.IdeHidden_BulkConfigurable, SecurityDiagnosticHelpers.GetLocalizableResourceString(nameof(MicrosoftNetFrameworkAnalyzersResources.DoNotUseInsecureXSLTScriptExecutionDescription)), isPortedFxCopRule: false, isDataflowRule: false); From 18456e48fd5a9120d2d13e626f80c253256175c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 12 Jan 2021 10:32:27 +0100 Subject: [PATCH 8/9] Revert "Run pack" and "Fix AnalyzerReleases.Unshipped.md" --- .../Core/AnalyzerReleases.Unshipped.md | 4 -- .../Microsoft.CodeAnalysis.NetAnalyzers.md | 2 +- .../Microsoft.CodeAnalysis.NetAnalyzers.sarif | 58 ++++++++++++------- 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md b/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md index 0c17fc7850..cdf4f1397e 100644 --- a/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md +++ b/src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md @@ -1,5 +1 @@ ; Please do not edit this file manually, it should only be updated through code fix application. -### Changed Rules -Rule ID | New Category | New Severity | Old Category | Old Severity | Notes ---------|--------------|--------------|--------------|--------------|------- -CA3076 | Security | Info | Security | Hidden | DoNotUseInsecureXSLTScriptExecutionAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3076) \ No newline at end of file diff --git a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md index a232efa756..1dff4237dc 100644 --- a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md +++ b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.md @@ -2324,7 +2324,7 @@ Providing an insecure XsltSettings instance and an insecure XmlResolver instance |-|-| |Category|Security| |Enabled|True| -|Severity|Info| +|Severity|Hidden| |CodeFix|False| --- diff --git a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif index 6e8821dac7..3f418be3a1 100644 --- a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif +++ b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif @@ -420,6 +420,25 @@ ] } }, + "CA3076": { + "id": "CA3076", + "shortDescription": "Insecure XSLT script processing.", + "fullDescription": "Providing an insecure XsltSettings instance and an insecure XmlResolver instance to XslCompiledTransform.Load method is potentially unsafe as it allows processing script within XSL, which on an untrusted XSL input may lead to malicious code execution. Either replace the insecure XsltSettings argument with XsltSettings.Default or an instance that has disabled document function and script execution, or replace the XmlResolver argument with null or an XmlSecureResolver instance. This message may be suppressed if the input is known to be from a trusted source and external resource resolution from locations that are not known in advance must be supported.", + "defaultLevel": "hidden", + "helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3076", + "properties": { + "category": "Security", + "isEnabledByDefault": true, + "typeName": "CSharpDoNotUseInsecureXSLTScriptExecutionAnalyzer", + "languages": [ + "C#" + ], + "tags": [ + "Telemetry", + "EnabledRuleInAggressiveMode" + ] + } + }, "CA3077": { "id": "CA3077", "shortDescription": "Insecure Processing in API Design, XmlDocument and XmlTextReader", @@ -3982,26 +4001,6 @@ ] } }, - "CA3076": { - "id": "CA3076", - "shortDescription": "Insecure XSLT script processing.", - "fullDescription": "Providing an insecure XsltSettings instance and an insecure XmlResolver instance to XslCompiledTransform.Load method is potentially unsafe as it allows processing script within XSL, which on an untrusted XSL input may lead to malicious code execution. Either replace the insecure XsltSettings argument with XsltSettings.Default or an instance that has disabled document function and script execution, or replace the XmlResolver argument with null or an XmlSecureResolver instance. This message may be suppressed if the input is known to be from a trusted source and external resource resolution from locations that are not known in advance must be supported.", - "defaultLevel": "note", - "helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3076", - "properties": { - "category": "Security", - "isEnabledByDefault": true, - "typeName": "DoNotUseInsecureXSLTScriptExecutionAnalyzer", - "languages": [ - "C#", - "Visual Basic" - ], - "tags": [ - "Telemetry", - "EnabledRuleInAggressiveMode" - ] - } - }, "CA3147": { "id": "CA3147", "shortDescription": "Mark Verb Handlers With Validate Antiforgery Token", @@ -5497,6 +5496,25 @@ ] } }, + "CA3076": { + "id": "CA3076", + "shortDescription": "Insecure XSLT script processing.", + "fullDescription": "Providing an insecure XsltSettings instance and an insecure XmlResolver instance to XslCompiledTransform.Load method is potentially unsafe as it allows processing script within XSL, which on an untrusted XSL input may lead to malicious code execution. Either replace the insecure XsltSettings argument with XsltSettings.Default or an instance that has disabled document function and script execution, or replace the XmlResolver argument with null or an XmlSecureResolver instance. This message may be suppressed if the input is known to be from a trusted source and external resource resolution from locations that are not known in advance must be supported.", + "defaultLevel": "hidden", + "helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3076", + "properties": { + "category": "Security", + "isEnabledByDefault": true, + "typeName": "BasicDoNotUseInsecureXSLTScriptExecutionAnalyzer", + "languages": [ + "Visual Basic" + ], + "tags": [ + "Telemetry", + "EnabledRuleInAggressiveMode" + ] + } + }, "CA3077": { "id": "CA3077", "shortDescription": "Insecure Processing in API Design, XmlDocument and XmlTextReader", From 402986b25a9a299e0b081681f58e817c17d7ad2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 12 Jan 2021 10:56:01 +0100 Subject: [PATCH 9/9] Run pack --- .../Microsoft.CodeAnalysis.NetAnalyzers.sarif | 58 +++++++------------ 1 file changed, 20 insertions(+), 38 deletions(-) diff --git a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif index 3f418be3a1..017f323cab 100644 --- a/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif +++ b/src/NetAnalyzers/Microsoft.CodeAnalysis.NetAnalyzers.sarif @@ -420,25 +420,6 @@ ] } }, - "CA3076": { - "id": "CA3076", - "shortDescription": "Insecure XSLT script processing.", - "fullDescription": "Providing an insecure XsltSettings instance and an insecure XmlResolver instance to XslCompiledTransform.Load method is potentially unsafe as it allows processing script within XSL, which on an untrusted XSL input may lead to malicious code execution. Either replace the insecure XsltSettings argument with XsltSettings.Default or an instance that has disabled document function and script execution, or replace the XmlResolver argument with null or an XmlSecureResolver instance. This message may be suppressed if the input is known to be from a trusted source and external resource resolution from locations that are not known in advance must be supported.", - "defaultLevel": "hidden", - "helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3076", - "properties": { - "category": "Security", - "isEnabledByDefault": true, - "typeName": "CSharpDoNotUseInsecureXSLTScriptExecutionAnalyzer", - "languages": [ - "C#" - ], - "tags": [ - "Telemetry", - "EnabledRuleInAggressiveMode" - ] - } - }, "CA3077": { "id": "CA3077", "shortDescription": "Insecure Processing in API Design, XmlDocument and XmlTextReader", @@ -4001,6 +3982,26 @@ ] } }, + "CA3076": { + "id": "CA3076", + "shortDescription": "Insecure XSLT script processing.", + "fullDescription": "Providing an insecure XsltSettings instance and an insecure XmlResolver instance to XslCompiledTransform.Load method is potentially unsafe as it allows processing script within XSL, which on an untrusted XSL input may lead to malicious code execution. Either replace the insecure XsltSettings argument with XsltSettings.Default or an instance that has disabled document function and script execution, or replace the XmlResolver argument with null or an XmlSecureResolver instance. This message may be suppressed if the input is known to be from a trusted source and external resource resolution from locations that are not known in advance must be supported.", + "defaultLevel": "hidden", + "helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3076", + "properties": { + "category": "Security", + "isEnabledByDefault": true, + "typeName": "DoNotUseInsecureXSLTScriptExecutionAnalyzer", + "languages": [ + "C#", + "Visual Basic" + ], + "tags": [ + "Telemetry", + "EnabledRuleInAggressiveMode" + ] + } + }, "CA3147": { "id": "CA3147", "shortDescription": "Mark Verb Handlers With Validate Antiforgery Token", @@ -5496,25 +5497,6 @@ ] } }, - "CA3076": { - "id": "CA3076", - "shortDescription": "Insecure XSLT script processing.", - "fullDescription": "Providing an insecure XsltSettings instance and an insecure XmlResolver instance to XslCompiledTransform.Load method is potentially unsafe as it allows processing script within XSL, which on an untrusted XSL input may lead to malicious code execution. Either replace the insecure XsltSettings argument with XsltSettings.Default or an instance that has disabled document function and script execution, or replace the XmlResolver argument with null or an XmlSecureResolver instance. This message may be suppressed if the input is known to be from a trusted source and external resource resolution from locations that are not known in advance must be supported.", - "defaultLevel": "hidden", - "helpUri": "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca3076", - "properties": { - "category": "Security", - "isEnabledByDefault": true, - "typeName": "BasicDoNotUseInsecureXSLTScriptExecutionAnalyzer", - "languages": [ - "Visual Basic" - ], - "tags": [ - "Telemetry", - "EnabledRuleInAggressiveMode" - ] - } - }, "CA3077": { "id": "CA3077", "shortDescription": "Insecure Processing in API Design, XmlDocument and XmlTextReader",