From 9bcd7cf53064f03f7f8918a35f362d6fb8f51261 Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Thu, 6 Oct 2022 17:19:03 +0200 Subject: [PATCH 01/12] Refactor UTs --- .../Hotspots/DoNotHardcodeCredentialsTest.cs | 66 +++++++++---------- ... DoNotHardcodeCredentials.CustomValues.cs} | 0 ... DoNotHardcodeCredentials.CustomValues.vb} | 0 ...codeCredentials.DefaultValues.CSharp10.cs} | 0 ...dcodeCredentials.DefaultValues.CSharp8.cs} | 0 ...DoNotHardcodeCredentials.DefaultValues.cs} | 0 ...DoNotHardcodeCredentials.DefaultValues.vb} | 0 7 files changed, 30 insertions(+), 36 deletions(-) rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/{DoNotHardcodeCredentials_CustomValues.cs => DoNotHardcodeCredentials.CustomValues.cs} (100%) rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/{DoNotHardcodeCredentials_CustomValues.vb => DoNotHardcodeCredentials.CustomValues.vb} (100%) rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/{DoNotHardcodeCredentials_DefaultValues.CSharp10.cs => DoNotHardcodeCredentials.DefaultValues.CSharp10.cs} (100%) rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/{DoNotHardcodeCredentials_DefaultValues.CSharp8.cs => DoNotHardcodeCredentials.DefaultValues.CSharp8.cs} (100%) rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/{DoNotHardcodeCredentials_DefaultValues.cs => DoNotHardcodeCredentials.DefaultValues.cs} (100%) rename analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/{DoNotHardcodeCredentials_DefaultValues.vb => DoNotHardcodeCredentials.DefaultValues.vb} (100%) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs index 43d4d134ab9..bc549e438a4 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs @@ -27,84 +27,78 @@ namespace SonarAnalyzer.UnitTest.Rules [TestClass] public class DoNotHardcodeCredentialsTest { - private readonly VerifierBuilder builderCS = new VerifierBuilder().AddAnalyzer(() => new CS.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled)); - private readonly VerifierBuilder builderVB = new VerifierBuilder().AddAnalyzer(() => new VB.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled)); + private readonly VerifierBuilder builderCS = CreateVerifierCS(); + private readonly VerifierBuilder builderVB = CreateVerifierVB(); [TestMethod] public void DoNotHardcodeCredentials_CS_DefaultValues() => - builderCS.AddPaths(@"Hotspots\DoNotHardcodeCredentials_DefaultValues.cs") - .AddReferences(AdditionalReferences) - .Verify(); + builderCS.AddPaths("DoNotHardcodeCredentials.DefaultValues.cs").Verify(); [TestMethod] public void DoNotHardcodeCredentials_CSharp8_DefaultValues() => - builderCS.AddPaths(@"Hotspots\DoNotHardcodeCredentials_DefaultValues.CSharp8.cs") - .AddReferences(AdditionalReferences) - .WithOptions(ParseOptionsHelper.FromCSharp8) - .Verify(); + builderCS.AddPaths("DoNotHardcodeCredentials.DefaultValues.CSharp8.cs").WithOptions(ParseOptionsHelper.FromCSharp8).Verify(); #if NET [TestMethod] public void DoNotHardcodeCredentials_CSharp10_DefaultValues() => - builderCS.AddPaths(@"Hotspots\DoNotHardcodeCredentials_DefaultValues.CSharp10.cs") - .AddReferences(AdditionalReferences) - .WithOptions(ParseOptionsHelper.FromCSharp10) - .Verify(); + builderCS.AddPaths("DoNotHardcodeCredentials.DefaultValues.CSharp10.cs").WithOptions(ParseOptionsHelper.FromCSharp10).Verify(); #endif [TestMethod] public void DoNotHardcodeCredentials_CS_CustomValues() => - new VerifierBuilder().AddAnalyzer(() => new CS.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled) { CredentialWords = @"kode,facal-faire,*,x\*+?|}{][)(^$.# " }) - .AddPaths(@"Hotspots\DoNotHardcodeCredentials_CustomValues.cs") - .AddReferences(AdditionalReferences) + CreateVerifierCS(@"kode,facal-faire,*,x\*+?|}{][)(^$.# ") + .AddPaths("DoNotHardcodeCredentials.CustomValues.cs") .Verify(); [TestMethod] public void DoNotHardcodeCredentials_CS_CustomValues_CaseInsensitive() => - new VerifierBuilder().AddAnalyzer(() => new CS.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled) { CredentialWords = @"KODE ,,,, FaCaL-FaIrE, x\*+?|}{][)(^$.# " }) - .AddPaths(@"Hotspots\DoNotHardcodeCredentials_CustomValues.cs") - .AddReferences(AdditionalReferences) + CreateVerifierCS(@"KODE ,,,, FaCaL-FaIrE, x\*+?|}{][)(^$.# ") + .AddPaths("DoNotHardcodeCredentials.CustomValues.cs") .Verify(); [TestMethod] public void DoNotHardcodeCredentials_VB_DefaultValues() => - builderVB.AddPaths(@"Hotspots\DoNotHardcodeCredentials_DefaultValues.vb") - .AddReferences(AdditionalReferences) - .Verify(); + builderVB.AddPaths("DoNotHardcodeCredentials.DefaultValues.vb").Verify(); [TestMethod] public void DoNotHardcodeCredentials_VB_CustomValues() => - new VerifierBuilder().AddAnalyzer(() => new VB.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled) { CredentialWords = @"kode,facal-faire,*,x\*+?|}{][)(^$.# " }) - .AddPaths(@"Hotspots\DoNotHardcodeCredentials_CustomValues.vb") - .AddReferences(AdditionalReferences) + CreateVerifierVB(@"kode,facal-faire,*,x\*+?|}{][)(^$.# ") + .AddPaths("DoNotHardcodeCredentials.CustomValues.vb") .Verify(); [TestMethod] public void DoNotHardcodeCredentials_VB_CustomValues_CaseInsensitive() => - new VerifierBuilder().AddAnalyzer(() => new VB.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled) { CredentialWords = @"KODE ,,,, FaCaL-FaIrE,x\*+?|}{][)(^$.# " }) - .AddPaths(@"Hotspots\DoNotHardcodeCredentials_CustomValues.vb") - .AddReferences(AdditionalReferences) + CreateVerifierVB(@"KODE ,,,, FaCaL-FaIrE,x\*+?|}{][)(^$.# ") + .AddPaths("DoNotHardcodeCredentials.CustomValues.vb") .Verify(); [TestMethod] public void DoNotHardcodeCredentials_ConfiguredCredentialsAreRead() { - var cs = new CS.DoNotHardcodeCredentials - { - CredentialWords = "Lorem, ipsum" - }; + var cs = new CS.DoNotHardcodeCredentials { CredentialWords = "Lorem, ipsum" }; cs.CredentialWords.Should().Be("Lorem, ipsum"); - var vb = new CS.DoNotHardcodeCredentials - { - CredentialWords = "Lorem, ipsum" - }; + var vb = new CS.DoNotHardcodeCredentials { CredentialWords = "Lorem, ipsum" }; vb.CredentialWords.Should().Be("Lorem, ipsum"); } internal static IEnumerable AdditionalReferences => MetadataReferenceFacade.SystemSecurityCryptography.Concat(MetadataReferenceFacade.SystemNetHttp); + + private static VerifierBuilder CreateVerifierCS(string credentialWords = null) => + new VerifierBuilder().AddAnalyzer(() => credentialWords is null + ? new CS.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled) + : new CS.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled) { CredentialWords = credentialWords }) + .WithBasePath("Hotspots") + .AddReferences(AdditionalReferences); + + private static VerifierBuilder CreateVerifierVB(string credentialWords = null) => + new VerifierBuilder().AddAnalyzer(() => credentialWords is null + ? new VB.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled) + : new VB.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled) { CredentialWords = credentialWords }) + .WithBasePath("Hotspots") + .AddReferences(AdditionalReferences); } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials_CustomValues.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.CustomValues.cs similarity index 100% rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials_CustomValues.cs rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.CustomValues.cs diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials_CustomValues.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.CustomValues.vb similarity index 100% rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials_CustomValues.vb rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.CustomValues.vb diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials_DefaultValues.CSharp10.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.CSharp10.cs similarity index 100% rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials_DefaultValues.CSharp10.cs rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.CSharp10.cs diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials_DefaultValues.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.CSharp8.cs similarity index 100% rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials_DefaultValues.CSharp8.cs rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.CSharp8.cs diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials_DefaultValues.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.cs similarity index 100% rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials_DefaultValues.cs rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.cs diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials_DefaultValues.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.vb similarity index 100% rename from analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials_DefaultValues.vb rename to analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.vb From 380dc51bf3182af8ecc7f91f7be7b8266ec427ec Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Thu, 6 Oct 2022 17:29:04 +0200 Subject: [PATCH 02/12] Prepare UTs --- .../Hotspots/DoNotHardcodeCredentialsTest.cs | 24 +++++++++++++++++++ .../DoNotHardcodeCredentials.DefaultValues.cs | 2 +- .../Corrupt/Web.config | 5 ++++ .../UnexpectedContent/Web.config | 9 +++++++ .../DoNotHardcodeCredentials/Valid/Web.config | 22 +++++++++++++++++ 5 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Corrupt/Web.config create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/UnexpectedContent/Web.config create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs index bc549e438a4..d3cc3996d6f 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs @@ -18,6 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using System.IO; using SonarAnalyzer.Common; using CS = SonarAnalyzer.Rules.CSharp; using VB = SonarAnalyzer.Rules.VisualBasic; @@ -58,6 +59,10 @@ public class DoNotHardcodeCredentialsTest .AddPaths("DoNotHardcodeCredentials.CustomValues.cs") .Verify(); + [TestMethod] + public void DoNotHardcodeCredentials_CS_WebConfig() => + DoNotHardcodeCredentials_WebConfig(AnalyzerLanguage.CSharp, new CS.DoNotHardcodeCredentials()); + [TestMethod] public void DoNotHardcodeCredentials_VB_DefaultValues() => builderVB.AddPaths("DoNotHardcodeCredentials.DefaultValues.vb").Verify(); @@ -74,6 +79,10 @@ public class DoNotHardcodeCredentialsTest .AddPaths("DoNotHardcodeCredentials.CustomValues.vb") .Verify(); + [TestMethod] + public void DoNotHardcodeCredentials_VB_WebConfig() => + DoNotHardcodeCredentials_WebConfig(AnalyzerLanguage.VisualBasic, new VB.DoNotHardcodeCredentials()); + [TestMethod] public void DoNotHardcodeCredentials_ConfiguredCredentialsAreRead() { @@ -100,5 +109,20 @@ public void DoNotHardcodeCredentials_ConfiguredCredentialsAreRead() : new VB.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled) { CredentialWords = credentialWords }) .WithBasePath("Hotspots") .AddReferences(AdditionalReferences); + + private static void DoNotHardcodeCredentials_WebConfig(AnalyzerLanguage language, DiagnosticAnalyzer analyzer) + { + var root = @"TestCases\WebConfig\DoNotHardcodeCredentials"; + var webConfigPaths = Directory.GetFiles(root, "web.config", SearchOption.AllDirectories); + webConfigPaths.Should().HaveCount(3); + var compilation = CreateCompilation(language); + foreach (var webConfigPath in webConfigPaths) + { + DiagnosticVerifier.VerifyExternalFile(compilation, analyzer, webConfigPath, TestHelper.CreateSonarProjectConfig(root, TestHelper.CreateFilesToAnalyze(root, webConfigPath))); + } + } + + private static Compilation CreateCompilation(AnalyzerLanguage language) => + SolutionBuilder.Create().AddProject(language).GetCompilation(); } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.cs index 4b8db4b0d40..fca0c5b0a8e 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.cs @@ -7,7 +7,7 @@ namespace Tests.Diagnostics { class Program { - public const string DBConnectionString = "Server=localhost; Database=Test; User=SA; Password=Secret123"; // Noncompliant + public const string DBConnectionString = "Server=localhost; Database=Test; User=SA; Password=Secret123"; // Noncompliant {{"password" detected here, make sure this is not a hard-coded credential.}} public const string EditPasswordPageUrlToken = "{Account.EditPassword.PageUrl}"; // Compliant private const string secretConst = "constantValue"; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Corrupt/Web.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Corrupt/Web.config new file mode 100644 index 00000000000..519f9ad73cb --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Corrupt/Web.config @@ -0,0 +1,5 @@ + + + + + <<< Corrupted, we don't raise here \ No newline at end of file diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/UnexpectedContent/Web.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/UnexpectedContent/Web.config new file mode 100644 index 00000000000..8c4e692b13d --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/UnexpectedContent/Web.config @@ -0,0 +1,9 @@ + + 4.0.0 + + com.mycompany.app + my-app + 1.0-SNAPSHOT + + diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config new file mode 100644 index 00000000000..c2f7aa29c67 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + From f03184d711a4100c0949b71f6ba1692b95199a7e Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Fri, 7 Oct 2022 13:10:55 +0200 Subject: [PATCH 03/12] Add more test cases --- .../DoNotHardcodeCredentials/Valid/Web.config | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config index c2f7aa29c67..af547cced71 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config @@ -12,11 +12,21 @@ + + + + + + + + Password=Secret42 + 42 + From 29451bd69fdda6562547a043d7ccd49286c610fe Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Fri, 7 Oct 2022 14:06:45 +0200 Subject: [PATCH 04/12] More tests --- .../Rules/Hotspots/DoNotHardcodeCredentialsTest.cs | 4 ++-- .../DoNotHardcodeCredentials/Valid/Web.Custom.config | 5 +++++ .../DoNotHardcodeCredentials/Valid/Web.Release.config | 9 +++++++++ .../WebConfig/DoNotHardcodeCredentials/Valid/Web.config | 2 +- 4 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Custom.config create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Release.config diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs index d3cc3996d6f..c25922968e7 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs @@ -113,8 +113,8 @@ public void DoNotHardcodeCredentials_ConfiguredCredentialsAreRead() private static void DoNotHardcodeCredentials_WebConfig(AnalyzerLanguage language, DiagnosticAnalyzer analyzer) { var root = @"TestCases\WebConfig\DoNotHardcodeCredentials"; - var webConfigPaths = Directory.GetFiles(root, "web.config", SearchOption.AllDirectories); - webConfigPaths.Should().HaveCount(3); + var webConfigPaths = Directory.GetFiles(root, "*.config", SearchOption.AllDirectories); + webConfigPaths.Should().HaveCount(5); var compilation = CreateCompilation(language); foreach (var webConfigPath in webConfigPaths) { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Custom.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Custom.config new file mode 100644 index 00000000000..cc25212d99c --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Custom.config @@ -0,0 +1,5 @@ + + + + + diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Release.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Release.config new file mode 100644 index 00000000000..c54103fc72d --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Release.config @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config index af547cced71..60b205592a3 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config @@ -25,7 +25,7 @@ - + Password=Secret42 42 From 890481bc9cd0c90b87502328839549d43798a8a3 Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Fri, 7 Oct 2022 14:31:50 +0200 Subject: [PATCH 05/12] Simple prototype --- .../Rules/DatabasePasswordsShouldBeSecure.cs | 2 +- .../Helpers/SonarAnalysisContext.cs | 2 +- .../DisablingRequestValidationBase.cs | 2 +- .../Hotspots/DoNotHardcodeCredentialsBase.cs | 25 +++++++++++++++++++ .../RequestsWithExcessiveLengthBase.cs | 2 +- .../Valid/Web.Custom.config | 2 +- .../Valid/Web.Release.config | 4 +-- .../DoNotHardcodeCredentials/Valid/Web.config | 20 +++++++++------ 8 files changed, 44 insertions(+), 15 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/DatabasePasswordsShouldBeSecure.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/DatabasePasswordsShouldBeSecure.cs index 7eaa1166705..4324b21b85e 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/DatabasePasswordsShouldBeSecure.cs +++ b/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("") && XmlHelper.ParseXDocument(webConfig) is { } doc) diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/SonarAnalysisContext.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/SonarAnalysisContext.cs index cac20f366a3..a530a883a5c 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/SonarAnalysisContext.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/SonarAnalysisContext.cs @@ -140,7 +140,7 @@ internal void RegisterSyntaxNodeAction(Action RegisterContextAction(x => context.RegisterSyntaxNodeAction(x, syntaxKinds), action, c => c.GetSyntaxTree(), c => c.Compilation, c => c.Options); - internal IEnumerable GetWebConfig(CompilationAnalysisContext c) + internal IEnumerable WebConfigFiles(CompilationAnalysisContext c) { return ProjectConfiguration(c.Options).FilesToAnalyze.FindFiles(WebConfigRegex).Where(ShouldProcess); diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DisablingRequestValidationBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DisablingRequestValidationBase.cs index 749a31c8a0b..7a2b1bb2c28 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DisablingRequestValidationBase.cs +++ b/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("") && XmlHelper.ParseXDocument(webConfig) is { } doc) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs index 538f9df1d71..78b3a8b9662 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs +++ b/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; @@ -98,6 +100,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,6 +109,28 @@ 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().Attributes()); + } + } + } + + private void CheckWebConfig(CompilationAnalysisContext c, string path, IEnumerable attributes) + { + foreach (var attribute in attributes) + { + if (attribute.Value.ToLower().Contains("password") && attribute.CreateLocation(path) is { } location) + { + c.ReportIssue(Diagnostic.Create(rule, location, string.Format(MessageFormatCredential, "password"))); // FIXME: Vyresit dynamicky + } + } + } + protected abstract class CredentialWordsFinderBase where TSyntaxNode : SyntaxNode { diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/RequestsWithExcessiveLengthBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/RequestsWithExcessiveLengthBase.cs index b85d76917f3..04fdbfe28bd 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/RequestsWithExcessiveLengthBase.cs +++ b/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(" - + diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Release.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Release.config index c54103fc72d..0b46d4f0f61 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Release.config +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Release.config @@ -1,9 +1,9 @@ - + - + diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config index 60b205592a3..8b0805ab431 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config @@ -3,29 +3,33 @@ - + + + "Server=localhost; Database=Test; User=SA; Password=Secret123" /> - - - + + + - + - + - + Password=Secret42 42 From ac2f226552db2529164a959f739b87e2257419a2 Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Fri, 7 Oct 2022 14:57:50 +0200 Subject: [PATCH 06/12] Extract and reuse logic --- .../Hotspots/DoNotHardcodeCredentialsBase.cs | 138 +++++++++--------- .../DoNotHardcodeCredentials/Valid/Web.config | 2 +- 2 files changed, 74 insertions(+), 66 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs index 78b3a8b9662..1827b91b8b0 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs @@ -45,10 +45,13 @@ public abstract class DoNotHardcodeCredentialsBase : ParameterLoadi private readonly IAnalyzerConfiguration configuration; private readonly DiagnosticDescriptor rule; + private readonly Regex validCredentialPattern = new(@"^\?|:\w+|\{\d+[^}]*\}|""|'$", RegexOptions.Compiled | RegexOptions.IgnoreCase); + private readonly Regex uriUserInfoPattern; private string credentialWords; private IEnumerable splitCredentialWords; private Regex passwordValuePattern; + protected abstract ILanguageFacade Language { get; } protected abstract void InitializeActions(ParameterLoadingAnalysisContext context); @@ -73,6 +76,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+:\/\/(?" + uriUserInfoPart + "):(?" + uriUserInfoPart + ")@", RegexOptions.Compiled); this.configuration = configuration; rule = Language.CreateDescriptor(DiagnosticId, MessageFormat); CredentialWords = DefaultCredentialWords; // Property will initialize multiple state variables @@ -124,36 +134,81 @@ private void CheckWebConfig(CompilationAnalysisContext c, string path, IEnumerab { foreach (var attribute in attributes) { - if (attribute.Value.ToLower().Contains("password") && attribute.CreateLocation(path) is { } location) + if (IssueMessage(attribute.Name.LocalName, attribute.Value) is { } message && attribute.CreateLocation(path) is { } location) { - c.ReportIssue(Diagnostic.Create(rule, location, string.Format(MessageFormatCredential, "password"))); // FIXME: Vyresit dynamicky + c.ReportIssue(Diagnostic.Create(rule, location, message)); } } } + 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 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(); + } + + 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 where TSyntaxNode : SyntaxNode { - private readonly Regex validCredentialPattern = new(@"^\?|:\w+|\{\d+[^}]*\}|""|'$", RegexOptions.Compiled | RegexOptions.IgnoreCase); - private readonly Regex uriUserInfoPattern; private readonly DoNotHardcodeCredentialsBase 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 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 analyzer) => this.analyzer = analyzer; - uriUserInfoPattern = new Regex(@"\w+:\/\/(?" + uriUserInfoPart + "):(?" + uriUserInfoPart + ")@", RegexOptions.Compiled); - } public Action AnalysisAction() => context => @@ -161,59 +216,12 @@ protected CredentialWordsFinderBase(DoNotHardcodeCredentialsBase an 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 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(); - } - - 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); - } } } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config index 8b0805ab431..5f3586fdaaa 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config @@ -16,7 +16,7 @@ --> - + From 44cdf2aa5cd954be427bafe0964e5c829e962829 Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Fri, 7 Oct 2022 15:00:22 +0200 Subject: [PATCH 07/12] Add URI test case --- .../WebConfig/DoNotHardcodeCredentials/Valid/Web.config | 1 + 1 file changed, 1 insertion(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config index 5f3586fdaaa..308afb68413 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config @@ -22,6 +22,7 @@ + From 9da0fc2b8e49b079c500facdd4b2c6c728178d8c Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Fri, 7 Oct 2022 15:13:16 +0200 Subject: [PATCH 08/12] Support XElement --- .../SonarAnalyzer.Common/Helpers/XmlHelper.cs | 16 ++++++++++++---- .../Hotspots/DoNotHardcodeCredentialsBase.cs | 17 ++++++++++++----- .../DoNotHardcodeCredentials/Valid/Web.config | 13 +++++++++++-- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/XmlHelper.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/XmlHelper.cs index 3a0ee4945d1..e3e5bd6c92a 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/XmlHelper.cs +++ b/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; + } } } } diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs index 1827b91b8b0..f69e13cd154 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs @@ -125,18 +125,25 @@ private void CheckWebConfig(SonarAnalysisContext context, CompilationAnalysisCon { if (XmlHelper.ParseXDocument(File.ReadAllText(path)) is { } doc) { - CheckWebConfig(c, path, doc.Descendants().Attributes()); + CheckWebConfig(c, path, doc.Descendants()); } } } - private void CheckWebConfig(CompilationAnalysisContext c, string path, IEnumerable attributes) + private void CheckWebConfig(CompilationAnalysisContext c, string path, IEnumerable elements) { - foreach (var attribute in attributes) + foreach (var element in elements) { - if (IssueMessage(attribute.Name.LocalName, attribute.Value) is { } message && attribute.CreateLocation(path) is { } location) + if (!element.HasElements && IssueMessage(element.Name.LocalName, element.Value) is { } elementMessage && element.CreateLocation(path) is { } elementLocation) { - c.ReportIssue(Diagnostic.Create(rule, location, message)); + 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)); + } } } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config index 308afb68413..00258484ff7 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config @@ -31,7 +31,16 @@ - Password=Secret42 - 42 + Password=Secret42 + 42 + 42 + + 42 + + + + 42 + + From 3468dda3723fc2936da00351fbd269b4df6706a3 Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Fri, 7 Oct 2022 16:02:04 +0200 Subject: [PATCH 09/12] Fix SecurityHotspotTest --- .../tests/SonarAnalyzer.UnitTest/Common/SecurityHotspotTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Common/SecurityHotspotTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Common/SecurityHotspotTest.cs index 239df4d805f..477463eae78 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Common/SecurityHotspotTest.cs +++ b/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", From 61f63999d623eade8c3d7affbb4333b0d55a038a Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Fri, 7 Oct 2022 16:27:00 +0200 Subject: [PATCH 10/12] Add UT for debug files --- .../Rules/Hotspots/DoNotHardcodeCredentialsTest.cs | 2 +- .../DoNotHardcodeCredentials/Valid/Web.Debug.config | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Debug.config diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs index c25922968e7..5b3470afb82 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs @@ -114,7 +114,7 @@ private static void DoNotHardcodeCredentials_WebConfig(AnalyzerLanguage language { var root = @"TestCases\WebConfig\DoNotHardcodeCredentials"; var webConfigPaths = Directory.GetFiles(root, "*.config", SearchOption.AllDirectories); - webConfigPaths.Should().HaveCount(5); + webConfigPaths.Should().HaveCount(6); var compilation = CreateCompilation(language); foreach (var webConfigPath in webConfigPaths) { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Debug.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Debug.config new file mode 100644 index 00000000000..0c54bf651c4 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.Debug.config @@ -0,0 +1,6 @@ + + + + + + From 67b66c14119119721dfbdd3437c417d68720357b Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Mon, 10 Oct 2022 16:02:26 +0200 Subject: [PATCH 11/12] Review update --- .../Rules/Hotspots/DoNotHardcodeCredentialsBase.cs | 6 ++---- .../DoNotHardcodeCredentials.DefaultValues.cs | 13 ++++++++----- .../DoNotHardcodeCredentials.DefaultValues.vb | 2 +- .../DoNotHardcodeCredentials/Valid/Web.config | 10 ++++++++++ 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs index f69e13cd154..5d51c820c4f 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/Hotspots/DoNotHardcodeCredentialsBase.cs @@ -45,13 +45,12 @@ public abstract class DoNotHardcodeCredentialsBase : ParameterLoadi private readonly IAnalyzerConfiguration configuration; private readonly DiagnosticDescriptor rule; - private readonly Regex validCredentialPattern = new(@"^\?|:\w+|\{\d+[^}]*\}|""|'$", RegexOptions.Compiled | RegexOptions.IgnoreCase); + private readonly Regex validCredentialPattern = new(@"^(\?|:\w+|\{\d+[^}]*\}|""|')$", RegexOptions.IgnoreCase); private readonly Regex uriUserInfoPattern; private string credentialWords; private IEnumerable splitCredentialWords; private Regex passwordValuePattern; - protected abstract ILanguageFacade Language { get; } protected abstract void InitializeActions(ParameterLoadingAnalysisContext context); @@ -184,8 +183,7 @@ private IEnumerable FindCredentialWords(string variableName, string vari 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. + // 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()); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.cs index fca0c5b0a8e..8ebd34c28b7 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.cs @@ -84,8 +84,11 @@ public void DefaultKeywords() public void Constants() { - const string ConnectionString = "Server=localhost; Database=Test; User=SA; Password=Secret123"; // Noncompliant - const string ConnectionStringWithSpaces = "Server=localhost; Database=Test; User=SA; Password = Secret123"; // Noncompliant + const string ConnectionString = "Server=localhost; Database=Test; User=SA; Password=Secret123"; // Noncompliant + const string ConnectionStringWithSpaces = "Server=localhost; Database=Test; User=SA; Password = Secret123"; // Noncompliant + const string inTheMiddle = "Server=localhost; Database=Test; User=SA; Password=Secret123; Application Name=Sonar"; // Noncompliant + const string withSemicolon = @"Server=localhost; Database=Test; User=SA; Password=""Secret;'123"""; // Noncompliant + const string withApostroph = @"Server=localhost; Database=Test; User=SA; Password='Secret""123"; // Noncompliant const string Password = "Secret123"; // Noncompliant const string LoginName = "Admin"; @@ -113,8 +116,8 @@ public void Concatenations(string arg) a = "Server = localhost; Database = Test; User = SA; Password = " + secretVariableMethod; // Compliant, not initialized to constant a = "Server = localhost; Database = Test; User = SA; Password = " + arg; // Compliant, not initialized to constant - const string passwordPrefixConst = "Password = "; // Compliant by it's name - var passwordPrefixVariable = "Password = "; // Compliant by it's name + const string passwordPrefixConst = "Password = "; // Compliant by its name + var passwordPrefixVariable = "Password = "; // Compliant by its name a = "Server = localhost;" + " Database = Test; User = SA; Password = " + secretConst; // Noncompliant a = "Server = localhost;" + " Database = Test; User = SA; Pa" + "ssword = " + secretConst; // FN, we don't track all concatenations to avoid duplications a = "Server = localhost;" + " Database = Test; User = SA; " + passwordPrefixConst + secretConst; // Noncompliant @@ -386,7 +389,7 @@ public void Foo(string user) this.password = "foo"; // False Negative Configuration.Password = "foo"; // False Negative this.password = Configuration.Password = "foo"; // False Negative - string query1 = "password=':crazy;secret';user=xxx"; // False Negative - passwords enclosed in '' are not covered + string query1 = "password=':crazy;secret';user=xxx"; // Noncompliant } class Configuration diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.vb index b1ae61d9167..c0eb80023d6 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.vb +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/DoNotHardcodeCredentials.DefaultValues.vb @@ -364,7 +364,7 @@ Phone: +0000000" Me.password = "foo" ' False Negative Configuration.Password = "foo" ' False Negative Me.password = Configuration.Password = "foo" ' False Negative - Dim query1 As String = "password=':crazy;secret';user=xxx" ' False Negative - passwords enclosed in '' are not covered + Dim query1 As String = "password=':crazy;secret';user=xxx" ' Noncompliant End Sub Class Configuration diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config index 00258484ff7..cfcc7697f2a 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config @@ -18,6 +18,16 @@ + + From 5d2573feeb2a65e892b14232a5530e6fb7db492b Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Tue, 11 Oct 2022 17:08:08 +0200 Subject: [PATCH 12/12] Add single line UT --- .../WebConfig/DoNotHardcodeCredentials/Valid/Web.config | 1 + 1 file changed, 1 insertion(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config index cfcc7697f2a..aac186c0121 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/WebConfig/DoNotHardcodeCredentials/Valid/Web.config @@ -28,6 +28,7 @@ Password=Secret123; MultipleActiveResultSets=True"" providerName="System.Data.EntityClient"/> +