Rule S2068: detect hard-coded passwords in web.config files

analyzers/src/SonarAnalyzer.CSharp/Rules/DatabasePasswordsShouldBeSecure.cs

@@ -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)

analyzers/src/SonarAnalyzer.Common/Helpers/SonarAnalysisContext.cs

@@ -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);
 

analyzers/src/SonarAnalyzer.Common/Helpers/XmlHelper.cs

@@ -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;
+            }
         }
     }
 }

analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DisablingRequestValidationBase.cs

@@ -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)

analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs

@@ -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;
@@ -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;
@@ -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
@@ -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)
@@ -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);
-            }
         }
     }
 }

analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/RequestsWithExcessiveLengthBase.cs

@@ -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)

analyzers/tests/SonarAnalyzer.UnitTest/Common/SecurityHotspotTest.cs

@@ -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",