Skip to content

Commit

Permalink
NET-971 S2612: Add secondary location message
Browse files Browse the repository at this point in the history
  • Loading branch information
@mary-georgiou-sonarsource
mary-georgiou-sonarsource authored and sonartech committed Jan 11, 2025
1 parent 0ffe49b commit a0f7eb4
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 172 deletions.
125 changes: 62 additions & 63 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/Hotspots/LooseFilePermissions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,86 +14,85 @@
* along with this program; if not, see https://sonarsource.com/license/ssal/
*/

namespace SonarAnalyzer.Rules.CSharp
namespace SonarAnalyzer.Rules.CSharp;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class LooseFilePermissions : LooseFilePermissionsBase<SyntaxKind>
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class LooseFilePermissions : LooseFilePermissionsBase<SyntaxKind>
{
protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;
protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;

public LooseFilePermissions() : this(AnalyzerConfiguration.Hotspot) { }
public LooseFilePermissions() : this(AnalyzerConfiguration.Hotspot) { }

internal LooseFilePermissions(IAnalyzerConfiguration configuration) : base(configuration) { }
internal LooseFilePermissions(IAnalyzerConfiguration configuration) : base(configuration) { }

protected override void VisitAssignments(SonarSyntaxNodeReportingContext context)
protected override void VisitAssignments(SonarSyntaxNodeReportingContext context)
{
var node = context.Node;
if (IsFileAccessPermissions(node, context.SemanticModel) && !node.IsPartOfBinaryNegationOrCondition())
{
var node = context.Node;
if (IsFileAccessPermissions(node, context.SemanticModel) && !node.IsPartOfBinaryNegationOrCondition())
{
context.ReportIssue(Rule, node);
}
context.ReportIssue(Rule, node);
}
}

protected override void VisitInvocations(SonarSyntaxNodeReportingContext context)
protected override void VisitInvocations(SonarSyntaxNodeReportingContext context)
{
var invocation = (InvocationExpressionSyntax)context.Node;
if ((IsSetAccessRule(invocation, context.SemanticModel) || IsAddAccessRule(invocation, context.SemanticModel))
&& (GetObjectCreation(invocation, context.SemanticModel) is { } objectCreation))
{
var invocation = (InvocationExpressionSyntax)context.Node;
if ((IsSetAccessRule(invocation, context.SemanticModel) || IsAddAccessRule(invocation, context.SemanticModel))
&& (GetObjectCreation(invocation, context.SemanticModel) is { } objectCreation))
{
var invocationLocation = invocation.GetLocation();
var secondaryLocation = objectCreation.Expression.GetLocation();
context.ReportIssue(Rule, invocationLocation, invocationLocation.StartLine() == secondaryLocation.StartLine() ? [] : [secondaryLocation.ToSecondary()]);
}
var invocationLocation = invocation.GetLocation();
var secondaryLocation = objectCreation.Expression.GetLocation();
context.ReportIssue(Rule, invocationLocation, invocationLocation.StartLine() == secondaryLocation.StartLine() ? [] : [secondaryLocation.ToSecondary(MessageFormat)]);
}
}

private static bool IsSetAccessRule(InvocationExpressionSyntax invocation, SemanticModel semanticModel) =>
invocation.IsMemberAccessOnKnownType("SetAccessRule", KnownType.System_Security_AccessControl_FileSystemSecurity, semanticModel);
private static bool IsSetAccessRule(InvocationExpressionSyntax invocation, SemanticModel model) =>
invocation.IsMemberAccessOnKnownType("SetAccessRule", KnownType.System_Security_AccessControl_FileSystemSecurity, model);

private static bool IsAddAccessRule(InvocationExpressionSyntax invocation, SemanticModel semanticModel) =>
invocation.IsMemberAccessOnKnownType("AddAccessRule", KnownType.System_Security_AccessControl_FileSystemSecurity, semanticModel);
private static bool IsAddAccessRule(InvocationExpressionSyntax invocation, SemanticModel model) =>
invocation.IsMemberAccessOnKnownType("AddAccessRule", KnownType.System_Security_AccessControl_FileSystemSecurity, model);

private static IObjectCreation GetObjectCreation(InvocationExpressionSyntax invocation, SemanticModel semanticModel)
private static IObjectCreation GetObjectCreation(InvocationExpressionSyntax invocation, SemanticModel model)
{
if (GetVulnerableFileSystemAccessRule(invocation.DescendantNodes()) is { } accessRuleSyntaxNode)
{
if (GetVulnerableFileSystemAccessRule(invocation.DescendantNodes()) is { } accessRuleSyntaxNode)
{
return accessRuleSyntaxNode;
}
else if (invocation.GetArgumentSymbolsOfKnownType(KnownType.System_Security_AccessControl_FileSystemAccessRule, semanticModel).FirstOrDefault() is { } accessRuleSymbol
&& !(accessRuleSymbol is IMethodSymbol))
{
return GetVulnerableFileSystemAccessRule(accessRuleSymbol.GetLocationNodes(invocation));
}
else
{
return null;
}

IObjectCreation GetVulnerableFileSystemAccessRule(IEnumerable<SyntaxNode> nodes) =>
FilterObjectCreations(nodes).FirstOrDefault(x => IsFileSystemAccessRuleForEveryoneWithAllow(x, semanticModel));
return accessRuleSyntaxNode;
}
else if (invocation.GetArgumentSymbolsOfKnownType(KnownType.System_Security_AccessControl_FileSystemAccessRule, model).FirstOrDefault() is { } accessRuleSymbol
&& accessRuleSymbol is not IMethodSymbol)
{
return GetVulnerableFileSystemAccessRule(accessRuleSymbol.GetLocationNodes(invocation));
}
else
{
return null;
}

private static bool IsFileSystemAccessRuleForEveryoneWithAllow(IObjectCreation objectCreation, SemanticModel semanticModel) =>
objectCreation.IsKnownType(KnownType.System_Security_AccessControl_FileSystemAccessRule, semanticModel)
&& objectCreation.ArgumentList is { } argumentList
&& IsEveryone(argumentList.Arguments.First().Expression, semanticModel)
&& semanticModel.GetConstantValue(argumentList.Arguments.Last().Expression) is { HasValue: true, Value: 0 };
IObjectCreation GetVulnerableFileSystemAccessRule(IEnumerable<SyntaxNode> nodes) =>
FilterObjectCreations(nodes).FirstOrDefault(x => IsFileSystemAccessRuleForEveryoneWithAllow(x, model));
}

private static bool IsEveryone(SyntaxNode syntaxNode, SemanticModel semanticModel) =>
semanticModel.GetConstantValue(syntaxNode) is { HasValue: true, Value: Everyone }
|| FilterObjectCreations(syntaxNode.DescendantNodesAndSelf()).Any(x => IsNTAccountWithEveryone(x, semanticModel) || IsSecurityIdentifierWithEveryone(x, semanticModel));
private static bool IsFileSystemAccessRuleForEveryoneWithAllow(IObjectCreation objectCreation, SemanticModel model) =>
objectCreation.IsKnownType(KnownType.System_Security_AccessControl_FileSystemAccessRule, model)
&& objectCreation.ArgumentList is { } argumentList
&& IsEveryone(argumentList.Arguments.First().Expression, model)
&& model.GetConstantValue(argumentList.Arguments.Last().Expression) is { HasValue: true, Value: 0 };

private static bool IsNTAccountWithEveryone(IObjectCreation objectCreation, SemanticModel semanticModel) =>
objectCreation.IsKnownType(KnownType.System_Security_Principal_NTAccount, semanticModel)
&& objectCreation.ArgumentList is { } argumentList
&& semanticModel.GetConstantValue(argumentList.Arguments.Last().Expression) is { HasValue: true, Value: Everyone };
private static bool IsEveryone(SyntaxNode node, SemanticModel model) =>
model.GetConstantValue(node) is { HasValue: true, Value: Everyone }
|| FilterObjectCreations(node.DescendantNodesAndSelf()).Any(x => IsNTAccountWithEveryone(x, model) || IsSecurityIdentifierWithEveryone(x, model));

private static bool IsSecurityIdentifierWithEveryone(IObjectCreation objectCreation, SemanticModel semanticModel) =>
objectCreation.IsKnownType(KnownType.System_Security_Principal_SecurityIdentifier, semanticModel)
&& objectCreation.ArgumentList is { } argumentList
&& semanticModel.GetConstantValue(argumentList.Arguments.First().Expression) is { HasValue: true, Value: 1 };
private static bool IsNTAccountWithEveryone(IObjectCreation objectCreation, SemanticModel model) =>
objectCreation.IsKnownType(KnownType.System_Security_Principal_NTAccount, model)
&& objectCreation.ArgumentList is { } argumentList
&& model.GetConstantValue(argumentList.Arguments.Last().Expression) is { HasValue: true, Value: Everyone };

private static IEnumerable<IObjectCreation> FilterObjectCreations(IEnumerable<SyntaxNode> nodes) =>
nodes.Where(x => x.Kind() is SyntaxKind.ObjectCreationExpression or SyntaxKindEx.ImplicitObjectCreationExpression)
.Select(ObjectCreationFactory.Create);
}
private static bool IsSecurityIdentifierWithEveryone(IObjectCreation objectCreation, SemanticModel model) =>
objectCreation.IsKnownType(KnownType.System_Security_Principal_SecurityIdentifier, model)
&& objectCreation.ArgumentList is { } argumentList
&& model.GetConstantValue(argumentList.Arguments.First().Expression) is { HasValue: true, Value: 1 };

private static IEnumerable<IObjectCreation> FilterObjectCreations(IEnumerable<SyntaxNode> nodes) =>
nodes.Where(x => x.Kind() is SyntaxKind.ObjectCreationExpression or SyntaxKindEx.ImplicitObjectCreationExpression)
.Select(ObjectCreationFactory.Create);
}
60 changes: 30 additions & 30 deletions analyzers/src/SonarAnalyzer.Core/Rules/Hotspots/LooseFilePermissionsBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,40 +14,40 @@
* along with this program; if not, see https://sonarsource.com/license/ssal/
*/

namespace SonarAnalyzer.Rules
namespace SonarAnalyzer.Rules;

public abstract class LooseFilePermissionsBase<TSyntaxKind> : HotspotDiagnosticAnalyzer
where TSyntaxKind : struct
{
public abstract class LooseFilePermissionsBase<TSyntaxKind> : HotspotDiagnosticAnalyzer
where TSyntaxKind : struct
{
protected const string DiagnosticId = "S2612";
protected const string Everyone = "Everyone";
private const string MessageFormat = "Make sure this permission is safe.";
protected const string DiagnosticId = "S2612";
protected const string Everyone = "Everyone";
protected const string MessageFormat = "Make sure this permission is safe.";

protected readonly DiagnosticDescriptor Rule;

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
protected readonly DiagnosticDescriptor Rule;
protected abstract ILanguageFacade<TSyntaxKind> Language { get; }
protected abstract void VisitAssignments(SonarSyntaxNodeReportingContext context);
protected abstract void VisitInvocations(SonarSyntaxNodeReportingContext context);

protected abstract ILanguageFacade<TSyntaxKind> Language { get; }
protected abstract void VisitAssignments(SonarSyntaxNodeReportingContext context);
protected abstract void VisitInvocations(SonarSyntaxNodeReportingContext context);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

protected LooseFilePermissionsBase(IAnalyzerConfiguration configuration) : base(configuration) =>
Rule = Language.CreateDescriptor(DiagnosticId, MessageFormat);
protected LooseFilePermissionsBase(IAnalyzerConfiguration configuration) : base(configuration) =>
Rule = Language.CreateDescriptor(DiagnosticId, MessageFormat);

protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterCompilationStartAction(c =>
protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterCompilationStartAction(c =>
{
if (!IsEnabled(c.Options))
{
if (!IsEnabled(c.Options))
{
return;
}

c.RegisterNodeAction(Language.GeneratedCodeRecognizer, VisitInvocations, Language.SyntaxKind.InvocationExpression);
c.RegisterNodeAction(Language.GeneratedCodeRecognizer, VisitAssignments, Language.SyntaxKind.IdentifierName);
});

protected bool IsFileAccessPermissions(SyntaxNode syntaxNode, SemanticModel semanticModel) =>
Language.Syntax.NodeIdentifier(syntaxNode) is { } identifier
&& LooseFilePermissionsConfig.WeakFileAccessPermissions.Contains(identifier.Text)
&& syntaxNode.IsKnownType(KnownType.Mono_Unix_FileAccessPermissions, semanticModel);
}
return;
}

c.RegisterNodeAction(Language.GeneratedCodeRecognizer, VisitInvocations, Language.SyntaxKind.InvocationExpression);
c.RegisterNodeAction(Language.GeneratedCodeRecognizer, VisitAssignments, Language.SyntaxKind.IdentifierName);
});

protected bool IsFileAccessPermissions(SyntaxNode node, SemanticModel model) =>
Language.Syntax.NodeIdentifier(node) is { } identifier
&& LooseFilePermissionsConfig.WeakFileAccessPermissions.Contains(identifier.Text)
&& node.IsKnownType(KnownType.Mono_Unix_FileAccessPermissions, model);
}
11 changes: 5 additions & 6 deletions analyzers/src/SonarAnalyzer.Core/Rules/Hotspots/LooseFilePermissionsConfig.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@
* along with this program; if not, see https://sonarsource.com/license/ssal/
*/

namespace SonarAnalyzer.Rules
namespace SonarAnalyzer.Rules;

public static class LooseFilePermissionsConfig
{
public static class LooseFilePermissionsConfig
{
public static readonly ImmutableHashSet<string> WeakFileAccessPermissions =
ImmutableHashSet.Create("AllPermissions", "DefaultPermissions", "OtherExecute", "OtherWrite", "OtherRead", "OtherReadWriteExecute");
}
public static readonly ImmutableHashSet<string> WeakFileAccessPermissions =
ImmutableHashSet.Create("AllPermissions", "DefaultPermissions", "OtherExecute", "OtherWrite", "OtherRead", "OtherReadWriteExecute");
}
Loading

0 comments on commit a0f7eb4

Please sign in to comment.