Skip to content

Commit

Permalink
Rule S2068: detect hard-coded passwords in web.config files (#6182)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pavel-mikula-sonarsource
pavel-mikula-sonarsource committed Oct 12, 2022
1 parent 852b3b1 commit 19f6fdb
Show file tree
Hide file tree
Showing 20 changed files with 273 additions and 115 deletions.
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.CSharp/Rules/DatabasePasswordsShouldBeSecure.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ protected override void Initialize(SonarAnalysisContext context)

private void CheckWebConfig(SonarAnalysisContext context, CompilationAnalysisContext c)
{
foreach (var fullPath in context.GetWebConfig(c))
foreach (var fullPath in context.WebConfigFiles(c))
{
var webConfig = File.ReadAllText(fullPath);
if (webConfig.Contains("<connectionStrings>") && XmlHelper.ParseXDocument(webConfig) is { } doc)
Expand Down
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.Common/Helpers/SonarAnalysisContext.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ internal void RegisterSyntaxNodeAction<TLanguageKindEnum>(Action<SyntaxNodeAnaly
where TLanguageKindEnum : struct =>
RegisterContextAction(x => context.RegisterSyntaxNodeAction(x, syntaxKinds), action, c => c.GetSyntaxTree(), c => c.Compilation, c => c.Options);

internal IEnumerable<string> GetWebConfig(CompilationAnalysisContext c)
internal IEnumerable<string> WebConfigFiles(CompilationAnalysisContext c)
{
return ProjectConfiguration(c.Options).FilesToAnalyze.FindFiles(WebConfigRegex).Where(ShouldProcess);

Expand Down
16 changes: 12 additions & 4 deletions analyzers/src/SonarAnalyzer.Common/Helpers/XmlHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,27 @@ public static XDocument ParseXDocument(string text)
? attribute
: null;

public static Location CreateLocation(this XAttribute attribute, string path)
public static Location CreateLocation(this XAttribute attribute, string path) =>
CreateLocation(attribute, path, attribute.Name);

public static Location CreateLocation(this XElement element, string path) =>
CreateLocation(element, path, element.Name);

private static Location CreateLocation(IXmlLineInfo startPos, string path, XName name)
{
// IXmlLineInfo is 1-based, whereas Roslyn is zero-based
var startPos = (IXmlLineInfo)attribute;
if (startPos.HasLineInfo())
{
// LoadOptions.PreserveWhitespace doesn't preserve whitespace inside nodes and attributes => there's no easy way to find full length of a XAttribute.
var length = attribute.Name.ToString().Length;
var length = name.ToString().Length;
var start = new LinePosition(startPos.LineNumber - 1, startPos.LinePosition - 1);
var end = new LinePosition(startPos.LineNumber - 1, startPos.LinePosition - 1 + length);
return Location.Create(path, new TextSpan(start.Line, length), new LinePositionSpan(start, end));
}
return null;
else
{
return null;
}
}
}
}
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DisablingRequestValidationBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ private void CheckWebConfig(SonarAnalysisContext context, CompilationAnalysisCon
return;
}

foreach (var fullPath in context.GetWebConfig(c))
foreach (var fullPath in context.WebConfigFiles(c))
{
var webConfig = File.ReadAllText(fullPath);
if (webConfig.Contains("<system.web>") && XmlHelper.ParseXDocument(webConfig) is { } doc)
Expand Down
164 changes: 101 additions & 63 deletions analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
using System.Xml.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using SonarAnalyzer.Common;
Expand All @@ -43,6 +45,8 @@ public abstract class DoNotHardcodeCredentialsBase<TSyntaxKind> : ParameterLoadi

private readonly IAnalyzerConfiguration configuration;
private readonly DiagnosticDescriptor rule;
private readonly Regex validCredentialPattern = new(@"^(\?|:\w+|\{\d+[^}]*\}|""|')$", RegexOptions.IgnoreCase);
private readonly Regex uriUserInfoPattern;
private string credentialWords;
private IEnumerable<string> splitCredentialWords;
private Regex passwordValuePattern;
Expand Down Expand Up @@ -71,6 +75,13 @@ public string CredentialWords

protected DoNotHardcodeCredentialsBase(IAnalyzerConfiguration configuration)
{
const string Rfc3986_Unreserved = "-._~"; // Numbers and letters are embedded in regex itself without escaping
const string Rfc3986_Pct = "%";
const string Rfc3986_SubDelims = "!$&'()*+,;=";
const string UriPasswordSpecialCharacters = Rfc3986_Unreserved + Rfc3986_Pct + Rfc3986_SubDelims;
// See https://tools.ietf.org/html/rfc3986 Userinfo can contain groups: unreserved | pct-encoded | sub-delims
var uriUserInfoPart = @"[\w\d" + Regex.Escape(UriPasswordSpecialCharacters) + "]+";
uriUserInfoPattern = new Regex(@"\w+:\/\/(?<Login>" + uriUserInfoPart + "):(?<Password>" + uriUserInfoPart + ")@", RegexOptions.Compiled);
this.configuration = configuration;
rule = Language.CreateDescriptor(DiagnosticId, MessageFormat);
CredentialWords = DefaultCredentialWords; // Property will initialize multiple state variables
Expand Down Expand Up @@ -98,6 +109,7 @@ protected sealed override void Initialize(ParameterLoadingAnalysisContext contex
pa.MatchProperty(new MemberDescriptor(KnownType.System_Net_NetworkCredential, "Password")));

InitializeActions(context);
context.Context.RegisterCompilationAction(c => CheckWebConfig(context.Context, c));
}

protected bool IsEnabled(AnalyzerOptions options)
Expand All @@ -106,89 +118,115 @@ protected bool IsEnabled(AnalyzerOptions options)
return configuration.IsEnabled(DiagnosticId);
}

private void CheckWebConfig(SonarAnalysisContext context, CompilationAnalysisContext c)
{
foreach (var path in context.WebConfigFiles(c))
{
if (XmlHelper.ParseXDocument(File.ReadAllText(path)) is { } doc)
{
CheckWebConfig(c, path, doc.Descendants());
}
}
}

private void CheckWebConfig(CompilationAnalysisContext c, string path, IEnumerable<XElement> elements)
{
foreach (var element in elements)
{
if (!element.HasElements && IssueMessage(element.Name.LocalName, element.Value) is { } elementMessage && element.CreateLocation(path) is { } elementLocation)
{
c.ReportIssue(Diagnostic.Create(rule, elementLocation, elementMessage));
}
foreach (var attribute in element.Attributes())
{
if (IssueMessage(attribute.Name.LocalName, attribute.Value) is { } attributeMessage && attribute.CreateLocation(path) is { } attributeLocation)
{
c.ReportIssue(Diagnostic.Create(rule, attributeLocation, attributeMessage));
}
}
}
}

private string IssueMessage(string variableName, string variableValue)
{
var bannedWords = FindCredentialWords(variableName, variableValue);
if (bannedWords.Any())
{
return string.Format(MessageFormatCredential, bannedWords.JoinAnd());
}
else if (ContainsUriUserInfo(variableValue))
{
return MessageUriUserInfo;
}
else
{
return null;
}
}

private IEnumerable<string> FindCredentialWords(string variableName, string variableValue)
{
var credentialWordsFound = variableName
.SplitCamelCaseToWords()
.Intersect(splitCredentialWords)
.ToHashSet(StringComparer.OrdinalIgnoreCase);

if (credentialWordsFound.Any(x => variableValue.IndexOf(x, StringComparison.InvariantCultureIgnoreCase) >= 0))
{
// See https://github.com/SonarSource/sonar-dotnet/issues/2868
return Enumerable.Empty<string>();
}

var match = passwordValuePattern.Match(variableValue);
if (match.Success && !IsValidCredential(match.Groups["suffix"].Value))
{
credentialWordsFound.Add(match.Groups["credential"].Value);
}

// Rule was initially implemented with everything lower (which is wrong) so we have to force lower before reporting to avoid new issues to appear on SQ/SC.
return credentialWordsFound.Select(x => x.ToLowerInvariant());
}

private bool IsValidCredential(string suffix)
{
var candidateCredential = suffix.Split(CredentialSeparator).First().Trim();
return string.IsNullOrWhiteSpace(candidateCredential) || validCredentialPattern.IsMatch(candidateCredential);
}

private bool ContainsUriUserInfo(string variableValue)
{
var match = uriUserInfoPattern.Match(variableValue);
return match.Success
&& match.Groups["Password"].Value is { } password
&& !string.Equals(match.Groups["Login"].Value, password, StringComparison.OrdinalIgnoreCase)
&& password != CredentialSeparator.ToString()
&& !validCredentialPattern.IsMatch(password);
}

protected abstract class CredentialWordsFinderBase<TSyntaxNode>
where TSyntaxNode : SyntaxNode
{
private readonly Regex validCredentialPattern = new(@"^\?|:\w+|\{\d+[^}]*\}|""|'$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
private readonly Regex uriUserInfoPattern;
private readonly DoNotHardcodeCredentialsBase<TSyntaxKind> analyzer;

protected abstract bool ShouldHandle(TSyntaxNode syntaxNode, SemanticModel semanticModel);
protected abstract string GetVariableName(TSyntaxNode syntaxNode);
protected abstract string GetAssignedValue(TSyntaxNode syntaxNode, SemanticModel semanticModel);

protected CredentialWordsFinderBase(DoNotHardcodeCredentialsBase<TSyntaxKind> analyzer)
{
// See https://tools.ietf.org/html/rfc3986 Userinfo can contain groups: unreserved | pct-encoded | sub-delims
const string Rfc3986_Unreserved = "-._~"; // Numbers and letters are embeded in regex itself without escaping
const string Rfc3986_Pct = "%";
const string Rfc3986_SubDelims = "!$&'()*+,;=";
const string UriPasswordSpecialCharacters = Rfc3986_Unreserved + Rfc3986_Pct + Rfc3986_SubDelims;
var uriUserInfoPart = @"[\w\d" + Regex.Escape(UriPasswordSpecialCharacters) + "]+";

protected CredentialWordsFinderBase(DoNotHardcodeCredentialsBase<TSyntaxKind> analyzer) =>
this.analyzer = analyzer;
uriUserInfoPattern = new Regex(@"\w+:\/\/(?<Login>" + uriUserInfoPart + "):(?<Password>" + uriUserInfoPart + ")@", RegexOptions.Compiled);
}

public Action<SyntaxNodeAnalysisContext> AnalysisAction() =>
context =>
{
var declarator = (TSyntaxNode)context.Node;
if (ShouldHandle(declarator, context.SemanticModel)
&& GetAssignedValue(declarator, context.SemanticModel) is { } variableValue
&& !string.IsNullOrWhiteSpace(variableValue))
&& !string.IsNullOrWhiteSpace(variableValue)
&& analyzer.IssueMessage(GetVariableName(declarator), variableValue) is { } message)
{
var bannedWords = FindCredentialWords(GetVariableName(declarator), variableValue);
if (bannedWords.Any())
{
context.ReportIssue(Diagnostic.Create(analyzer.rule, declarator.GetLocation(), string.Format(MessageFormatCredential, bannedWords.JoinAnd())));
}
else if (ContainsUriUserInfo(variableValue))
{
context.ReportIssue(Diagnostic.Create(analyzer.rule, declarator.GetLocation(), MessageUriUserInfo));
}
context.ReportIssue(Diagnostic.Create(analyzer.rule, declarator.GetLocation(), message));
}
};

private IEnumerable<string> FindCredentialWords(string variableName, string variableValue)
{
var credentialWordsFound = variableName
.SplitCamelCaseToWords()
.Intersect(analyzer.splitCredentialWords)
.ToHashSet(StringComparer.OrdinalIgnoreCase);

if (credentialWordsFound.Any(x => variableValue.IndexOf(x, StringComparison.InvariantCultureIgnoreCase) >= 0))
{
// See https://github.com/SonarSource/sonar-dotnet/issues/2868
return Enumerable.Empty<string>();
}

var match = analyzer.passwordValuePattern.Match(variableValue);
if (match.Success && !IsValidCredential(match.Groups["suffix"].Value))
{
credentialWordsFound.Add(match.Groups["credential"].Value);
}

// Rule was initially implemented with everything lower (which is wrong) so we have to force lower
// before reporting to avoid new issues to appear on SQ/SC.
return credentialWordsFound.Select(x => x.ToLowerInvariant());
}

private bool IsValidCredential(string suffix)
{
var candidateCredential = suffix.Split(CredentialSeparator).First().Trim();
return string.IsNullOrWhiteSpace(candidateCredential) || validCredentialPattern.IsMatch(candidateCredential);
}

private bool ContainsUriUserInfo(string variableValue)
{
var match = uriUserInfoPattern.Match(variableValue);
return match.Success
&& match.Groups["Password"].Value is { } password
&& !string.Equals(match.Groups["Login"].Value, password, StringComparison.OrdinalIgnoreCase)
&& password != CredentialSeparator.ToString()
&& !validCredentialPattern.IsMatch(password);
}
}
}
}
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/RequestsWithExcessiveLengthBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ private bool IsEnabled(AnalyzerOptions options)

private void CheckWebConfig(SonarAnalysisContext context, CompilationAnalysisContext c)
{
foreach (var fullPath in context.GetWebConfig(c))
foreach (var fullPath in context.WebConfigFiles(c))
{
var webConfig = File.ReadAllText(fullPath);
if (webConfig.Contains("<system.web") && XmlHelper.ParseXDocument(webConfig) is { } doc)
Expand Down
2 changes: 1 addition & 1 deletion analyzers/tests/SonarAnalyzer.UnitTest/Common/SecurityHotspotTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ analyzerName switch
"ConfiguringLoggers" => "ConfiguringLoggers_Log4Net",
"CookieShouldBeHttpOnly" => "CookieShouldBeHttpOnly_Nancy",
"CookieShouldBeSecure" => "CookieShouldBeSecure_Nancy",
"DoNotHardcodeCredentials" => "DoNotHardcodeCredentials_DefaultValues",
"DoNotHardcodeCredentials" => "DoNotHardcodeCredentials.DefaultValues",
"DeliveringDebugFeaturesInProduction" => "DeliveringDebugFeaturesInProduction.NetCore2",
#if NETFRAMEWORK
"ExecutingSqlQueries" => "ExecutingSqlQueries_Net46",
Expand Down

0 comments on commit 19f6fdb

Please sign in to comment.