Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Create rule S6507 with "lock on local variable" check moved from S2445 (
- Loading branch information
1 parent
38232f1
commit 229b9e4
Showing
9 changed files
with
111 additions
and
3 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
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
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
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,8 @@ | ||
=== is related to: S2445 | ||
|
||
=== on 3 Mar 2022, 10:46:00 Antonio Aversa wrote: | ||
Rule derived from the C# version of S2445, due to this branch of the rule generating a lot of FPs. | ||
|
||
Valid scenarios using local variables include retrieval of the object being locked from a collection or complex logic, to support a fine graned synchronization, renaming of a readonly field in the context of the current method or locking inside a loop, on the iteration variable. | ||
|
||
The rule still makes sense, however, for all scenarios which don't require advanced synchronization, and prevents synchronization issues captured by S2445 to be circumvented via a local variable. For example via `var local = new object(); lock (local) { ... }`. |
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,4 @@ | ||
=== Highlighting | ||
|
||
locked object in `lock (xxx)` statement | ||
|
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,4 @@ | ||
=== Message | ||
|
||
Do not lock on local variable "xxx", use a readonly field instead. | ||
|
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,3 @@ | ||
{ | ||
|
||
} |
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,56 @@ | ||
Locking on a local variable can undermine synchronization because two different threads running the same method in parallel will potentially lock on different instances of the same object, allowing them to access the synchronized block at the same time. | ||
|
||
== Noncompliant Code Example | ||
|
||
[source,csharp] | ||
---- | ||
private void DoSomething() | ||
{ | ||
object local = new object(); | ||
// Code potentially modifying the local variable ... | ||
lock (local) // Noncompliant | ||
{ | ||
// ... | ||
} | ||
} | ||
---- | ||
|
||
|
||
== Compliant Solution | ||
|
||
[source,csharp] | ||
---- | ||
private readonly object lockObj = new object(); | ||
private void DoSomething() | ||
{ | ||
lock (lockObj) | ||
{ | ||
//... | ||
} | ||
} | ||
---- | ||
|
||
== See | ||
|
||
* https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/lock[Lock Statement] - lock statement - ensure exclusive access to a shared resource | ||
* https://cwe.mitre.org/data/definitions/412[MITRE, CWE-412] - Unrestricted Externally Accessible Lock | ||
* https://cwe.mitre.org/data/definitions/413[MITRE, CWE-413] - Improper Resource Locking | ||
|
||
ifdef::env-github,rspecator-view[] | ||
|
||
''' | ||
== Implementation Specification | ||
(visible only on this page) | ||
|
||
include::message.adoc[] | ||
|
||
include::highlighting.adoc[] | ||
|
||
''' | ||
== Comments And Links | ||
(visible only on this page) | ||
|
||
include::../comments-and-links.adoc[] | ||
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,33 @@ | ||
{ | ||
"title": "Blocks should not be synchronized on local variables", | ||
"type": "BUG", | ||
"status": "ready", | ||
"remediation": { | ||
"func": "Constant\/Issue", | ||
"constantCost": "15min" | ||
}, | ||
"tags": [ | ||
"cwe", | ||
"multi-threading" | ||
], | ||
"extra": { | ||
"replacementRules": [ | ||
|
||
], | ||
"legacyKeys": [ | ||
|
||
] | ||
}, | ||
"defaultSeverity": "Major", | ||
"ruleSpecification": "RSPEC-6507", | ||
"sqKey": "S6507", | ||
"scope": "All", | ||
"securityStandards": { | ||
"CWE": [ | ||
412, | ||
413 | ||
] | ||
}, | ||
"defaultQualityProfiles": [ ], | ||
"quickfix": "unknown" | ||
} |