Merge pull request #2741 from dotpaul/moreTls

CA5364 and CA5386 not diagnosing on &= assignments
dotnet · Aug 5, 2019 · 7b1eec2 · 7b1eec2
2 parents 002ddc1 + 7a5f5bc
commit 7b1eec2
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 38 deletions.
diff --git a/src/Microsoft.NetCore.Analyzers/Core/Security/DoNotUseDeprecatedSecurityProtocols.cs b/src/Microsoft.NetCore.Analyzers/Core/Security/DoNotUseDeprecatedSecurityProtocols.cs
@@ -6,6 +6,7 @@
 using Analyzer.Utilities.Extensions;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.FlowAnalysis.DataFlow;
 using Microsoft.CodeAnalysis.Operations;
 using Microsoft.NetCore.Analyzers.Security.Helpers;
 
@@ -54,54 +55,32 @@ public override void Initialize(AnalysisContext context)
             context.RegisterCompilationStartAction(
                 (CompilationStartAnalysisContext compilationStartAnalysisContext) =>
                 {
-                    var securityProtocolTypeTypeSymbol = compilationStartAnalysisContext.Compilation.GetTypeByMetadataName(WellKnownTypeNames.SystemNetSecurityProtocolType);
-
-                    if (securityProtocolTypeTypeSymbol == null)
+                    var wellKnownTypeProvider = WellKnownTypeProvider.GetOrCreate(compilationStartAnalysisContext.Compilation);
+                    if (!wellKnownTypeProvider.TryGetTypeByMetadataName(
+                            WellKnownTypeNames.SystemNetSecurityProtocolType,
+                            out var securityProtocolTypeTypeSymbol)
+                        || !wellKnownTypeProvider.TryGetTypeByMetadataName(
+                                WellKnownTypeNames.SystemNetServicePointManager,
+                                out var servicePointManagerTypeSymbol))
                     {
                         return;
                     }
 
-                    bool IsReferencingSecurityProtocolType(
-                        IFieldReferenceOperation fieldReferenceOperation,
-                        out bool isDeprecatedProtocol,
-                        out bool isHardCodedOkayProtocol)
-                    {
-                        if (securityProtocolTypeTypeSymbol.Equals(fieldReferenceOperation.Field.ContainingType))
-                        {
-                            if (HardCodedSafeProtocolMetadataNames.Contains(fieldReferenceOperation.Field.Name))
-                            {
-                                isHardCodedOkayProtocol = true;
-                                isDeprecatedProtocol = false;
-                            }
-                            else if (fieldReferenceOperation.Field.Name == SystemDefaultName)
-                            {
-                                isHardCodedOkayProtocol = false;
-                                isDeprecatedProtocol = false;
-                            }
-                            else
-                            {
-                                isDeprecatedProtocol = true;
-                                isHardCodedOkayProtocol = false;
-                            }
-
-                            return true;
-                        }
-                        else
-                        {
-                            isHardCodedOkayProtocol = false;
-                            isDeprecatedProtocol = false;
-                            return false;
-                        }
-                    }
-
                     compilationStartAnalysisContext.RegisterOperationAction(
                         (OperationAnalysisContext operationAnalysisContext) =>
                         {
                             var fieldReferenceOperation = (IFieldReferenceOperation)operationAnalysisContext.Operation;
+
+                            // Make sure we're not inside an &= assignment like:
+                            //   ServicePointManager.SecurityProtocol &= ~(SecurityProtocolType.Ssl3 | SecurityProtocolType.Tls | SecurityProtocolType.Tls11)
+                            // cuz &= is at worst, disabling protocol versions.
                             if (IsReferencingSecurityProtocolType(
                                     fieldReferenceOperation,
                                     out var isDeprecatedProtocol,
-                                    out var isHardCodedOkayProtocol))
+                                    out var isHardCodedOkayProtocol)
+                                && null == fieldReferenceOperation.GetAncestor<ICompoundAssignmentOperation>(
+                                      OperationKind.CompoundAssignment,
+                                      IsAndEqualsServicePointManagerAssignment))
                             {
                                 if (isDeprecatedProtocol)
                                 {
@@ -124,7 +103,14 @@ bool IsReferencingSecurityProtocolType(
                         (OperationAnalysisContext operationAnalysisContext) =>
                         {
                             var assignmentOperation = (IAssignmentOperation)operationAnalysisContext.Operation;
-                            if (!securityProtocolTypeTypeSymbol.Equals(assignmentOperation.Target.Type))
+
+                            // Make sure this is an assignment operation for a SecurityProtocolType, and not
+                            // an assignment like:
+                            //   ServicePointManager.SecurityProtocol &= ~(SecurityProtocolType.Ssl3 | SecurityProtocolType.Tls | SecurityProtocolType.Tls11)
+                            // cuz &= is at worst, disabling protocol versions.
+                            if (!securityProtocolTypeTypeSymbol.Equals(assignmentOperation.Target.Type)
+                                || (assignmentOperation is ICompoundAssignmentOperation compoundAssignmentOperation
+                                    && IsAndEqualsServicePointManagerAssignment(compoundAssignmentOperation)))
                             {
                                 return;
                             }
@@ -193,6 +179,51 @@ bool IsReferencingSecurityProtocolType(
                         },
                         OperationKind.SimpleAssignment,
                         OperationKind.CompoundAssignment);
+
+                    return;
+
+                    // Local function(s).
+                    bool IsReferencingSecurityProtocolType(
+                        IFieldReferenceOperation fieldReferenceOperation,
+                        out bool isDeprecatedProtocol,
+                        out bool isHardCodedOkayProtocol)
+                    {
+                        if (securityProtocolTypeTypeSymbol.Equals(fieldReferenceOperation.Field.ContainingType))
+                        {
+                            if (HardCodedSafeProtocolMetadataNames.Contains(fieldReferenceOperation.Field.Name))
+                            {
+                                isHardCodedOkayProtocol = true;
+                                isDeprecatedProtocol = false;
+                            }
+                            else if (fieldReferenceOperation.Field.Name == SystemDefaultName)
+                            {
+                                isHardCodedOkayProtocol = false;
+                                isDeprecatedProtocol = false;
+                            }
+                            else
+                            {
+                                isDeprecatedProtocol = true;
+                                isHardCodedOkayProtocol = false;
+                            }
+
+                            return true;
+                        }
+                        else
+                        {
+                            isHardCodedOkayProtocol = false;
+                            isDeprecatedProtocol = false;
+                            return false;
+                        }
+                    }
+
+                    bool IsAndEqualsServicePointManagerAssignment(ICompoundAssignmentOperation compoundAssignmentOperation)
+                    {
+                        return compoundAssignmentOperation.OperatorKind == BinaryOperatorKind.And
+                            && compoundAssignmentOperation.Target is IPropertyReferenceOperation targetPropertyReference
+                            && targetPropertyReference.Instance == null
+                            && servicePointManagerTypeSymbol.Equals(targetPropertyReference.Property.ContainingType)
+                            && targetPropertyReference.Property.MetadataName == "SecurityProtocol";
+                    }
                 });
         }
     }

diff --git a/...icrosoft.NetCore.Analyzers/UnitTests/Security/DoNotUseDeprecatedSecurityProtocolsTests.cs b/...icrosoft.NetCore.Analyzers/UnitTests/Security/DoNotUseDeprecatedSecurityProtocolsTests.cs
@@ -490,6 +490,43 @@ public void TestMethod()
 }");
         }
 
+        [Fact]
+        public void TestMaskOutUnsafeOnServicePointManagerNoDiagnostic()
+        {
+            VerifyCSharp(@"
+using System;
+using System.Net;
+
+class TestClass
+{
+    public void TestMethod()
+    {
+        ServicePointManager.SecurityProtocol &= ~(SecurityProtocolType.Ssl3 | SecurityProtocolType.Tls | SecurityProtocolType.Tls11);
+    }
+}");
+        }
+
+        [Fact]
+        public void TestMaskOutUnsafeOnVariableDiagnostic()
+        {
+            VerifyCSharp(@"
+using System;
+using System.Net;
+
+class TestClass
+{
+    public void TestMethod()
+    {
+        SecurityProtocolType t = default(SecurityProtocolType);
+        t &= ~(SecurityProtocolType.Ssl3 | SecurityProtocolType.Tls | SecurityProtocolType.Tls11);
+    }
+}",
+                GetCSharpResultAt(10, 14, DoNotUseDeprecatedSecurityProtocols.HardCodedRule, "-1009"),
+                GetCSharpResultAt(10, 16, DoNotUseDeprecatedSecurityProtocols.DeprecatedRule, "Ssl3"),
+                GetCSharpResultAt(10, 44, DoNotUseDeprecatedSecurityProtocols.DeprecatedRule, "Tls"),
+                GetCSharpResultAt(10, 71, DoNotUseDeprecatedSecurityProtocols.DeprecatedRule, "Tls11"));
+        }
+
         protected override DiagnosticAnalyzer GetBasicDiagnosticAnalyzer()
         {
             return new DoNotUseDeprecatedSecurityProtocols();

diff --git a/src/Utilities/Compiler/WellKnownTypeNames.cs b/src/Utilities/Compiler/WellKnownTypeNames.cs
@@ -325,5 +325,6 @@ internal static class WellKnownTypeNames
         public const string SystemConvert = "System.Convert";
         public const string SystemSecurityCryptographySymmetricAlgorithm = "System.Security.Cryptography.SymmetricAlgorithm";
         public const string NewtonsoftJsonTypeNameHandling = "Newtonsoft.Json.TypeNameHandling";
+        public const string SystemNetServicePointManager = "System.Net.ServicePointManager";
     }
 }