public static void RunThreadUnSafeICryptoTransformLambdaBad()

const int threadCount = 4;

// This local variable for a hash object is going to be shared across multiple threads

var sha1 = SHA1.Create();

var b = new Barrier(threadCount);

Action start = () => {

b.SignalAndWait();

for (int i = 0; i < 1000; i++)

{

var pwd = Guid.NewGuid().ToString();

var bytes = Encoding.UTF8.GetBytes(pwd);

// This call may fail, or return incorrect results

sha1.ComputeHash(bytes);

}

};

var threads = Enumerable.Range(0, threadCount)

.Select(_ => new ThreadStart(start))

.Select(x => new Thread(x))

.ToList();

foreach (var t in threads) t.Start();

foreach (var t in threads) t.Join();

public static void RunThreadUnSafeICryptoTransformLambdaFixed()

const int threadCount = 4;

var b = new Barrier(threadCount);

Action start = () => {

b.SignalAndWait();

for (int i = 0; i < 1000; i++)

{

// The hash object is no longer shared

var sha1 = SHA1.Create();

var pwd = Guid.NewGuid().ToString();

var bytes = Encoding.UTF8.GetBytes(pwd);

sha1.ComputeHash(bytes);

}

};

var threads = Enumerable.Range(0, threadCount)

.Select(_ => new ThreadStart(start))

.Select(x => new Thread(x))

.ToList();

foreach (var t in threads) t.Start();

foreach (var t in threads) t.Join();

<!DOCTYPE qhelp PUBLIC

"-//Semmle//qhelp//EN"

"qhelp.dtd">

<qhelp>

<overview>

<include src="ThreadUnsafeICryptoTransform.qhelp" />

</overview>

<recommendation>

<p>Create new instances of the object that implements or has a field of type <code>System.Security.Cryptography.ICryptoTransform</code> to avoid sharing it accross multiple threads.</p>

</recommendation>

<example>

<p>This example demonstrates the dangers of using a static <code>System.Security.Cryptography.ICryptoTransform</code> in a way that generates incorrect results or may raise an exception.</p>

<sample src="ThreadUnSafeICryptoTransformLambdaBad.cs" />

<p>A simple fix is to change the local variable <code>sha1</code> being captured by the lambda to be a local variable within the lambda.</p>

<sample src="ThreadUnSafeICryptoTransformLambdaGood.cs" />

</example>

<references>

<li>

Microsoft documentation, <a href="https://docs.microsoft.com/en-us/dotnet/api/system.threadstaticattribute?view=netframework-4.7.2">ThreadStaticAttribute Class</a>.

</li>

<li>

Stack Overflow, <a href="https://stackoverflow.com/questions/26592596/why-does-sha1-computehash-fail-under-high-load-with-many-threads">Why does SHA1.ComputeHash fail under high load with many threads?</a>.

</li>

</references>

</qhelp>

import csharp

import semmle.code.csharp.dataflow.DataFlow

class ICryptoTransform extends Class {

ICryptoTransform() {

this.getABaseType*().hasQualifiedName("System.Security.Cryptography", "ICryptoTransform")

}

predicate usesICryptoTransformType( Type t ) {

exists( ICryptoTransform ict |

ict = t

or usesICryptoTransformType( t.getAChild() )

or usesICryptoTransformType( t.(Class).getAMember() )

)

predicate hasICryptoTransformMember( Class c) {

exists( Field f |

f = c.getAMember()

and (

exists( ICryptoTransform ict | ict = f.getType() )

or hasICryptoTransformMember(f.getType())

or usesICryptoTransformType(f.getType())

)

)

class UsesICryptoTransform extends Class {

UsesICryptoTransform() {

usesICryptoTransformType(this) or hasICryptoTransformMember(this)

}

class NotThreadSafeCryptoUsageIntoStartingCallingConfig extends TaintTracking::Configuration {

NotThreadSafeCryptoUsageIntoStartingCallingConfig() { this = "NotThreadSafeCryptoUsageIntoStartingCallingConfig" }

override predicate isSource(DataFlow::Node source) {

exists( LambdaExpr l, LocalScopeVariable lsvar, UsesICryptoTransform ict |

l = source.asExpr() |

ict = lsvar.getType()

and lsvar.getACapturingCallable() = l

)

}

override predicate isSink(DataFlow::Node sink) {

exists( DelegateCreation dc, Expr e |

e = sink.asExpr() |

dc.getArgument() = e

and dc.getType().getName().matches("%Start")

)

}

from NotThreadSafeCryptoUsageIntoStartingCallingConfig config, LambdaExpr l, Expr e

where config.hasFlow(DataFlow::exprNode(l), DataFlow::exprNode(e))

select e, "A Lambda expression at " + l.getLocation() + " seems to be used to start a new thread is capturing a local variable that either implements 'System.Security.Cryptography.ICryptoTransform' or has a field of this type."

using System;

using System.Linq;

using System.Collections.Generic;

using System.Threading;

using System.Threading.Tasks;

using System.Security.Cryptography;

class DirectUsagePositiveCase

public static void Run(int max)

{

const int threadCount = 4;

// This variable is used in multiple threads

var sha1 = SHA1.Create();

Action start = () => {

for (int i = 0; i < max; i++)

{

var bytes = new byte[4];

sha1.ComputeHash(bytes);

}

};

// BUG expected

var threads = Enumerable.Range(0, threadCount)

.Select(_ => new ThreadStart(start))

.Select(x => new Thread(x))

.ToList();

foreach (var t in threads) t.Start();

foreach (var t in threads) t.Join();

}

class DirectUsageNegativeCase

public static void Run(int max)

{

const int threadCount = 4;

Action start = () => {

for (int i = 0; i < max; i++)

{

var sha1 = SHA1.Create();

var bytes = new byte[4];

sha1.ComputeHash(bytes);

}

};

var threads = Enumerable.Range(0, threadCount)

.Select(_ => new ThreadStart(start))

.Select(x => new Thread(x))

.ToList();

foreach (var t in threads) t.Start();

foreach (var t in threads) t.Join();

}

public class Nest01

private readonly SHA256 _sha;

public Nest01()

{

_sha = SHA256.Create();

}

public byte[] ComputeHash(byte[] bytes)

{

return _sha.ComputeHash(bytes);

}

class IndirectUsagePositiveCase

public static void Run(int max)

{

const int threadCount = 4;

// This variable is used in multiple threads

var sha1 = new Nest01();

// BUG expected

Action start = () => {

for (int i = 0; i < max; i++)

{

var bytes = new byte[4];

sha1.ComputeHash(bytes);

}

};

var threads = Enumerable.Range(0, threadCount)

.Select(_ => new ThreadStart(start))

.Select(x => new Thread(x))

.ToList();

foreach (var t in threads) t.Start();

foreach (var t in threads) t.Join();

}

class indirectUsageNegativeCase

public static void Run(int max)

{

const int threadCount = 4;

Action start = () => {

for (int i = 0; i < max; i++)

{

var sha1 = new Nest01();

var bytes = new byte[4];

sha1.ComputeHash(bytes);

}

};

var threads = Enumerable.Range(0, threadCount)

.Select(_ => new ThreadStart(start))

.Select(x => new Thread(x))

.ToList();

foreach (var t in threads) t.Start();

foreach (var t in threads) t.Join();

}

class LambdaNotStart

public static void Run()

{

var sha1 = SHA1.Create();

Func<string> myFunc = () =>

{

var bytes = new byte[4];

return Convert.ToBase64String(sha1.ComputeHash(bytes));

};

var d = myFunc.DynamicInvoke();

}

| ThreadUnsafeICryptoTransformLambda.cs:27:62:27:66 | access to local variable start | A Lambda expression at ThreadUnsafeICryptoTransformLambda.cs:17:24:23:9 seems to be used to start a new thread is capturing a local variable that either implements 'System.Security.Cryptography.ICryptoTransform' or has a field of this type. |

| ThreadUnsafeICryptoTransformLambda.cs:89:62:89:66 | access to local variable start | A Lambda expression at ThreadUnsafeICryptoTransformLambda.cs:81:24:87:9 seems to be used to start a new thread is capturing a local variable that either implements 'System.Security.Cryptography.ICryptoTransform' or has a field of this type. |

Likely Bugs/ThreadUnsafeICryptoTransformLambda.ql