Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,6 @@ ClientBin/
*.dbmdl
*.dbproj.schemaview
*.jfm
*.pfx
*.publishsettings
orleans.codegen.cs

Expand Down
12 changes: 3 additions & 9 deletions Analyzers/Analyzers.CodeFixes/SecureIO/PathCombineCodeFix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,25 +70,19 @@ private async Task<Document> ReplacePathCombineWithConstructSecurePath(Document

private static InvocationExpressionSyntax GetNewInvocation(InvocationExpressionSyntax invocation)
{
var arguments = invocation.ArgumentList.Arguments;
if (arguments == null)
if (invocation.ArgumentList.Arguments.Count < 2)
{
return invocation;
}

if (arguments.Count < 2)
{
return invocation;
}
else if (arguments.Count > 2)
else if (invocation.ArgumentList.Arguments.Count > 2)
{
return invocation
.WithExpression(SyntaxFactory.ParseExpression("SecurePath.ConstructSecurePath"))
.WithArgumentList(invocation.ArgumentList);
}
else
{
var lastArgument = arguments.Last().ToString();
var lastArgument = invocation.ArgumentList.Arguments.Last().ToString();

if (lastArgument.Contains("/") || lastArgument.Contains("\\"))
{
Expand Down
127 changes: 112 additions & 15 deletions Analyzers/Analyzers.CodeFixes/SecureReflection/SecureAssemblyCodeFix.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Skyline.DataMiner.Utils.SecureCoding.Analyzers.SecureReflection;
using Skyline.DataMiner.Utils.SecureCoding.SecureReflection;
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
Expand All @@ -15,7 +18,22 @@ namespace Skyline.DataMiner.Utils.SecureCoding.CodeFixProviders.SecureReflection
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(SecureAssemblyCodeFix)), Shared]
public class SecureAssemblyCodeFix : CodeFixProvider
{
private const string SECURE_REFLECTION_NAMESPACE = "Skyline.DataMiner.Utils.SecureCoding.SecureReflection";
private static string[] requiredNamespaces = new string[]
{
"System.Security.Cryptography.X509Certificates",
"Skyline.DataMiner.Utils.SecureCoding.SecureReflection",
};

private static Dictionary<string, string> defaultCertificateArguments = new Dictionary<string, string>
{
{ "X509Certificate2", "default(X509Certificate2)" },
{ "IEnumerable<X509Certificate2>", "default(IEnumerable<X509Certificate2>)" },
{ "Certificate path", "default(string)" },
{ "Certificate paths", "default(string[])" },
{ "byte[]", "default(byte[])" },
{ "IEnumerable<byte[]>", "default(IEnumerable<byte[]>)" }
};


public sealed override ImmutableArray<string> FixableDiagnosticIds
{
Expand All @@ -40,45 +58,124 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
return;
}

context.RegisterCodeFix(
CodeAction.Create(
title: "Replace Assembly.Load with SecureAssembly.Load",
createChangedDocument: c => ReplaceAssemblyLoadWithSecureAssemblyLoad(context.Document, invocation, c),
equivalenceKey: nameof(SecureAssemblyCodeFix)),
diagnostic);
if (!TryGetNestedCodeAction(context, invocation, out CodeAction[] nestedCodeActions))
{
return;
}

var codeAction = CodeActionWithOptions.Create(
title: "Replace with secure assembly loading",
nestedActions: ImmutableArray.Create<CodeAction>(nestedCodeActions),
isInlinable: false);

context.RegisterCodeFix(codeAction, diagnostic);
}

private bool TryGetNestedCodeAction(CodeFixContext context, InvocationExpressionSyntax invocation, out CodeAction[] nestedCodeActionsArr)
{
var nestedCodeActions = new List<CodeAction>();

var methodSymbol = GetAssemblyMethodSymbol(context, invocation);
if (methodSymbol == null)
{
nestedCodeActionsArr = new CodeAction[0];
return false;
}

if (methodSymbol.Name == nameof(System.Reflection.Assembly.LoadFrom))
{
AddNestedLoadFromCodeActions(context, invocation, nameof(SecureAssembly.LoadFrom), nestedCodeActions);
}

if (methodSymbol.Name == nameof(System.Reflection.Assembly.LoadFile))
{
AddNestedLoadFromCodeActions(context, invocation, nameof(SecureAssembly.LoadFile), nestedCodeActions);
}

nestedCodeActionsArr = nestedCodeActions.ToArray();
return nestedCodeActionsArr.Length > 0;
}

private async Task<Document> ReplaceAssemblyLoadWithSecureAssemblyLoad(
private void AddNestedLoadFromCodeActions(
CodeFixContext context,
InvocationExpressionSyntax invocation,
string method,
List<CodeAction> nestedCodeActions)
{
var secureAssemblyMethod = $"SecureAssembly.{method}";

foreach (var defaultCertificateArg in defaultCertificateArguments)
{
var title = $"'{secureAssemblyMethod}' ({defaultCertificateArg.Key} load option)";
var argument = defaultCertificateArg.Value;

nestedCodeActions.Add(CodeAction.Create(
title,
createChangedDocument: c => ReplaceAssemblyLoadSecureAssemblyLoad(context.Document, invocation, secureAssemblyMethod, argument, c),
equivalenceKey: nameof(SecureAssemblyCodeFix)));
}
}

private static IMethodSymbol GetAssemblyMethodSymbol(CodeFixContext context, InvocationExpressionSyntax invocation)
{
var semanticModel = context.Document.GetSemanticModelAsync(context.CancellationToken)?.Result;
if (semanticModel == null)
{
return null;
}

var methodSymbol = semanticModel?.GetSymbolInfo(invocation.Expression, context.CancellationToken).Symbol as IMethodSymbol;
if (methodSymbol == null)
{
return null;
}

if (!methodSymbol.ReceiverType.ToDisplayString().Contains("System.Reflection.Assembly"))
{
return null;
}

return methodSymbol;
}

private async Task<Document> ReplaceAssemblyLoadSecureAssemblyLoad(
Document document,
InvocationExpressionSyntax invocation,
string secureAssemblyMethod,
string defaultCertificateArgument,
CancellationToken cancellationToken)
{
var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);

InvocationExpressionSyntax newInvocation = GetNewInvocation(invocation);
InvocationExpressionSyntax newInvocation = GetNewInvocation(invocation, secureAssemblyMethod, defaultCertificateArgument);

var newRoot = root.ReplaceNode(invocation, newInvocation.WithTriviaFrom(invocation)); // Keep original form

var compilationUnit = newRoot as CompilationUnitSyntax ?? (CompilationUnitSyntax)root;

if (!compilationUnit.Usings.Any(@using => @using.Name.ToString() == SECURE_REFLECTION_NAMESPACE))
var missingNamespaces = requiredNamespaces
.Where(ns => !compilationUnit.Usings.Any(u => u.Name.ToString() == ns))
.Select(ns => SyntaxFactory.UsingDirective(SyntaxFactory.ParseName(ns)))
.ToArray();

if (missingNamespaces.Length > 0)
{
compilationUnit = compilationUnit.AddUsings(SyntaxFactory.UsingDirective(SyntaxFactory.ParseName(SECURE_REFLECTION_NAMESPACE)));
compilationUnit = compilationUnit.AddUsings(missingNamespaces);
}

var newDocument = document.WithSyntaxRoot(compilationUnit);

return newDocument;
}

private static InvocationExpressionSyntax GetNewInvocation(InvocationExpressionSyntax invocation)
private static InvocationExpressionSyntax GetNewInvocation(InvocationExpressionSyntax invocation, string secureAssemblyMethod, string defaultCertificateArgument)
{
var arguments = invocation.ArgumentList
.AddArguments(SyntaxFactory.Argument(SyntaxFactory.ParseExpression("allowedCertificates")));
.AddArguments(SyntaxFactory.Argument(SyntaxFactory.ParseExpression(defaultCertificateArgument)));

return invocation
.WithExpression(SyntaxFactory.ParseExpression("SecureAssembly.Load"))
.WithExpression(SyntaxFactory.ParseExpression(secureAssemblyMethod))
.WithArgumentList(arguments);
}
}
}
}
2 changes: 1 addition & 1 deletion Analyzers/Analyzers.Test/AnalyzerVerifierHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public static CSharpCodeFixTest<T1, T2, MSTestVerifier> BuildAnalyzerVerifierWit
)
.AddPackages(packageToAdd)
},

FixedState =
{
Sources = { fixedStateSources },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Skyline.DataMiner.Utils.SecureCoding.Analyzers.SecureReflection;
using Skyline.DataMiner.Utils.SecureCoding.SecureReflection;

#pragma warning disable S2699 // Tests should include assertions is not valid for Roslyn Analyzers Unit Tests.
namespace Skyline.DataMiner.Utils.SecureCoding.Analyzers.Tests.SecureReflection
{
[TestClass]
Expand All @@ -14,29 +15,77 @@ public class SecureAssemblyAnalyzerUnitTests
private readonly string insecureAssemblyTestCase = File.ReadAllText(
@"..\..\..\SecureReflection\TestScenarios\InsecureAssembly.test");

private readonly string insecureAssemblyCodeFix = File.ReadAllText(
@"..\..\..\SecureReflection\TestScenarios\InsecureAssembly.fix");

private readonly string defaultArgumentTestCase = File.ReadAllText(
@"..\..\..\SecureReflection\TestScenarios\DefaultArgument.test");

private readonly string bypassCertificateChainTestCase = File.ReadAllText(
@"..\..\..\SecureReflection\TestScenarios\BypassCertificateChain.test");

[TestMethod]
public async Task VerifyInsecureAssemblyUsageDiagnostic()
public async Task VerifyDefaultArgumentDiagnostic()
{
var expectedDiagnostics = new List<DiagnosticResult>()
{
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureAssembly, 7, 46),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureAssembly, 8, 44),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureAssembly, 9, 44),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureAssembly, 13, 13),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureAssembly, 14, 4),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureAssembly, 15, 4),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureAssembly, 20, 13),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureAssembly, 21, 4),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureAssembly, 22, 4),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureAssembly, 29, 24),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureAssembly, 37, 24),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureAssembly, 45, 24),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleDefaultArgument, 9, 84),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleDefaultArgument, 10, 84),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleDefaultArgument, 14, 44),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleDefaultArgument, 15, 44),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleDefaultArgument, 20, 44),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleDefaultArgument, 21, 44),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleDefaultArgument, 28, 64),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleDefaultArgument, 36, 64),
};

// [NOTE]: SecureAssemblyCodeFix cannot be unit tested since it introduces a compile error on it's own.
var analyzerVerifier = AnalyzerVerifierHelper.BuildAnalyzerVerifier<SecureAssemblyAnalyzer>(
defaultArgumentTestCase);

analyzerVerifier.TestState.AdditionalReferences.Add(typeof(SecureAssembly).Assembly);

analyzerVerifier.TestState.ExpectedDiagnostics.AddRange(expectedDiagnostics);

await analyzerVerifier.RunAsync();
}

[TestMethod]
public void VerifyInsecureAssemblyCodeFix()
{
var expectedCodeFix = insecureAssemblyTestCase
.Replace(
@"Assembly.LoadFile(""assemblyPath"");",
@"SecureAssembly.LoadFile(""assemblyPath"", default(X509Certificate2));")
.Replace(
@"Assembly.LoadFrom(""assemblyPath"");",
@"SecureAssembly.LoadFrom(""assemblyPath"", default(X509Certificate2));")
.Replace(
@"using System.Reflection;",
"using System.Reflection;"
+ "\r\nusing System.Security.Cryptography.X509Certificates;"
+ "\r\nusing Skyline.DataMiner.Utils.SecureCoding.SecureReflection;");

Assert.AreEqual(expectedCodeFix, insecureAssemblyCodeFix);
}

[TestMethod]
public async Task VerifyInsecureAssemblyUsageDiagnostic()
{
var expectedDiagnostics = new List<DiagnosticResult>()
{
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureLoadAssembly, 7, 46),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureLoadFileAssembly, 8, 44),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureLoadFromAssembly, 9, 44),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureLoadAssembly, 13, 13),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureLoadFileAssembly, 14, 4),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureLoadFromAssembly, 15, 4),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureLoadAssembly, 20, 13),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureLoadFileAssembly, 21, 4),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureLoadFromAssembly, 22, 4),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureLoadAssembly, 29, 24),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureLoadFileAssembly, 37, 24),
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleInsecureLoadFromAssembly, 45, 24),
};

var analyzerVerifier = AnalyzerVerifierHelper.BuildAnalyzerVerifier<SecureAssemblyAnalyzer>(
insecureAssemblyTestCase);
Expand All @@ -59,8 +108,6 @@ public async Task VerifyBypassCertificateChain()
AnalyzerVerifierHelper.BuildDiagnosticResult(SecureAssemblyAnalyzer.RuleBypassCertificateChain, 46, 24),
};

// [NOTE]: SecureAssemblyCodeFix cannot be unit tested since it introduces a compile error on it's own.

var analyzerVerifier = AnalyzerVerifierHelper.BuildAnalyzerVerifier<SecureAssemblyAnalyzer>(
bypassCertificateChainTestCase);

Expand All @@ -71,4 +118,5 @@ public async Task VerifyBypassCertificateChain()
await analyzerVerifier.RunAsync();
}
}
}
}
#pragma warning restore S2699 // Tests should include assertions is not valid for Roslyn Analyzers Unit Tests.
Original file line number Diff line number Diff line change
Expand Up @@ -5,45 +5,45 @@ namespace Skyline.DataMiner.Utils.SecureCoding.Analyzers.Tests.SecureReflection.
{
internal class BypassCertificateChain
{
private Assembly secureLoadAssemblyField = SecureAssembly.Load("assemblyPath", "targetDllPath", verifyCertificateChain: false);
private Assembly secureLoadAssemblyField = SecureAssembly.LoadFrom("assemblyPath", "targetDllPath", verifyCertificateChain: false);

public BypassCertificateChain()
{
SecureAssembly.Load("assemblyPath", "targetDllPath", verifyCertificateChain: false);
SecureAssembly.LoadFrom("assemblyPath", "targetDllPath", verifyCertificateChain: false);
}

public void TobeDetected()
{
SecureAssembly.Load("assemblyPath", "targetDllPath", verifyCertificateChain: false);
SecureAssembly.LoadFrom("assemblyPath", "targetDllPath", verifyCertificateChain: false);
}

public void NotTobeDetected()
{
SecureAssembly.Load("assemblyPath", "targetDllPath", verifyCertificateChain: true);
SecureAssembly.Load("assemblyPath", "targetDllPath");
SecureAssembly.LoadFrom("assemblyPath", "targetDllPath", verifyCertificateChain: true);
SecureAssembly.LoadFrom("assemblyPath", "targetDllPath");
}

public Assembly SecureLoadAssemblyProperty
{
get
{
return SecureAssembly.Load("assemblyPath", "targetDllPath", verifyCertificateChain: false);
return SecureAssembly.LoadFrom("assemblyPath", "targetDllPath", verifyCertificateChain: false);
}
}

public Assembly SecureLoadFileAssemblyProperty
{
get
{
return SecureAssembly.Load("assemblyPath", "targetDllPath", verifyCertificateChain: false);
return SecureAssembly.LoadFrom("assemblyPath", "targetDllPath", verifyCertificateChain: false);
}
}

public Assembly SecureLoadFromAssemblyProperty
{
get
{
return SecureAssembly.Load("assemblyPath", "targetDllPath", verifyCertificateChain: false);
return SecureAssembly.LoadFrom("assemblyPath", "targetDllPath", verifyCertificateChain: false);
}
}
}
Expand Down
Loading