diff --git a/analyzers/its/expected/akka.net/Akka.Remote--netstandard2.0-S6640.json b/analyzers/its/expected/akka.net/Akka.Remote--netstandard2.0-S6640.json new file mode 100644 index 00000000000..0f7fca2f9f5 --- /dev/null +++ b/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 +} +} +} +] +} diff --git a/analyzers/rspec/cs/S6640.html b/analyzers/rspec/cs/S6640.html new file mode 100644 index 00000000000..51aa522bc66 --- /dev/null +++ b/analyzers/rspec/cs/S6640.html @@ -0,0 +1,59 @@ +

Using unsafe code blocks can lead to unintended security or stability risks.

+

unsafe 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.

+

unsafe 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. unsafe code blocks should be used with caution because +of these security and stability risks.

+

Ask Yourself Whether

+ +

There is a risk if you answered yes to the question.

+

Recommended Secure Coding Practices

+

Unless absolutely necessary, do not use unsafe code blocks. If unsafe is used to increase performance, then the Span and Memory APIs may serve a similar purpose in a safe context.

+

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 unsafe code block, make sure that:

+ +

Sensitive Code Example

+
+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 < end; i++)
+            sum += *(firstNumber + i);
+    }
+
+    return sum;
+}
+
+

Compliant Solution

+
+public int SubarraySum(int[] array, int start, int end)
+{
+    var sum = 0;
+
+    Span<int> span = array.AsSpan();
+    for (int i = start; i < end; i++)
+        sum += span[i];
+
+    return sum;
+}
+
+

See

+ + diff --git a/analyzers/rspec/cs/S6640.json b/analyzers/rspec/cs/S6640.json new file mode 100644 index 00000000000..62cb24199f1 --- /dev/null +++ b/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" +} diff --git a/analyzers/rspec/cs/Sonar_way_profile.json b/analyzers/rspec/cs/Sonar_way_profile.json index 90c47c23402..76b14b10578 100644 --- a/analyzers/rspec/cs/Sonar_way_profile.json +++ b/analyzers/rspec/cs/Sonar_way_profile.json @@ -284,6 +284,7 @@ "S6612", "S6613", "S6617", - "S6618" + "S6618", + "S6640" ] } diff --git a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxTokenListExtensions.cs b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxTokenListExtensions.cs new file mode 100644 index 00000000000..2e3b21a4a4c --- /dev/null +++ b/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; +} diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/Hotspots/UnsafeCodeBlocks.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/Hotspots/UnsafeCodeBlocks.cs new file mode 100644 index 00000000000..e8a91bfc39c --- /dev/null +++ b/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 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())); + } + } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs b/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs index 2f289418aad..86fec7766de 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/PackagingTests/RuleTypeMappingCS.cs +++ b/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"], diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/UnsafeCodeBlocksTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/Hotspots/UnsafeCodeBlocksTest.cs new file mode 100644 index 00000000000..542a076f13b --- /dev/null +++ b/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 +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/UnsafeCodeBlocks.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/Hotspots/UnsafeCodeBlocks.cs new file mode 100644 index 00000000000..2d709a9f44c --- /dev/null +++ b/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); + } +}