-
Notifications
You must be signed in to change notification settings - Fork 30
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Create rule S6640: unsafe code block (C#) (#1945)
[Specification ticket](https://sonarsource.atlassian.net/browse/APPSEC-729) [AppSec PoC](SonarSource/appsec-poc#147) [Implementation ticket](SonarSource/sonar-dotnet#7290) [RSPEC Preview](https://sonarsource.github.io/rspec/#/rspec/S6640/csharp) --------- Co-authored-by: egon-okerman-sonarsource <[email protected]> Co-authored-by: Egon Okerman <[email protected]>
- Loading branch information
1 parent
03be578
commit ccb31e8
Showing
3 changed files
with
102 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
{ | ||
"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", | ||
"defaultQualityProfiles": ["Sonar way"], | ||
"quickfix": "unknown" | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
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 are any pointers or fixed buffers declared within the `unsafe` code block. | ||
|
||
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 https://learn.microsoft.com/en-us/dotnet/standard/memory-and-spans/[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: | ||
|
||
* All type casts are correct. | ||
* Memory is correctly allocated and then released. | ||
* Array accesses can never go out of bounds. | ||
|
||
|
||
== Sensitive Code Example | ||
|
||
[source,csharp,diff-id=1,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 < end; i++) | ||
sum += *(firstNumber + i); | ||
} | ||
return sum; | ||
} | ||
---- | ||
|
||
== Compliant Solution | ||
|
||
[source,csharp,diff-id=1,diff-type=compliant] | ||
---- | ||
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 | ||
|
||
* https://cwe.mitre.org/data/definitions/787.html[MITRE, CWE-787] - Out-of-bounds Write | ||
* https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/unsafe-code[Microsoft Learn] - Unsafe code, pointer types, and function pointers | ||
|
||
|
||
ifdef::env-github,rspecator-view[] | ||
|
||
''' | ||
== Implementation Specification | ||
(visible only on this page) | ||
|
||
== Message | ||
|
||
* Make sure that using an `unsafe` code block is safe here. | ||
|
||
== Highlighting | ||
|
||
Highlight the `unsafe` keyword. | ||
|
||
''' | ||
|
||
endif::env-github,rspecator-view[] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
{ | ||
} |