Improved NewtonsoftDeserializationAnalyzer

Analyzers/Analyzers.Test/SecureSerialization/Json/NewtonsoftDeserializationAnalyzerUnitTests.cs

@@ -8,6 +8,8 @@
 using System.Collections.Generic;
 using Microsoft.CodeAnalysis;
 using Skyline.DataMiner.Utils.SecureCoding.SecureSerialization.Json.Newtonsoft;
+using Skyline.DataMiner.Utils.SecureCoding.Analyzers.SecureIO;
+using Skyline.DataMiner.Utils.SecureCoding.SecureIO;
 
 namespace Skyline.DataMiner.Utils.SecureCoding.Analyzers.Tests.SecureSerialization.Json
 {
@@ -81,5 +83,109 @@ public async Task VerifyNewtonsoftDeserializationUsageDiagnosticAndKnownTypesCod
 
             await analyzerVerifier.RunAsync();
         }
+
+        [TestMethod]
+        public async Task VerifyNewtonsoftDeserializationInsecureUsageDefaultSettings()
+        {
+            var packages = new PackageIdentity[]
+            {
+                new PackageIdentity("Newtonsoft.json","13.0.3"),
+            }
+            .ToImmutableArray();
+
+            var testCode = @"using Newtonsoft.Json;
+                using Newtonsoft.Json.Serialization;
+                using System;
+                using System.Collections.Generic;
+
+                namespace Skyline.DataMiner.Utils.SecureCoding.Analyzers.Tests.SecureSerialization.TestScenarios
+                {
+                    internal class Insecure_JsonDefaultSettings
+                    {
+                        public void Insecure_TypeNameHandling_All()
+                        {
+                            JsonConvert.DefaultSettings = () => new JsonSerializerSettings
+                            {
+                                TypeNameHandling = TypeNameHandling.All
+                            };
+
+                            JsonConvert.DefaultSettings = () =>
+                            {
+                                return new JsonSerializerSettings
+                                {
+                                    TypeNameHandling = TypeNameHandling.All
+                                };
+                            };
+                        }
+
+                        public void Insecure_TypeNameHandling_Auto()
+                        {
+                            JsonConvert.DefaultSettings = () => new JsonSerializerSettings
+                            {
+                                TypeNameHandling = TypeNameHandling.Auto
+                            };
+
+                            JsonConvert.DefaultSettings = () =>
+                            {
+                                return new JsonSerializerSettings
+                                {
+                                    TypeNameHandling = TypeNameHandling.Auto
+                                };
+                            };
+                        }
+
+
+                        public void Insecure_TypeNameHandling_Objects()
+                        {
+                            JsonConvert.DefaultSettings = () => new JsonSerializerSettings
+                            {
+                                TypeNameHandling = TypeNameHandling.Objects
+                            };
+
+                            JsonConvert.DefaultSettings = () =>
+                            {
+                                return new JsonSerializerSettings
+                                {
+                                    TypeNameHandling = TypeNameHandling.Objects
+                                };
+                            };
+                        }
+
+
+                        public void Insecure_TypeNameHandling_Arrays()
+                        {
+                            JsonConvert.DefaultSettings = () => new JsonSerializerSettings
+                            {
+                                TypeNameHandling = TypeNameHandling.Arrays
+                            };
+
+                            JsonConvert.DefaultSettings = () =>
+                            {
+                                return new JsonSerializerSettings
+                                {
+                                    TypeNameHandling = TypeNameHandling.Arrays
+                                };
+                            };
+                        }
+                    }
+                }";
+
+            var expectedDiagnostics = new List<DiagnosticResult>()
+            {
+                AnalyzerVerifierHelper.BuildDiagnosticResult(NewtonsoftDeserializationAnalyzer.DiagnosticId, DiagnosticSeverity.Warning, 14, 33),
+                AnalyzerVerifierHelper.BuildDiagnosticResult(NewtonsoftDeserializationAnalyzer.DiagnosticId, DiagnosticSeverity.Warning, 21, 37),
+                AnalyzerVerifierHelper.BuildDiagnosticResult(NewtonsoftDeserializationAnalyzer.DiagnosticId, DiagnosticSeverity.Warning, 30, 33),
+                AnalyzerVerifierHelper.BuildDiagnosticResult(NewtonsoftDeserializationAnalyzer.DiagnosticId, DiagnosticSeverity.Warning, 37, 37),
+                AnalyzerVerifierHelper.BuildDiagnosticResult(NewtonsoftDeserializationAnalyzer.DiagnosticId, DiagnosticSeverity.Warning, 47, 33),
+                AnalyzerVerifierHelper.BuildDiagnosticResult(NewtonsoftDeserializationAnalyzer.DiagnosticId, DiagnosticSeverity.Warning, 54, 37),
+                AnalyzerVerifierHelper.BuildDiagnosticResult(NewtonsoftDeserializationAnalyzer.DiagnosticId, DiagnosticSeverity.Warning, 64, 33),
+                AnalyzerVerifierHelper.BuildDiagnosticResult(NewtonsoftDeserializationAnalyzer.DiagnosticId, DiagnosticSeverity.Warning, 71, 37),
+            };
+
+            var analyzerVerifier = AnalyzerVerifierHelper.BuildAnalyzerVerifier<NewtonsoftDeserializationAnalyzer>(testCode, packages);
+            analyzerVerifier.TestState.ExpectedDiagnostics.AddRange(expectedDiagnostics);
+
+            await analyzerVerifier.RunAsync();
+        }
     }
 }

Analyzers/Analyzers/SecureSerialization/Json/NewtonsoftDeserializationAnalyzer.cs

@@ -2,16 +2,23 @@
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
+using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Linq;
 
 namespace Skyline.DataMiner.Utils.SecureCoding.Analyzers.SecureSerialization.Json
 {
     [DiagnosticAnalyzer(LanguageNames.CSharp)]
     public class NewtonsoftDeserializationAnalyzer : DiagnosticAnalyzer
     {
+        private static readonly HashSet<string> insecureTypeNames = new HashSet<string>()
+        {
+            "All", "Auto", "Objects", "Arrays"
+        };
+
         public const string DiagnosticId = "SLC_SC0004";
 
-        public static DiagnosticDescriptor Rule => new DiagnosticDescriptor(
+        public static DiagnosticDescriptor DeserializationRule => new DiagnosticDescriptor(
             DiagnosticId,
             "Avoid deserializing json strings by using Newtonsoft directly.",
             "Consider using SecureNewtonsoftDeserialization.DeserializeObject instead. These secure methods are available in the Skyline.DataMiner.Utils.SecureCoding NuGet package.",
@@ -21,18 +28,72 @@ public class NewtonsoftDeserializationAnalyzer : DiagnosticAnalyzer
             isEnabledByDefault: true
          );
 
-        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get { return ImmutableArray.Create(Rule); } }
+        public static DiagnosticDescriptor DefaultSettingsRule => new DiagnosticDescriptor(
+            DiagnosticId,
+            "Insecure TypeNameHandling Usage Without SerializationBinder",
+            "Setting TypeNameHandling to '{0}' at the global level is insecure and may lead to remote code execution during deserialization",
+            "Usage",
+            DiagnosticSeverity.Warning,
+            helpLinkUri: $"https://github.com/SkylineCommunications/Skyline.DataMiner.Utils.SecureCoding/blob/main/docs/Rules/{DiagnosticId}.md",
+            isEnabledByDefault: true
+         );
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get { return ImmutableArray.Create(DeserializationRule); } }
 
         public override void Initialize(AnalysisContext context)
         {
             context.EnableConcurrentExecution();
 
             context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
 
-            context.RegisterSyntaxNodeAction(Analyze, SyntaxKind.InvocationExpression);
+            context.RegisterSyntaxNodeAction(AnalyzeJsonConvertInvocation, SyntaxKind.InvocationExpression);
+
+            context.RegisterSyntaxNodeAction(AnalyzeJsonConvertAssignment, SyntaxKind.SimpleAssignmentExpression);
         }
 
-        public static void Analyze(SyntaxNodeAnalysisContext context)
+        private static void AnalyzeJsonConvertAssignment(SyntaxNodeAnalysisContext context)
+        {
+            var assignment = (AssignmentExpressionSyntax)context.Node;
+            if (assignment == null)
+            {
+                return;
+            }
+
+            // Match JsonConvert.DefaultSettings = ...
+            if (!assignment.Left.ToString().Contains("JsonConvert.DefaultSettings"))
+            {
+                return;
+            }
+
+            // Look for lambda or method returning JsonSerializerSettings
+            if (!(assignment.Right is ParenthesizedLambdaExpressionSyntax lambda))
+            {
+                return;
+            }
+
+            var defaultSettings = GetDefaultSettings(lambda);
+            if (defaultSettings?.Initializer == null)
+            {
+                return;
+            }
+
+            foreach (var expr in defaultSettings.Initializer.Expressions.OfType<AssignmentExpressionSyntax>())
+            {
+                if (expr.Left is IdentifierNameSyntax leftName &&
+                    leftName.Identifier.Text == "TypeNameHandling" &&
+                    expr.Right is MemberAccessExpressionSyntax rightMember)
+                {
+                    var typeName = rightMember.Name.Identifier.Text;
+
+                    if (insecureTypeNames.Contains(typeName))
+                    {
+                        context.ReportDiagnostic(Diagnostic.Create(DefaultSettingsRule, expr.GetLocation(), typeName));
+                    }
+                }
+            }
+        }
+
+        private static void AnalyzeJsonConvertInvocation(SyntaxNodeAnalysisContext context)
         {
             if (!(context.Node is InvocationExpressionSyntax node))
             {
@@ -46,12 +107,30 @@ public static void Analyze(SyntaxNodeAnalysisContext context)
 
             context.ReportDiagnostic(
                 Diagnostic.Create(
-                    Rule,
+                    DeserializationRule,
                     node.GetLocation()
                 )
             );
         }
 
+        private static ObjectCreationExpressionSyntax GetDefaultSettings(ParenthesizedLambdaExpressionSyntax lambda)
+        {
+            if (lambda.Body is ObjectCreationExpressionSyntax creation)  // Handle: () => new JsonSerializerSettings { ... }
+            {
+                return creation;
+            }
+            else if (lambda.Body is BlockSyntax block) // Handle: () => { return new JsonSerializerSettings { ... }; }
+            {
+                return block.DescendantNodes()
+                    .OfType<ObjectCreationExpressionSyntax>()
+                    .FirstOrDefault(expr => expr.Type.ToString().Contains("JsonSerializerSettings"));
+            }
+            else
+            {
+                return null;
+            }
+        }
+
         private static bool IsJsonConvertDeserialization(InvocationExpressionSyntax node, SyntaxNodeAnalysisContext context)
         {
             if (!(node.Expression is MemberAccessExpressionSyntax deserializeObjectAccess))

SecureCoding/SecureCoding.csproj

@@ -37,4 +37,8 @@
     <PackageReference Include="System.Security.Cryptography.Pkcs" Version="9.0.2" />
   </ItemGroup>
 
+  <ItemGroup>
+    <Folder Include="SecureCryptography\" />
+  </ItemGroup>
+
 </Project>