Updates to Populate Sarif Fields for GitHub Severity + Precision (#606)

* Update dependencies * Improve Confidence Reporting Adds Confidence Field to Issue Record Sets Confidence to either Confidence of Pattern if specified, or confidence of overall rule if specified Report Confidence and Severity in special Github sarif fields. Add Confidence values to rules * Update Guidance (#600) Fixed typo Tokens/keys in source code DES->AES Guidance * Update Changelog.md --------- Co-authored-by: Cristián Rojas <[email protected]>
microsoft · Feb 29, 2024 · b08bf6e · b08bf6e
1 parent d08607c
commit b08bf6e
Show file tree

Hide file tree

Showing 59 changed files with 200 additions and 11 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -4,6 +4,19 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.0.31] - 2024-1-28
+### Sarif Format
+Populate additional fields for GitHub Code scanning
+
+### Rules
+Populate Confidence values for rules
+
+### Dependencies
+Update Dependencies
+
+### Engine
+Prioritize confidence value from Pattern level in Issue records but fall back to rule level if not specified.
+
 ## [1.0.30] - 2024-1-31
 ### Pipeline
 Additional pipeline fixes

diff --git a/DevSkim-DotNet/Microsoft.DevSkim.CLI/Microsoft.DevSkim.CLI.csproj b/DevSkim-DotNet/Microsoft.DevSkim.CLI/Microsoft.DevSkim.CLI.csproj
@@ -37,9 +37,9 @@
     <ItemGroup>
       <PackageReference Include="CommandLineParser" Version="2.9.1" />
       <PackageReference Include="LibGit2Sharp" Version="0.29.0" />
-      <PackageReference Include="Microsoft.CST.ApplicationInspector.Logging" Version="1.9.18" />
+      <PackageReference Include="Microsoft.CST.ApplicationInspector.Logging" Version="1.9.19" />
       <PackageReference Include="Microsoft.Extensions.CommandLineUtils" Version="1.1.1" />
-      <PackageReference Include="Sarif.Sdk" Version="4.4.0" />
+      <PackageReference Include="Sarif.Sdk" Version="4.5.3" />
     </ItemGroup>
 
 </Project>
diff --git a/DevSkim-DotNet/Microsoft.DevSkim.CLI/Writers/SarifWriter.cs b/DevSkim-DotNet/Microsoft.DevSkim.CLI/Writers/SarifWriter.cs
@@ -213,13 +213,36 @@ private void AddRuleToSarifRule(DevSkimRule devskimRule)
                     Enabled = true,
                     Level = DevSkimLevelToSarifLevel(devskimRule.Severity)
                 };
+                // Set github code scanning properties
+                sarifRule.SetProperty("precision", ConfidenceToPrecision(devskimRule.Confidence));
+                sarifRule.SetProperty("problem.severity", DevSkimLevelToGitHubLevel(devskimRule.Severity));
                 sarifRule.SetProperty("DevSkimSeverity", devskimRule.Severity.ToString());
                 sarifRule.SetProperty("DevSkimConfidence", devskimRule.Confidence.ToString());
 
                 _rules.TryAdd(devskimRule.Id, sarifRule);
             }
         }
 
+        private object DevSkimLevelToGitHubLevel(Severity severity) => severity switch
+        {
+            Severity.Unspecified => string.Empty,
+            Severity.Critical => "error",
+            Severity.Important => "warning",
+            Severity.Moderate => "warning",
+            Severity.BestPractice => "recommendation",
+            Severity.ManualReview => "recommendation",
+            _ => string.Empty,
+        };
+
+        private static string ConfidenceToPrecision(Confidence confidence) => confidence switch
+        {
+            Confidence.High => "high",
+            Confidence.Medium => "medium",
+            Confidence.Low => "low",
+            Confidence.Unspecified => string.Empty,
+            _ => string.Empty
+        };
+
         private string ToSarifFriendlyName(string devskimRuleName) =>
             string.Concat(devskimRuleName.Split(' ', StringSplitOptions.RemoveEmptyEntries)
                 .Select(x => string.Concat(x.Where(char.IsLetterOrDigit)))

diff --git a/DevSkim-DotNet/Microsoft.DevSkim.Tests/Microsoft.DevSkim.Tests.csproj b/DevSkim-DotNet/Microsoft.DevSkim.Tests/Microsoft.DevSkim.Tests.csproj
@@ -10,8 +10,8 @@
 
     <ItemGroup>
         <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
-        <PackageReference Include="MSTest.TestAdapter" Version="3.2.0" />
-        <PackageReference Include="MSTest.TestFramework" Version="3.2.0" />
+        <PackageReference Include="MSTest.TestAdapter" Version="3.2.2" />
+        <PackageReference Include="MSTest.TestFramework" Version="3.2.2" />
     </ItemGroup>
 
     <ItemGroup>

diff --git a/DevSkim-DotNet/Microsoft.DevSkim.VisualStudio/Microsoft.DevSkim.VisualStudio.csproj b/DevSkim-DotNet/Microsoft.DevSkim.VisualStudio/Microsoft.DevSkim.VisualStudio.csproj
@@ -87,15 +87,15 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.8.0" />
     <PackageReference Include="Microsoft.VisualStudio.LanguageServer.Client">
-      <Version>17.8.36</Version>
+      <Version>17.9.46</Version>
     </PackageReference>
     <PackageReference Include="Microsoft.VisualStudio.LanguageServer.Protocol">
       <Version>17.2.8</Version>
     </PackageReference>
-    <PackageReference Include="Microsoft.VisualStudio.SDK" Version="17.8.37222" ExcludeAssets="runtime">
+    <PackageReference Include="Microsoft.VisualStudio.SDK" Version="17.9.37000" ExcludeAssets="runtime">
       <IncludeAssets>compile; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
-    <PackageReference Include="Microsoft.VSSDK.BuildTools" Version="17.8.2365">
+    <PackageReference Include="Microsoft.VSSDK.BuildTools" Version="17.9.3168">
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>

diff --git a/DevSkim-DotNet/Microsoft.DevSkim/DevSkimRuleProcessor.cs b/DevSkim-DotNet/Microsoft.DevSkim/DevSkimRuleProcessor.cs
@@ -41,6 +41,9 @@ public IEnumerable<Issue> Analyze(string text, string fileName)
                         StartLocation: textContainer.GetLocation(matchRecord.Boundary.Index),
                         EndLocation: textContainer.GetLocation(matchRecord.Boundary.Index + matchRecord.Boundary.Length),
                         Rule: devSkimRule);
+                        // Match record confidence is based on pattern confidence (from AI engine)
+                        // As a backup, DevSkim Rules may also have an overall confidence specified for the rule, use that when match confidence undefined
+                        issue.Confidence = matchRecord.Confidence == Confidence.Unspecified ? devSkimRule.Confidence : matchRecord.Confidence;
                         if (_processorOptions.EnableSuppressions)
                         {
                             Suppression supp = new(textContainer, issue.StartLocation.Line);

diff --git a/DevSkim-DotNet/Microsoft.DevSkim/Issue.cs b/DevSkim-DotNet/Microsoft.DevSkim/Issue.cs
@@ -41,5 +41,9 @@ public Issue(Boundary Boundary, Location StartLocation, Location EndLocation, De
         ///     Location (line, column) where issue starts
         /// </summary>
         public Location StartLocation { get; set; }
+        /// <summary>
+        ///     Confidence level of match
+        /// </summary>
+        public Confidence Confidence { get; internal set; }
     }
 }
diff --git a/DevSkim-DotNet/Microsoft.DevSkim/Microsoft.DevSkim.csproj b/DevSkim-DotNet/Microsoft.DevSkim/Microsoft.DevSkim.csproj
@@ -24,7 +24,7 @@
   </ItemGroup>
 
     <ItemGroup>
-      <PackageReference Include="Microsoft.CST.ApplicationInspector.RulesEngine" Version="1.9.18" />
+      <PackageReference Include="Microsoft.CST.ApplicationInspector.RulesEngine" Version="1.9.19" />
       <PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
     </ItemGroup>
 

diff --git a/guidance/DS106864.md b/guidance/DS106864.md
@@ -11,6 +11,12 @@ anywhere.
 In general, the [Advanced Encryption Standard](https://en.wikipedia.org/wiki/Advanced_Encryption_Standard), 
 or AES, algorithm, is preferred for all cases where symmetric encryption is needed.
 
+#### Solution
+
+##### .NET
+
+Use the following method: `System.Security.Cryptography.Aes.Create()`
+
 ### Implementation
 
 #### C# / .NET

diff --git a/guidance/DS113286.md b/guidance/DS113286.md
@@ -1,4 +1,4 @@
-## Do not include user-input directoy in format strings
+## Do not include user-input directly in format strings
 
 ### Summary
 Do not create NSString objects using a user-provided format string, as this could lead to a security vulnerability. https://www.securecoding.cert.org/confluence/display/c/FIO30-C.+Exclude+user+input+from+format+strings

diff --git a/guidance/DS117838.md b/guidance/DS117838.md
@@ -4,8 +4,21 @@
 A token or key was found in source code. If this represents a secret, it should be moved somewhere else.
 
 ### Details
-TO DO - put more details of problem and solution here
+
+Secrets in source code pose a threat to the application's components, like
+databases and other users, especially if this source code is leaked or shared.
+This applies to:
+
+* Users/passwords
+* Tokens (JWT's, etc.)
+* Hashes
+* Encryption keys
 
 ### Severity Considerations
-TO DO - put more details on the severity of the issue here.  Generally how big of a problem is this, and what makes it more or less of a problem?
+
+Follow these steps:
+
+* Change passwords/keys/secrets on the target components.
+* Store them in a secrets vault
+* Remove them from your code.
 
diff --git a/rules/default/correctness/datetime.json b/rules/default/correctness/datetime.json
@@ -14,6 +14,7 @@
         "rule_info": "",
         "patterns": [
             {
+                "confidence": "high",
                 "pattern": "(%Y-%M-%d)|(%M-%d-%Y)|(%M/%d/%Y)",
                 "type": "regex",
                 "scopes": [

diff --git a/rules/default/security/TLS/tls_appconfig.json b/rules/default/security/TLS/tls_appconfig.json
@@ -10,6 +10,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS112838.md",
         "patterns": [

diff --git a/rules/default/security/TLS/tls_appcontext.json b/rules/default/security/TLS/tls_appcontext.json
@@ -13,6 +13,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [

diff --git a/rules/default/security/TLS/tls_cobol.json b/rules/default/security/TLS/tls_cobol.json
@@ -13,6 +13,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [

diff --git a/rules/default/security/TLS/tls_functioncall.json b/rules/default/security/TLS/tls_functioncall.json
@@ -13,6 +13,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [

diff --git a/rules/default/security/TLS/tls_generic.json b/rules/default/security/TLS/tls_generic.json
@@ -15,6 +15,7 @@
         "overrides": [
             "DS440000"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440001.md",
         "patterns": [
@@ -58,6 +59,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "_comment": "Applies to all languages since many just wrap OpenSSL constructs.",
         "rule_info": "DS440001.md",
@@ -127,6 +129,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "_comment": "Applies to all languages since many just wrap OpenSSL constructs.",
         "rule_info": "DS440001.md",
@@ -156,6 +159,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "_comment": "Applies to all languages since many just wrap GnuTLS constructs.",
         "rule_info": "DS440001.md",
@@ -185,6 +189,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "_comment": "Applies to all languages since many just wrap LibreSSL constructs.",
         "rule_info": "DS440001.md",
@@ -214,6 +219,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "_comment": "Applies to all languages since many just wrap mbedTLS constructs.",
         "rule_info": "DS440001.md",
@@ -251,6 +257,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440001.md",
         "patterns": [
@@ -293,6 +300,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440001.md",
         "patterns": [
@@ -364,6 +372,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Elliptic-Curve.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440001.md",
         "patterns": [

diff --git a/rules/default/security/TLS/tls_go.json b/rules/default/security/TLS/tls_go.json
@@ -13,6 +13,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [

diff --git a/rules/default/security/TLS/tls_java.json b/rules/default/security/TLS/tls_java.json
@@ -13,6 +13,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [
@@ -55,6 +56,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [

diff --git a/rules/default/security/TLS/tls_javascript.json b/rules/default/security/TLS/tls_javascript.json
@@ -14,6 +14,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [

diff --git a/rules/default/security/TLS/tls_macos.json b/rules/default/security/TLS/tls_macos.json
@@ -12,6 +12,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "_comment": "Generic, since there are multiple languages that bind to these constants.",
         "rule_info": "DS440000.md",

diff --git a/rules/default/security/TLS/tls_python.json b/rules/default/security/TLS/tls_python.json
@@ -13,6 +13,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [

diff --git a/rules/default/security/TLS/tls_rust.json b/rules/default/security/TLS/tls_rust.json
@@ -13,6 +13,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [

diff --git a/rules/default/security/TLS/tls_securityprotocol.json b/rules/default/security/TLS/tls_securityprotocol.json
@@ -16,6 +16,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS112835.md",
         "patterns": [

diff --git a/rules/default/security/TLS/tls_sslprotocol.json b/rules/default/security/TLS/tls_sslprotocol.json
@@ -16,6 +16,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [

diff --git a/rules/default/security/TLS/tls_win32.json b/rules/default/security/TLS/tls_win32.json
@@ -15,6 +15,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [
@@ -43,6 +44,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [
@@ -71,6 +73,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [
@@ -155,6 +158,7 @@
         "tags": [
             "Cryptography.Protocol.TLS.Hard-Coded"
         ],
+        "confidence": "high",
         "severity": "ManualReview",
         "rule_info": "DS440000.md",
         "patterns": [