rebase-squash

SonarSource · Jul 14, 2023 · 40d8c76 · 40d8c76
1 parent 17192a3
commit 40d8c76
Show file tree

Hide file tree

Showing 9 changed files with 140 additions and 44 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SymbolicExecution/SymbolicExecutionRunner.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SymbolicExecution/SymbolicExecutionRunner.cs
@@ -38,7 +38,6 @@ public class SymbolicExecutionRunner : SymbolicExecutionRunnerBase
     private static readonly ImmutableArray<ISymbolicExecutionAnalyzer> SonarRules = ImmutableArray.Create<ISymbolicExecutionAnalyzer>(
         new SonarRules.ConditionEvaluatesToConstant(),
         new SonarRules.RestrictDeserializedTypes(),
-        new SonarRules.InitializationVectorShouldBeRandom(),
         new SonarRules.HashesShouldHaveUnpredictableSalt());
 
     public SymbolicExecutionRunner() : this(AnalyzerConfiguration.AlwaysEnabled) { }
@@ -49,6 +48,7 @@ public SymbolicExecutionRunner() : this(AnalyzerConfiguration.AlwaysEnabled) { }
         .Add(InvalidCastToInterface.S1944, CreateFactory<EmptyRuleCheck, SonarRules.InvalidCastToInterfaceSymbolicExecution>()) // This old SE rule is part of S3655.
         .Add(LocksReleasedAllPaths.S2222, CreateFactory<LocksReleasedAllPaths>())
         .Add(NullPointerDereference.S2259, CreateFactory<NullPointerDereference, SonarRules.NullPointerDereference>())
+        .Add(InitializationVectorShouldBeRandom.S3329, CreateFactory<InitializationVectorShouldBeRandom, SonarRules.InitializationVectorShouldBeRandom>())
         .Add(EmptyNullableValueAccess.S3655, CreateFactory<EmptyNullableValueAccess, SonarRules.EmptyNullableValueAccess>())
         .Add(PublicMethodArgumentsShouldBeCheckedForNull.S3900, CreateFactory<PublicMethodArgumentsShouldBeCheckedForNull, SonarRules.PublicMethodArgumentsShouldBeCheckedForNull>())
         .Add(CalculationsShouldNotOverflow.S3949, CreateFactory<CalculationsShouldNotOverflow>())

diff --git a/...s/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/InitializationVectorShouldBeRandom.cs b/...s/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/InitializationVectorShouldBeRandom.cs
@@ -25,5 +25,5 @@ public sealed class InitializationVectorShouldBeRandom : InitializationVectorSho
     public static readonly DiagnosticDescriptor S3329 = DescriptorFactory.Create(DiagnosticId, MessageFormat);
     protected override DiagnosticDescriptor Rule => S3329;
 
-    public override bool ShouldExecute() => false;
+    public override bool ShouldExecute() => true;
 }
diff --git a/...yzer.Common/SymbolicExecution/Roslyn/RuleChecks/InitializationVectorShouldBeRandomBase.cs b/...yzer.Common/SymbolicExecution/Roslyn/RuleChecks/InitializationVectorShouldBeRandomBase.cs
@@ -18,10 +18,112 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
 
+using System.Security.Cryptography;
+using SonarAnalyzer.SymbolicExecution.Constraints;
+
 namespace SonarAnalyzer.SymbolicExecution.Roslyn.RuleChecks;
 
 public abstract class InitializationVectorShouldBeRandomBase : SymbolicRuleCheck
 {
     protected const string DiagnosticId = "S3329";
     protected const string MessageFormat = "Use a dynamically-generated, random IV.";
+
+    private static readonly ImmutableArray<MemberDescriptor> CryptographicallyStrongRandomNumberGenerators =
+        ImmutableArray.Create(
+            new MemberDescriptor(KnownType.System_Security_Cryptography_RandomNumberGenerator, nameof(RandomNumberGenerator.GetBytes)),
+            new MemberDescriptor(KnownType.System_Security_Cryptography_RandomNumberGenerator, nameof(RandomNumberGenerator.GetNonZeroBytes)));
+
+    protected override ProgramState PreProcessSimple(SymbolicContext context)
+    {
+        var state = context.State;
+        if (context.Operation.Instance.AsAssignment() is { } assignment)
+        {
+            return ProcessSimpleAssignment(assignment, state)
+                    ?? ProcessAssignmentToIVProperty(assignment, state)
+                    ?? state;
+        }
+        else if (context.Operation.Instance.AsInvocation() is { } invocation)
+        {
+            state = ProcessStrongRandomGeneratorMethodInvocation(invocation, state)
+                    ?? ProcessCreateEncryptorMethodInvocation(invocation, state)
+                    ?? state;
+
+            if (IsCreateEncryptorMethod(invocation)
+                && invocation.Instance.TrackedSymbol() is { } instanceSymbol
+                && state[instanceSymbol]?.HasConstraint(ByteCollectionConstraint.CryptographicallyWeak) is true)
+            {
+                ReportIssue(context.Operation.Instance, invocation.Instance.Syntax.ToString());
+            }
+            return state;
+        }
+        else
+        {
+            return state;
+        }
+    }
+
+    private static ProgramState ProcessSimpleAssignment(IAssignmentOperationWrapper assignment, ProgramState state) =>
+        assignment.Value.AsArrayCreation() is { } arrayCreation
+            ? state.SetOperationConstraint(arrayCreation, ByteCollectionConstraint.CryptographicallyWeak)
+            : null;
+
+    private static ProgramState ProcessAssignmentToIVProperty(IAssignmentOperationWrapper assignment, ProgramState state)
+    {
+        return GetIVPropertyFromAssignment(assignment) is { } ivPropertySymbol
+                && (assignment.Value.AsArrayCreation() is { }
+                    || state[assignment.Value.TrackedSymbol()].HasConstraint(ByteCollectionConstraint.CryptographicallyWeak))
+                ? state.SetSymbolConstraint(ivPropertySymbol, ByteCollectionConstraint.CryptographicallyWeak)
+                : null;
+
+        static ISymbol GetIVPropertyFromAssignment(IAssignmentOperationWrapper assignment) =>
+            assignment.Target?.AsPropertyReference() is { } property
+            && property.Property.Name.Equals("IV")
+            && property.Instance.TrackedSymbol() is { } propertySymbol
+               ? propertySymbol
+               : null;
+    }
+
+    private static ProgramState ProcessStrongRandomGeneratorMethodInvocation(IInvocationOperationWrapper invocation, ProgramState state)
+    {
+        if (CryptographicallyStrongRandomNumberGenerators.Any(x => IsStrongRandomGeneratorInvocation(x, invocation))
+            && invocation.ArgumentValue("data") is { } byteArray
+            && byteArray.TrackedSymbol() is { } byteArraySymbol)
+        {
+            return state.SetSymbolConstraint(byteArraySymbol, ByteCollectionConstraint.CryptographicallyStrong);
+        }
+        else if (IsGenerateIVMethod(invocation))
+        {
+            return state.SetSymbolConstraint(invocation.Instance.TrackedSymbol(), ByteCollectionConstraint.CryptographicallyStrong);
+        }
+        else
+        {
+            return null;
+        }
+
+        static bool IsStrongRandomGeneratorInvocation(MemberDescriptor method, IInvocationOperationWrapper invocation) =>
+            invocation.TargetMethod.Name == method.Name
+            && invocation.TargetMethod.ContainingType.DerivesFrom(method.ContainingType);
+    }
+
+    private static ProgramState ProcessCreateEncryptorMethodInvocation(IInvocationOperationWrapper invocation, ProgramState state)
+    {
+        return IsCreateEncryptorMethod(invocation)
+        && invocation.Arguments.Length == 2
+        && IvArgument(invocation) is { } ivArgument
+        && (ivArgument.AsArrayCreation() is { } || (ivArgument.TrackedSymbol() is { } symbol && state[symbol]?.HasConstraint(ByteCollectionConstraint.CryptographicallyWeak) is true))
+            ? state.SetSymbolConstraint(invocation.Instance.TrackedSymbol(), ByteCollectionConstraint.CryptographicallyWeak)
+            : null;
+
+        static IOperation IvArgument(IInvocationOperationWrapper invocation) =>
+            invocation.ArgumentValue("rgbIV")
+            ?? invocation.ArgumentValue("iv");
+    }
+
+    private static bool IsCreateEncryptorMethod(IInvocationOperationWrapper invocation) =>
+        invocation.TargetMethod.Name.Equals(nameof(SymmetricAlgorithm.CreateEncryptor))
+        && invocation.TargetMethod.ContainingType.DerivesFrom(KnownType.System_Security_Cryptography_SymmetricAlgorithm);
+
+    private static bool IsGenerateIVMethod(IInvocationOperationWrapper invocation) =>
+        invocation.TargetMethod.Name.Equals(nameof(SymmetricAlgorithm.GenerateIV))
+        && invocation.TargetMethod.ContainingType.DerivesFrom(KnownType.System_Security_Cryptography_SymmetricAlgorithm);
 }
diff --git a/.../SonarAnalyzer.UnitTest/Rules/SymbolicExecution/InitializationVectorShouldBeRandomTest.cs b/.../SonarAnalyzer.UnitTest/Rules/SymbolicExecution/InitializationVectorShouldBeRandomTest.cs
@@ -46,27 +46,18 @@ public void InitializationVectorShouldBeRandom_Sonar_CS() =>
                 .WithOptions(ParseOptionsHelper.FromCSharp8)
                 .Verify();
 
-        [Ignore] // ToDo: Remove after S3329 implementation
         [TestMethod]
         public void InitializationVectorShouldBeRandom_Roslyn_CS() =>
             roslynCS.AddPaths("InitializationVectorShouldBeRandom.cs")
             .Verify();
 
-        [Ignore] // ToDo: Remove after S3329 implementation
-        [TestMethod]
-        public void InitializationVectorShouldBeRandom_Roslyn_CSharp8() =>
-            roslynCS.AddPaths("InitializationVectorShouldBeRandom.CSharp8.cs")
-                .WithOptions(ParseOptionsHelper.FromCSharp8)
-                .Verify();
-
         [TestMethod]
         public void InitializationVectorShouldBeRandom_DoesNotRaiseIssuesForTestProject_Sonar() =>
             sonar.AddPaths("InitializationVectorShouldBeRandom.cs")
                 .WithOptions(ParseOptionsHelper.FromCSharp8)
                 .AddTestReference()
                 .VerifyNoIssueReported();
 
-        [Ignore] // ToDo: Remove after S3329 implementation
         [TestMethod]
         public void InitializationVectorShouldBeRandom_DoesNotRaiseIssuesForTestProject_Roslyn_CS() =>
             roslynCS.AddPaths("InitializationVectorShouldBeRandom.cs")
@@ -75,13 +66,18 @@ public void InitializationVectorShouldBeRandom_DoesNotRaiseIssuesForTestProject_
 
 #if NET
 
+        [TestMethod]
+        public void InitializationVectorShouldBeRandom_Roslyn_CSharp8() =>
+            roslynCS.AddPaths("InitializationVectorShouldBeRandom.CSharp8.cs")
+            .WithOptions(ParseOptionsHelper.FromCSharp8)
+            .Verify();
+
         [TestMethod]
         public void InitializationVectorShouldBeRandom_Sonar_CSharp9() =>
             sonar.AddPaths("InitializationVectorShouldBeRandom.CSharp9.cs")
                 .WithTopLevelStatements()
                 .Verify();
 
-        [Ignore] // ToDo: Remove after S3329 implementation
         [TestMethod]
         public void InitializationVectorShouldBeRandom_Roslyn_CSharp9() =>
             roslynCS.AddPaths("InitializationVectorShouldBeRandom.CSharp9.cs")
@@ -94,7 +90,6 @@ public void InitializationVectorShouldBeRandom_Sonar_CSharp10() =>
                 .WithOptions(ParseOptionsHelper.FromCSharp10)
                 .Verify();
 
-        [Ignore] // ToDo: Remove after S3329 implementation
         [TestMethod]
         public void InitializationVectorShouldBeRandom_Roslyn_CSharp10() =>
             roslynCS.AddPaths("InitializationVectorShouldBeRandom.CSharp10.cs")
@@ -107,7 +102,6 @@ public void InitializationVectorShouldBeRandom_Sonar_CSharp11() =>
                 .WithOptions(ParseOptionsHelper.FromCSharp11)
                 .Verify();
 
-        [Ignore] // ToDo: Remove after S3329 implementation
         [TestMethod]
         public void InitializationVectorShouldBeRandom_Roslyn_CSharp11() =>
             roslynCS.AddPaths("InitializationVectorShouldBeRandom.CSharp11.cs")

diff --git a/...nitTest/TestCases/SymbolicExecution/Roslyn/InitializationVectorShouldBeRandom.CSharp10.cs b/...nitTest/TestCases/SymbolicExecution/Roslyn/InitializationVectorShouldBeRandom.CSharp10.cs
@@ -9,6 +9,6 @@ public void Examples()
         AesCng aes = new AesCng();
         aes.CreateEncryptor();
         (var rgb, int a) = (new byte[16], 42);
-        aes.CreateEncryptor(aes.Key, rgb); // FIXME Non-compliant
+        aes.CreateEncryptor(aes.Key, rgb); // FN
     }
 }
diff --git a/...nitTest/TestCases/SymbolicExecution/Roslyn/InitializationVectorShouldBeRandom.CSharp11.cs b/...nitTest/TestCases/SymbolicExecution/Roslyn/InitializationVectorShouldBeRandom.CSharp11.cs
@@ -12,7 +12,7 @@ public void SymetricAlgorithmCreateEncryptor()
         {
             sa.GenerateKey();
             var generateIVNotCalled = sa.CreateEncryptor(sa.Key, sa.IV);
-            var constantVector2 = sa.CreateEncryptor(sa.Key, "1234567890123456"u8.ToArray()); // FIXME Non-compliant
+            var constantVector2 = sa.CreateEncryptor(sa.Key, "1234567890123456"u8.ToArray()); // FN
         }
     }
 }
diff --git a/...UnitTest/TestCases/SymbolicExecution/Roslyn/InitializationVectorShouldBeRandom.CSharp8.cs b/...UnitTest/TestCases/SymbolicExecution/Roslyn/InitializationVectorShouldBeRandom.CSharp8.cs
@@ -34,7 +34,7 @@ public void AesCryptoServiceProviderCreateEncryptor()
         var noParams = aes.CreateEncryptor(); // Compliant
 
         var constantIV = new byte[16];
-        var withConstant = aes.CreateEncryptor(aes.Key, constantIV); // FIXME Non-compliant
+        var withConstant = aes.CreateEncryptor(aes.Key, constantIV); // Noncompliant
 
         aes.GenerateKey();
         aes.GenerateIV();
@@ -58,7 +58,7 @@ public void AesCreateEncryptor()
         var reGeneratedKey = aes.CreateEncryptor(); // Compliant
 
         var constantIV = new byte[16];
-        var withConstant = aes.CreateEncryptor(aes.Key, constantIV); // FIXME Non-compliant
+        var withConstant = aes.CreateEncryptor(aes.Key, constantIV); // Noncompliant
 
         aes.GenerateIV();
         aes.CreateEncryptor();
@@ -81,7 +81,7 @@ public void AesCngCreateEncryptor()
         var withGeneratedKey = aes.CreateEncryptor(); // Compliant
 
         var constantIV = new byte[16];
-        var withConstant = aes.CreateEncryptor(aes.Key, constantIV); // FIXME Non-compliant
+        var withConstant = aes.CreateEncryptor(aes.Key, constantIV); // Noncompliant
 
         aes.GenerateIV();
         aes.CreateEncryptor();
@@ -104,7 +104,7 @@ public void CustomImplementationOfAes()
         var withGeneratedKey = aes.CreateEncryptor(); // Compliant
 
         var constantIV = new byte[16];
-        var withConstant = aes.CreateEncryptor(aes.Key, constantIV); // FIXME Non-compliant
+        var withConstant = aes.CreateEncryptor(aes.Key, constantIV); // Noncompliant
 
         aes.GenerateIV();
         aes.CreateEncryptor();
@@ -126,7 +126,7 @@ public void InConditionals(int a)
         var e = a switch
         {
             1 => aes.CreateEncryptor(), // Compliant
-            2 => aes.CreateEncryptor(aes.Key, constantIV), // FIXME Non-compliant
+            2 => aes.CreateEncryptor(aes.Key, constantIV), // Noncompliant
             _ => null
         };
 
@@ -137,10 +137,10 @@ public void InConditionals(int a)
         {
             aes2.IV = iv; // Set IV to constant
         }
-        aes2.CreateEncryptor(); // FIXME Non-compliant
+        aes2.CreateEncryptor(); // Noncompliant
 
         var aes3 = a == 2 ? aes2 : aes;
-        aes3.CreateEncryptor(); // FIXME Non-compliant
+        aes3.CreateEncryptor(); // Noncompliant
     }
 }
 

diff --git a/...UnitTest/TestCases/SymbolicExecution/Roslyn/InitializationVectorShouldBeRandom.CSharp9.cs b/...UnitTest/TestCases/SymbolicExecution/Roslyn/InitializationVectorShouldBeRandom.CSharp9.cs
@@ -8,13 +8,13 @@
 using var rng = new RNGCryptoServiceProvider();
 
 aes.CreateEncryptor();
-aes.CreateEncryptor(aes.Key, new byte[16]); // FIXME Non-compliant
+aes.CreateEncryptor(aes.Key, new byte[16]); // Noncompliant
 
 void TopLevelLocalFunction()
 {
     using var aes = new AesCng();
     aes.CreateEncryptor();
-    aes.CreateEncryptor(aes.Key, new byte[16]); // FIXME Non-compliant
+    aes.CreateEncryptor(aes.Key, new byte[16]); // Noncompliant
 }
 
 public class Sample
@@ -23,7 +23,7 @@ public void TargetTypedNew()
     {
         AesCng aes = new();
         aes.CreateEncryptor();
-        aes.CreateEncryptor(aes.Key, new byte[16]); // FIXME Non-compliant
+        aes.CreateEncryptor(aes.Key, new byte[16]); // Noncompliant
     }
 
     public void StaticLambda()
@@ -32,7 +32,7 @@ public void StaticLambda()
         {
             AesCng aes = new AesCng();
             aes.CreateEncryptor();
-            aes.CreateEncryptor(aes.Key, new byte[16]); // FIXME Non-compliant
+            aes.CreateEncryptor(aes.Key, new byte[16]); // Noncompliant
         };
         a();
     }
@@ -43,7 +43,7 @@ public int Property
         init
         {
             AesCng aes = new AesCng();
-            aes.CreateEncryptor(); // FIXME Non-compliant
+            aes.CreateEncryptor(); // FN
             aes.GenerateIV();
             aes.CreateEncryptor();
         }
@@ -65,7 +65,7 @@ public void Method()
     {
         AesCng aes = new AesCng();
         aes.CreateEncryptor();
-        aes.CreateEncryptor(aes.Key, new byte[16]); // FIXME Non-compliant
+        aes.CreateEncryptor(aes.Key, new byte[16]); // Noncompliant
     }
 }
 
@@ -80,7 +80,7 @@ public partial void Method()
     {
         AesCng aes = new AesCng();
         aes.CreateEncryptor();
-        aes.CreateEncryptor(aes.Key, new byte[16]); // FIXME Non-compliant
+        aes.CreateEncryptor(aes.Key, new byte[16]); // Noncompliant
     }
 }
 
@@ -92,7 +92,7 @@ public void Go(bool condition)
         {
             SymmetricAlgorithm aes = condition ? new AesCng() : new AesCryptoServiceProvider();
             aes.CreateEncryptor();
-            aes.CreateEncryptor(aes.Key, new byte[16]); // FIXME Non-compliant
+            aes.CreateEncryptor(aes.Key, new byte[16]); // Noncompliant
         }
     }
 }