New rule S6640: Allowing unsafe code is security-sensitive (#7474)

analyzers/its/expected/akka.net/Akka.Remote--netstandard2.0-S6640.json

@@ -0,0 +1,17 @@
+{
+"issues":  [
+{
+"id":  "S6640",
+"message":  "Make sure that using "unsafe" is safe here.",
+"location":  {
+"uri":  "sources\akka.net\src\core\Akka.Remote\Serialization\LruBoundedCache.cs",
+"region":  {
+"startLine":  61,
+"startColumn":  13,
+"endLine":  61,
+"endColumn":  19
+}
+}
+}
+]
+}

analyzers/rspec/cs/S6640.html

@@ -0,0 +1,59 @@
+<p>Using <code>unsafe</code> code blocks can lead to unintended security or stability risks.</p>
+<p><code>unsafe</code> code blocks allow developers to use features such as pointers, fixed buffers, function calls through pointers and manual memory
+management. Such features may be necessary for interoperability with native libraries, as these often require pointers. It may also increase
+performance in some critical areas, as certain bounds checks are not executed in an unsafe context.</p>
+<p><code>unsafe</code> code blocks aren’t necessarily dangerous, however, the contents of such blocks are not verified by the Common Language Runtime.
+Therefore, it is up to the programmer to ensure that no bugs are introduced through manual memory management or casting. If not done correctly, then
+those bugs can lead to memory corruption vulnerabilities such as stack overflows. <code>unsafe</code> code blocks should be used with caution because
+of these security and stability risks.</p>
+<h2>Ask Yourself Whether</h2>
+<ul>
+  <li> There are any pointers or fixed buffers declared within the <code>unsafe</code> code block. </li>
+</ul>
+<p>There is a risk if you answered yes to the question.</p>
+<h2>Recommended Secure Coding Practices</h2>
+<p>Unless absolutely necessary, do not use <code>unsafe</code> code blocks. If <code>unsafe</code> is used to increase performance, then the <a
+href="https://learn.microsoft.com/en-us/dotnet/standard/memory-and-spans/">Span and Memory APIs</a> may serve a similar purpose in a safe context.</p>
+<p>If it is not possible to remove the code block, then it should be kept as short as possible. Doing so reduces risk, as there is less code that can
+potentially introduce new bugs. Within the <code>unsafe</code> code block, make sure that:</p>
+<ul>
+  <li> All type casts are correct. </li>
+  <li> Memory is correctly allocated and then released. </li>
+  <li> Array accesses can never go out of bounds. </li>
+</ul>
+<h2>Sensitive Code Example</h2>
+<pre data-diff-id="1" data-diff-type="noncompliant">
+public unsafe int SubarraySum(int[] array, int start, int end)  // Sensitive
+{
+    var sum = 0;
+
+    // Skip array bound checks for extra performance
+    fixed (int* firstNumber = array)
+    {
+        for (int i = start; i &lt; end; i++)
+            sum += *(firstNumber + i);
+    }
+
+    return sum;
+}
+</pre>
+<h2>Compliant Solution</h2>
+<pre data-diff-id="1" data-diff-type="compliant">
+public int SubarraySum(int[] array, int start, int end)
+{
+    var sum = 0;
+
+    Span&lt;int&gt; span = array.AsSpan();
+    for (int i = start; i &lt; end; i++)
+        sum += span[i];
+
+    return sum;
+}
+</pre>
+<h2>See</h2>
+<ul>
+  <li> <a href="https://cwe.mitre.org/data/definitions/787.html">MITRE, CWE-787</a> - Out-of-bounds Write </li>
+  <li> <a href="https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/unsafe-code">Microsoft Learn</a> - Unsafe code, pointer types, and
+  function pointers </li>
+</ul>
+

analyzers/rspec/cs/S6640.json

@@ -0,0 +1,15 @@
+{
+  "title": "Using unsafe code blocks is security-sensitive",
+  "type": "SECURITY_HOTSPOT",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "60min"
+  },
+  "tags": [],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6640",
+  "sqKey": "S6640",
+  "scope": "All",
+  "quickfix": "infeasible"
+}

analyzers/rspec/cs/Sonar_way_profile.json

@@ -284,6 +284,7 @@
     "S6612",
     "S6613",
     "S6617",
-    "S6618"
+    "S6618",
+    "S6640"
   ]
 }

analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxTokenListExtensions.cs

@@ -0,0 +1,28 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2023 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+namespace SonarAnalyzer.Extensions;
+
+public static class SyntaxTokenListExtensions
+{
+    public static SyntaxToken? Find(this SyntaxTokenList tokenList, SyntaxKind kind) =>
+        tokenList.IndexOf(kind) is var index and >= 0
+        ? tokenList[index] : null;
+}

analyzers/src/SonarAnalyzer.CSharp/Rules/Hotspots/UnsafeCodeBlocks.cs

@@ -0,0 +1,65 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2023 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+namespace SonarAnalyzer.Rules.CSharp;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp)]
+public sealed class UnsafeCodeBlocks : HotspotDiagnosticAnalyzer
+{
+    private const string DiagnosticId = "S6640";
+    private const string MessageFormat = """Make sure that using "unsafe" is safe here.""";
+
+    private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
+
+    public UnsafeCodeBlocks() : this(AnalyzerConfiguration.Hotspot) { }
+
+    public UnsafeCodeBlocks(IAnalyzerConfiguration configuration) : base(configuration) { }
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+    protected override void Initialize(SonarAnalysisContext context)
+    {
+        context.RegisterNodeAction(c => Report(c, ((UnsafeStatementSyntax)c.Node).UnsafeKeyword), SyntaxKind.UnsafeStatement);
+        context.RegisterNodeAction(c =>
+            {
+                if (c.Node is BaseMethodDeclarationSyntax { Modifiers: var modifiers })
+                {
+                    ReportIfUnsafe(c, modifiers);
+                }
+            },
+            SyntaxKind.MethodDeclaration, SyntaxKind.ConstructorDeclaration, SyntaxKind.DestructorDeclaration, SyntaxKind.OperatorDeclaration);
+    }
+
+    private void ReportIfUnsafe(SonarSyntaxNodeReportingContext context, SyntaxTokenList modifiers)
+    {
+        if (modifiers.Find(SyntaxKind.UnsafeKeyword) is { } unsafeModifier)
+        {
+            Report(context, unsafeModifier);
+        }
+    }
+
+    private void Report(SonarSyntaxNodeReportingContext context, SyntaxToken token)
+    {
+        if (IsEnabled(context.Options))
+        {
+            context.ReportIssue(Diagnostic.Create(Rule, token.GetLocation()));
+        }
+    }
+}

analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs

@@ -6564,7 +6564,7 @@ internal static class RuleTypeMappingCS
         // ["S6637"],
         // ["S6638"],
         // ["S6639"],
-        // ["S6640"],
+        ["S6640"] = "SECURITY_HOTSPOT",
         // ["S6641"],
         // ["S6642"],
         // ["S6643"],

analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/UnsafeCodeBlocksTest.cs

@@ -0,0 +1,48 @@
+/*
+ * SonarAnalyzer for .NET
+ * Copyright (C) 2015-2023 SonarSource SA
+ * mailto: contact AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using SonarAnalyzer.Rules.CSharp;
+
+namespace SonarAnalyzer.UnitTest.Rules;
+
+[TestClass]
+public class UnsafeCodeBlocksTest
+{
+    private readonly VerifierBuilder builder = new VerifierBuilder().WithBasePath("Hotspots").AddAnalyzer(() => new UnsafeCodeBlocks(AnalyzerConfiguration.AlwaysEnabled));
+
+    [TestMethod]
+    public void UnsafeCodeBlocks() =>
+        builder.AddPaths("UnsafeCodeBlocks.cs").Verify();
+
+#if NET
+
+    [TestMethod]
+    public void UnsafeRecord() =>
+        builder.AddSnippet("""unsafe record MyRecord(byte* Pointer);        // FN""")
+        .WithOptions(ParseOptionsHelper.FromCSharp9).Verify();
+
+    [TestMethod]
+    public void UnsafeRecordStruct() =>
+        builder.AddSnippet("""unsafe record struct MyRecord(byte* Pointer); // FN""")
+        .WithOptions(ParseOptionsHelper.FromCSharp10).Verify();
+
+#endif
+}

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/UnsafeCodeBlocks.cs

@@ -0,0 +1,79 @@
+using System;
+
+public class Sample
+{
+    unsafe void MethodScope(byte* pointer) { }                      // Noncompliant {{Make sure that using "unsafe" is safe here.}}
+//  ^^^^^^
+
+    void BlockScope()
+    {
+        unsafe                                                      // Noncompliant
+//      ^^^^^^
+        {
+        }
+    }
+
+    void SafeMethod() { }                                           // Compliant
+
+    void LocalFunction()
+    {
+        unsafe void Local(byte* pointer) { }                        // FN
+        void SafeLocal(byte noPointer) { }                          // Compliant
+    }
+
+    unsafe class UnsafeClass { }                                    // FN
+
+    unsafe struct UnsafeStruct                                      // FN
+    {
+        unsafe fixed byte unsafeFixedBuffer[16];                    // FN
+    }
+
+    unsafe interface IUnsafeInterface { }                           // FN
+
+    unsafe Sample(byte* pointer) { }                                // Noncompliant
+
+    static unsafe Sample() { }                                      // Noncompliant
+
+    unsafe ~Sample() { }                                            // Noncompliant
+
+    unsafe byte* unsafeField;                                       // FN
+
+    unsafe byte* UnsafeProperty { get; }                            // FN
+
+    unsafe event EventHandler UnsafeEvent;                          // FN
+
+    unsafe delegate void UnsafeDelegate(byte* pointer);             // FN
+
+    unsafe int this[int i] => 5;                                    // FN
+
+    public unsafe static Sample operator +(Sample other) => other;  // Noncompliant
+
+    // from RSPEC
+    public unsafe int SubarraySum(int[] array, int start, int end)  // Noncompliant
+    {
+        var sum = 0;
+
+        // Skip array bound checks for extra performance
+        fixed (int* firstNumber = array)
+        {
+            for (int i = start; i < end; i++)
+                sum += *(firstNumber + i);
+        }
+
+        return sum;
+    }
+
+    // from C# docs
+    unsafe static void SquarePtrParam(int* p)                       // Noncompliant
+    {
+        *p *= *p;
+    }
+
+    unsafe static void Main()                                       // Noncompliant
+    {
+        int i = 5;
+        // Unsafe method: uses address-of operator (&).
+        SquarePtrParam(&i);
+        Console.WriteLine(i);
+    }
+}