Use nullable references in security rules

src/Microsoft.NetCore.Analyzers/Core/Security/DoNotSerializeTypeWithPointerFields.cs

@@ -1,8 +1,5 @@
 // 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.
-#nullable disable
-
 using System.Collections.Concurrent;
 using System.Collections.Immutable;
 using System.Linq;
@@ -89,14 +86,15 @@ public sealed override void Initialize(AnalysisContext context)
                     // typeSymbol: The symbol of the type to be analyzed
                     // relatedFieldSymbol: When relatedFieldSymbol is null, traverse all descendants of typeSymbol to
                     //     find pointer fields; otherwise, traverse to find if relatedFieldSymbol is a pointer field
-                    void LookForSerializationWithPointerFields(ITypeSymbol typeSymbol, IFieldSymbol relatedFieldSymbol)
+                    void LookForSerializationWithPointerFields(ITypeSymbol typeSymbol, IFieldSymbol? relatedFieldSymbol)
                     {
                         if (typeSymbol is IPointerTypeSymbol pointerTypeSymbol)
                         {
                             // If the field is a valid pointer.
                             if (pointerTypeSymbol.PointedAtType.TypeKind == TypeKind.Struct ||
                                 pointerTypeSymbol.PointedAtType.TypeKind == TypeKind.Pointer)
                             {
+                                RoslynDebug.Assert(relatedFieldSymbol != null);
                                 pointerFields.TryAdd(relatedFieldSymbol, true);
                             }
                         }

src/Microsoft.NetCore.Analyzers/Core/Security/DoNotUseInsecureCryptographicAlgorithms.cs

@@ -1,8 +1,5 @@
 // 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.
-#nullable disable
-
 using System.Collections.Immutable;
 using Microsoft.NetCore.Analyzers.Security.Helpers;
 using Analyzer.Utilities;
@@ -109,8 +106,8 @@ public override void Initialize(AnalysisContext analysisContext)
                             }
 
                             INamedTypeSymbol type = method.ContainingType;
-                            DiagnosticDescriptor rule = null;
-                            string algorithmName = null;
+                            DiagnosticDescriptor rule;
+                            string algorithmName;
 
                             if (type.DerivesFrom(cryptTypes.MD5))
                             {
@@ -139,7 +136,7 @@ public override void Initialize(AnalysisContext analysisContext)
                                     && method.MetadataName == WellKnownMemberNames.InstanceConstructorName))
                             {
                                 rule = DoNotUseBrokenCryptographyRule;
-                                algorithmName = cryptTypes.DSA.Name;
+                                algorithmName = "DSA";
                             }
                             else if (type.DerivesFrom(cryptTypes.HMACMD5))
                             {
@@ -166,16 +163,17 @@ public override void Initialize(AnalysisContext analysisContext)
                                 rule = DoNotUseWeakCryptographyRule;
                                 algorithmName = cryptTypes.HMACRIPEMD160.Name;
                             }
-
-                            if (rule != null)
+                            else
                             {
-                                operationAnalysisContext.ReportDiagnostic(
-                                    Diagnostic.Create(
-                                        rule,
-                                        operationAnalysisContext.Operation.Syntax.GetLocation(),
-                                        operationAnalysisContext.ContainingSymbol.Name,
-                                        algorithmName));
+                                return;
                             }
+
+                            operationAnalysisContext.ReportDiagnostic(
+                                Diagnostic.Create(
+                                    rule,
+                                    operationAnalysisContext.Operation.Syntax.GetLocation(),
+                                    operationAnalysisContext.ContainingSymbol.Name,
+                                    algorithmName));
                         },
                         OperationKind.Invocation,
                         OperationKind.ObjectCreation);

src/Microsoft.NetCore.Analyzers/Core/Security/SourceTriggeredTaintedDataAnalyzerBase.cs

@@ -1,8 +1,5 @@
 // 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.
-#nullable disable
-
 using System;
 using System.Collections.Immutable;
 using System.Linq;
@@ -85,9 +82,9 @@ public override void Initialize(AnalysisContext context)
                                                                     SupportedDiagnostics,
                                                                     defaultInterproceduralAnalysisKind: InterproceduralAnalysisKind.ContextSensitive,
                                                                     cancellationToken: cancellationToken);
-                            Lazy<ControlFlowGraph> controlFlowGraphFactory = new Lazy<ControlFlowGraph>(
+                            Lazy<ControlFlowGraph?> controlFlowGraphFactory = new Lazy<ControlFlowGraph?>(
                                 () => operationBlockStartContext.OperationBlocks.GetControlFlowGraph());
-                            Lazy<PointsToAnalysisResult> pointsToFactory = new Lazy<PointsToAnalysisResult>(
+                            Lazy<PointsToAnalysisResult?> pointsToFactory = new Lazy<PointsToAnalysisResult?>(
                                 () =>
                                 {
                                     if (controlFlowGraphFactory.Value == null)
@@ -103,22 +100,22 @@ public override void Initialize(AnalysisContext context)
                                                                 interproceduralAnalysisConfiguration,
                                                                 interproceduralAnalysisPredicateOpt: null);
                                 });
-                            Lazy<(PointsToAnalysisResult, ValueContentAnalysisResult)> valueContentFactory = new Lazy<(PointsToAnalysisResult, ValueContentAnalysisResult)>(
+                            Lazy<(PointsToAnalysisResult?, ValueContentAnalysisResult?)> valueContentFactory = new Lazy<(PointsToAnalysisResult?, ValueContentAnalysisResult?)>(
                                 () =>
                                 {
                                     if (controlFlowGraphFactory.Value == null)
                                     {
                                         return (null, null);
                                     }
 
-                                    ValueContentAnalysisResult valuecontentAnalysisResult = ValueContentAnalysis.TryGetOrComputeResult(
+                                    ValueContentAnalysisResult? valuecontentAnalysisResult = ValueContentAnalysis.TryGetOrComputeResult(
                                                                     controlFlowGraphFactory.Value,
                                                                     owningSymbol,
                                                                     options,
                                                                     wellKnownTypeProvider,
                                                                     interproceduralAnalysisConfiguration,
                                                                     out _,
-                                                                    out PointsToAnalysisResult p);
+                                                                    out PointsToAnalysisResult? p);
 
                                     return (p, valuecontentAnalysisResult);
                                 });
@@ -195,7 +192,7 @@ public override void Initialize(AnalysisContext context)
 
                                             foreach (IOperation rootOperation in rootOperationsNeedingAnalysis)
                                             {
-                                                TaintedDataAnalysisResult taintedDataAnalysisResult = TaintedDataAnalysis.TryGetOrComputeResult(
+                                                TaintedDataAnalysisResult? taintedDataAnalysisResult = TaintedDataAnalysis.TryGetOrComputeResult(
                                                     controlFlowGraphFactory.Value,
                                                     operationBlockAnalysisContext.Compilation,
                                                     operationBlockAnalysisContext.OwningSymbol,

src/Microsoft.NetFramework.Analyzers/Core/DoNotUseInsecureDtdProcessing.cs

@@ -1,6 +1,6 @@
 // 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.
+// TODO(dotpaul): Enable nullable analysis after rewriting these to use DFA.
 #nullable disable
 
 using System;
@@ -781,7 +781,7 @@ private static bool ReferencesAnyTargetType(CompilationSecurityTypes types)
                 || types.XmlSerializer != null;
         }
 
-        private static DiagnosticDescriptor CreateDiagnosticDescriptor(LocalizableResourceString messageFormat, LocalizableResourceString description, string helpLink = null)
+        private static DiagnosticDescriptor CreateDiagnosticDescriptor(LocalizableResourceString messageFormat, LocalizableResourceString description, string helpLink)
         {
             return new DiagnosticDescriptor(RuleId,
                                             SecurityDiagnosticHelpers.GetLocalizableResourceString(nameof(MicrosoftNetFrameworkAnalyzersResources.InsecureXmlDtdProcessing)),

src/Microsoft.NetFramework.Analyzers/Core/DoNotUseInsecureDtdProcessingInApiDesign.cs

@@ -1,6 +1,6 @@
 // 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.
+// TODO(dotpaul): Enable nullable analysis when rewriting this to use DFA.
 #nullable disable
 
 using System;

src/Microsoft.NetFramework.Analyzers/Core/DoNotUseInsecureXSLTScriptExecution.cs

@@ -1,6 +1,6 @@
 // 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.
+// TODO(dotpaul): Enable nullable analysis when rewriting this to use DFA.
 #nullable disable
 
 using System.Collections.Generic;

src/Microsoft.NetFramework.Analyzers/Core/Helpers/SecurityDiagnosticHelpers.cs

@@ -1,8 +1,5 @@
 // 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.
-#nullable disable
-
 using System;
 using System.Threading;
 using Analyzer.Utilities;
@@ -17,13 +14,15 @@ public static class SecurityDiagnosticHelpers
         public static bool IsXslCompiledTransformLoad(IMethodSymbol method, CompilationSecurityTypes xmlTypes)
         {
             return method != null
+                && xmlTypes.XslCompiledTransform != null
                 && method.MatchMethodByName(xmlTypes.XslCompiledTransform, SecurityMemberNames.Load);
         }
 
         public static bool IsXmlDocumentCtorDerived(IMethodSymbol method, CompilationSecurityTypes xmlTypes)
         {
-            return method != null &&
-                   method.MatchMethodDerivedByName(xmlTypes.XmlDocument, WellKnownMemberNames.InstanceConstructorName);
+            return method != null
+                && xmlTypes.XmlDocument != null
+                && method.MatchMethodDerivedByName(xmlTypes.XmlDocument, WellKnownMemberNames.InstanceConstructorName);
         }
 
         public static bool IsXmlDocumentXmlResolverProperty(IPropertySymbol symbol, CompilationSecurityTypes xmlTypes)
@@ -34,6 +33,7 @@ public static bool IsXmlDocumentXmlResolverProperty(IPropertySymbol symbol, Comp
         public static bool IsXmlTextReaderCtorDerived(IMethodSymbol method, CompilationSecurityTypes xmlTypes)
         {
             return method != null
+                && xmlTypes.XmlTextReader != null
                 && method.MatchMethodDerivedByName(xmlTypes.XmlTextReader, WellKnownMemberNames.InstanceConstructorName);
         }
 
@@ -60,6 +60,7 @@ public static bool IsXmlTextReaderDtdProcessingProperty(IPropertySymbol symbol,
         public static bool IsXmlReaderSettingsCtor(IMethodSymbol method, CompilationSecurityTypes xmlTypes)
         {
             return method != null
+                && xmlTypes.XmlReaderSettings != null
                 && method.MatchMethodByName(xmlTypes.XmlReaderSettings, WellKnownMemberNames.InstanceConstructorName);
         }
 
@@ -81,6 +82,7 @@ public static bool IsXmlReaderSettingsMaxCharactersFromEntitiesProperty(IPropert
         public static bool IsXsltSettingsCtor(IMethodSymbol method, CompilationSecurityTypes xmlTypes)
         {
             return method != null
+                && xmlTypes.XsltSettings != null
                 && method.MatchMethodByName(xmlTypes.XsltSettings, WellKnownMemberNames.InstanceConstructorName);
         }
 
@@ -173,9 +175,9 @@ public static bool IsExpressionEqualsIntZero(IOperation operation)
             return literal.HasConstantValue(0);
         }
 
-        private static bool IsSpecifiedProperty(IPropertySymbol symbol, INamedTypeSymbol namedType, string propertyName)
+        private static bool IsSpecifiedProperty(IPropertySymbol symbol, INamedTypeSymbol? namedType, string propertyName)
         {
-            if (symbol != null)
+            if (symbol != null && namedType != null)
             {
                 IPropertySymbol property = (IPropertySymbol)symbol;
                 return property.MatchPropertyByName(namedType, propertyName);
@@ -184,9 +186,9 @@ private static bool IsSpecifiedProperty(IPropertySymbol symbol, INamedTypeSymbol
             return false;
         }
 
-        private static bool IsSpecifiedPropertyDerived(IPropertySymbol symbol, INamedTypeSymbol namedType, string propertyName)
+        private static bool IsSpecifiedPropertyDerived(IPropertySymbol symbol, INamedTypeSymbol? namedType, string propertyName)
         {
-            if (symbol != null)
+            if (symbol != null && namedType != null)
             {
                 IPropertySymbol property = (IPropertySymbol)symbol;
                 return property.MatchPropertyDerivedByName(namedType, propertyName);
@@ -229,7 +231,8 @@ private static int GetSpecifiedParameterIndex(IMethodSymbol method, CompilationS
             {
                 return null;
             }
-            INamedTypeSymbol typeSymbol = compilation.GetOrCreateTypeByMetadataName(typeName);
+
+            INamedTypeSymbol? typeSymbol = compilation.GetOrCreateTypeByMetadataName(typeName);
             return typeSymbol?.ContainingAssembly.Identity.Name.Equals(assemblyName, StringComparison.Ordinal);
         }
 
@@ -274,7 +277,7 @@ public static string GetNonEmptyParentName(SyntaxNode current, SemanticModel mod
         /// For .NET Framework 4.X, this method returns the actual framework version instead of assembly verison of mscorlib,
         /// i.e. for .NET framework 4.5.2, this method return 4.5.2 instead of 4.0.0.0.
         /// </remarks>
-        public static Version GetDotNetFrameworkVersion(Compilation compilation)
+        public static Version? GetDotNetFrameworkVersion(Compilation compilation)
         {
             if (compilation == null || !IsTypeDeclaredInExpectedAssembly(compilation, "System.String", "mscorlib").GetValueOrDefault())
             {

src/TestReferenceAssembly/OtherDllClass.cs

@@ -1,8 +1,5 @@
 // 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.
-#nullable disable
-
 #pragma warning disable CA1801 // Remove unused parameter
 #pragma warning disable IDE0060 // Remove unused parameter
 
@@ -22,15 +19,16 @@ namespace OtherDll
     /// analysis implementations.
     /// </remarks>
     public class OtherDllClass<T>
+        where T : class
     {
-        public OtherDllClass(T constructedInput)
+        public OtherDllClass(T? constructedInput)
         {
             this.ConstructedInput = constructedInput;
         }
 
-        public T ConstructedInput { get; set; }
+        public T? ConstructedInput { get; set; }
 
-        public T Default
+        public T? Default
         {
             get { return default; }
             set { }
@@ -50,22 +48,22 @@ public string RandomString
             set { }
         }
 
-        public T ReturnsConstructedInput()
+        public T? ReturnsConstructedInput()
         {
             return this.ConstructedInput;
         }
 
-        public T ReturnsDefault()
+        public T? ReturnsDefault()
         {
             return default;
         }
 
-        public T ReturnsInput(T input)
+        public T? ReturnsInput(T? input)
         {
             return input;
         }
 
-        public T ReturnsDefault(T input)
+        public T? ReturnsDefault(T? input)
         {
             return default;
         }
@@ -79,22 +77,22 @@ public string ReturnsRandom(string input)
             return Encoding.ASCII.GetString(bytes);
         }
 
-        public void SetsOutputToConstructedInput(out T output)
+        public void SetsOutputToConstructedInput(out T? output)
         {
             output = this.ConstructedInput;
         }
 
-        public void SetsOutputToDefault(out T output)
+        public void SetsOutputToDefault(out T? output)
         {
             output = default;
         }
 
-        public void SetsOutputToInput(T input, out T output)
+        public void SetsOutputToInput(T? input, out T? output)
         {
             output = input;
         }
 
-        public void SetsOutputToDefault(T input, out T output)
+        public void SetsOutputToDefault(T? input, out T? output)
         {
             output = default;
         }
@@ -108,22 +106,22 @@ public void SetsOutputToRandom(string input, out string output)
             output = Encoding.ASCII.GetString(bytes);
         }
 
-        public void SetsReferenceToConstructedInput(ref T output)
+        public void SetsReferenceToConstructedInput(ref T? output)
         {
             output = this.ConstructedInput;
         }
 
-        public void SetsReferenceToDefault(ref T output)
+        public void SetsReferenceToDefault(ref T? output)
         {
             output = default;
         }
 
-        public void SetsReferenceToInput(T input, ref T output)
+        public void SetsReferenceToInput(T? input, ref T? output)
         {
             output = input;
         }
 
-        public void SetsReferenceToDefault(T input, ref T output)
+        public void SetsReferenceToDefault(T? input, ref T? output)
         {
             output = default;
         }