Making changes based on feedback.

This commit is contained in:
Raul Garcia
2019-02-22 10:10:20 -08:00
parent 7d197692ac
commit 9bb7816a3c
6 changed files with 10 additions and 19 deletions

View File

@@ -16,10 +16,10 @@
</recommendation>
<example>
<p>This example demonstrates the dangers of using a static <code>System.Security.Cryptography.ICryptoTransform</code> in such a way that the results may be incorrect.</p>
<sample src="ThreadUnsafeICryptoTransform.cs" />
<sample src="ThreadUnSafeICryptoTransformBad.cs" />
<p>A simple fix is to change the <code>_sha</code> field from being a static member to an instance one by removing the <code>static</code> keyword.</p>
<sample src="ThreadUnSafeICryptoTransformFix.cs" />
<sample src="ThreadUnSafeICryptoTransformGood.cs" />
</example>
<references>

View File

@@ -23,13 +23,13 @@ class ICryptoTransform extends Class {
predicate usesICryptoTransformType( Type t ) {
exists( ICryptoTransform ict |
ict = t
or usesICryptoTransformType( t.getAChild*() )
or usesICryptoTransformType( t.getAChild() )
)
}
predicate hasICryptoTransformMember( Class c) {
exists( Field f |
f = c.getAMember*()
f = c.getAMember()
and (
exists( ICryptoTransform ict | ict = f.getType() )
or hasICryptoTransformMember(f.getType())
@@ -75,14 +75,6 @@ predicate hasICryptoTransformStaticMember( Class c, string msg) {
)
)
)
or
exists( Field f |
f = c.getAMember*()
and not f.isStatic()
and ( hasICryptoTransformStaticMember( f.getType(), _ )
and msg = "Non-static field " + f + " of type " + f.getType() + " internally makes use of an static object that implements 'System.Security.Cryptography.ICryptoTransform'. This causes that usage of this class member is unsafe for concurrent threads."
)
)
or ( hasICryptoTransformStaticMemberNested(c)
and msg = "Class" + c + " implementation depends on a static object of type 'System.Security.Cryptography.ICryptoTransform' in a way that is unsafe for concurrent threads."
)

View File

@@ -66,11 +66,6 @@ public class StaticMember
private static SHA1 _sha1 = SHA1.Create();
}
public class IndirectStatic
{
StaticMember tc;
}
public class IndirectStatic2
{
static Nest02 _n = new Nest02();
@@ -80,6 +75,11 @@ public class IndirectStatic2
/// Should not be flagged (thread safe)
/// </summary>
public class IndirectStatic
{
StaticMember tc;
}
public class TokenCacheFP
{
/// <summary>

View File

@@ -2,5 +2,4 @@
| ThreadUnsafeICryptoTransform.cs:44:14:44:19 | Nest04 | ClassNest04 implementation depends on a static object of type 'System.Security.Cryptography.ICryptoTransform' in a way that is unsafe for concurrent threads. |
| ThreadUnsafeICryptoTransform.cs:49:21:49:42 | StaticMemberChildUsage | Static field HashMap of type Dictionary<DigestAlgorithm,HashAlgorithm> makes usage of 'System.Security.Cryptography.ICryptoTransform', but it does not have an attribute [ThreadStatic]. The usage of this class is unsafe for concurrent threads. |
| ThreadUnsafeICryptoTransform.cs:64:14:64:25 | StaticMember | Static field _sha1 of type SHA1, implements 'System.Security.Cryptography.ICryptoTransform', but it does not have an attribute [ThreadStatic]. The usage of this class is unsafe for concurrent threads. |
| ThreadUnsafeICryptoTransform.cs:69:14:69:27 | IndirectStatic | Non-static field tc of type StaticMember internally makes use of an static object that implements 'System.Security.Cryptography.ICryptoTransform'. This causes that usage of this class member is unsafe for concurrent threads. |
| ThreadUnsafeICryptoTransform.cs:74:14:74:28 | IndirectStatic2 | ClassIndirectStatic2 implementation depends on a static object of type 'System.Security.Cryptography.ICryptoTransform' in a way that is unsafe for concurrent threads. |
| ThreadUnsafeICryptoTransform.cs:69:14:69:28 | IndirectStatic2 | ClassIndirectStatic2 implementation depends on a static object of type 'System.Security.Cryptography.ICryptoTransform' in a way that is unsafe for concurrent threads. |