diff --git a/csharp/CWE-326/EncryptionInsufficientKeySize.ql b/csharp/CWE-326/EncryptionInsufficientKeySize.ql new file mode 100644 index 0000000000..fc6b4b8b24 --- /dev/null +++ b/csharp/CWE-326/EncryptionInsufficientKeySize.ql @@ -0,0 +1,20 @@ +/** + * @name Weak encryption: Insufficient key size + * @description Finds uses of encryption algorithms with too small a key size + * @kind problem + * @problem.severity warning + * @security-severity 7.5 + * @precision high + * @id cs/insufficient-key-size + * @tags security + * external/cwe/cwe-326 + */ + +import csharp +import github.crypto + +from Crypto::AsymmetricAlgorithms aglms, int key_size +where + key_size = aglms.getKeySize() and + key_size < aglms.minKeySize() +select aglms, "Key size " + key_size.toString() + " is to small (min: " + aglms.minKeySize() + ")" diff --git a/csharp/CWE-326/EncryptionKeySizeToLarge.ql b/csharp/CWE-326/EncryptionKeySizeToLarge.ql new file mode 100644 index 0000000000..dffe0e89ae --- /dev/null +++ b/csharp/CWE-326/EncryptionKeySizeToLarge.ql @@ -0,0 +1,21 @@ +/** + * @name Weak encryption: Key size is too large for algorithm + * @description Finds uses of encryption algorithms with too large of a key size + * @kind problem + * @problem.severity warning + * @security-severity 1.0 + * @precision high + * @id cs/insufficient-key-size + * @tags security + * external/cwe/cwe-326 + */ + +import csharp +import github.crypto + +from Crypto::AsymmetricAlgorithms aglms, int key_size +where + key_size = aglms.getKeySize() and + key_size > aglms.maxKeySize() +select aglms, + "Key size " + key_size.toString() + " is to large for algorithm (max: " + aglms.maxKeySize() + ")" diff --git a/csharp/CWE-326/EncryptionKeySizeUseGetter.ql b/csharp/CWE-326/EncryptionKeySizeUseGetter.ql new file mode 100644 index 0000000000..9a9f9402a2 --- /dev/null +++ b/csharp/CWE-326/EncryptionKeySizeUseGetter.ql @@ -0,0 +1,22 @@ +/** + * @name Weak encryption: Key size set using a Getter and failing to set key size + * @description Finds uses of encryption algorithms using a Getter and not setting the size of the key + * @kind problem + * @problem.severity warning + * @security-severity 1.0 + * @precision high + * @id cs/encryption-keysize-use-getter + * @tags security + * external/cwe/cwe-326 + */ + +import csharp +import github.crypto + +from Crypto::AsymmetricAlgorithms aglms, PropertyAccess props, Expr expr +where + aglms.getType().hasName(["DSACryptoServiceProvider", "RSACryptoServiceProvider"]) and + props.getTarget().getDeclaringType() = aglms.getType() and + props.getTarget().getName() = "KeySize" and + expr = props.getParent().(Assignment).getRValue() +select expr, "Cannot use Getter to set key size" diff --git a/csharp/CWE-327/WeakHMac.ql b/csharp/CWE-327/WeakHMac.ql new file mode 100644 index 0000000000..b0fee4dd82 --- /dev/null +++ b/csharp/CWE-327/WeakHMac.ql @@ -0,0 +1,18 @@ +/** + * @name Use of Weak Hmac Algorithms + * @description Use of Weak Hmac Algorithms + * @kind problem + * @problem.severity warning + * @security-severity 4.0 + * @precision medium + * @id cs/weak-hmac-algorithm + * @tags security + * external/cwe/cwe-327 + */ + +import csharp +import github.crypto + +from Crypto::HMacSigningAlgorithms algorithms +where algorithms.algorithm() = ["MD5", "SHA1"] +select algorithms, "Weak Hmac Algorithms" diff --git a/csharp/CWE-760/HardcodedSalt.ql b/csharp/CWE-760/HardcodedSalt.ql new file mode 100644 index 0000000000..5ed6f5bb36 --- /dev/null +++ b/csharp/CWE-760/HardcodedSalt.ql @@ -0,0 +1,72 @@ +/** + * @name Hardcoded Salt + * @description Hardcoded Salt + * @kind path-problem + * @problem.severity error + * @security-severity 6.1 + * @precision medium + * @id cs/hardcoded-salt + * @tags security + * external/cwe/cwe-760 + */ + +import csharp +private import semmle.code.csharp.frameworks.Moq +private import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph +// import semmle.code.csharp.frameworks.system.security.Cryptography +private import github.hardcoded +private import github.crypto + +module HardcodedSalt { + abstract class Source extends DataFlow::ExprNode { } + + abstract class Sink extends DataFlow::ExprNode { } + + abstract class Sanitizer extends DataFlow::ExprNode { } + + abstract class SanitizerGuard extends DataFlow::BarrierGuard { } + + /* + * Sources + */ + + class Hardcoded extends Source { + Hardcoded() { this instanceof HardcodedValues } + } + + /* + * Sinks + */ + + class HashAlgSalts extends Sink { + HashAlgSalts() { exists(Crypto::HashingAlgorithms hash | this = hash.getSalt()) } + } + + /* + * Config + */ + + class TaintTrackingConfiguration extends TaintTracking::Configuration { + TaintTrackingConfiguration() { this = "HardcodedSalt" } + + override predicate isSource(DataFlow::Node source) { source instanceof HardcodedSalt::Source } + + override predicate isSink(DataFlow::Node sink) { + sink instanceof HardcodedSalt::Sink and + not any(ReturnedByMockObject mock).getAMemberInitializationValue() = sink.asExpr() and + not any(ReturnedByMockObject mock).getAnArgument() = sink.asExpr() + } + + override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer } + + override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { + guard instanceof SanitizerGuard + } + } +} + +from + HardcodedSalt::TaintTrackingConfiguration config, DataFlow::PathNode source, + DataFlow::PathNode sink +where config.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "Use of $@.", source.getNode(), "hardcoded salt" diff --git a/csharp/CWE-916/WeakIterations.ql b/csharp/CWE-916/WeakIterations.ql new file mode 100644 index 0000000000..7e19777fa4 --- /dev/null +++ b/csharp/CWE-916/WeakIterations.ql @@ -0,0 +1,83 @@ +/** + * @name Use of Password Hash With Insufficient Computational Effort + * @description Use of Password Hash With Insufficient Computational Effort + * @kind path-problem + * @problem.severity warning + * @security-severity 4.0 + * @precision medium + * @id cs/weak-hash-algorithms-iterations + * @tags security + * external/cwe/cwe-916 + */ + +import csharp +private import semmle.code.csharp.frameworks.Moq +private import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph +// import semmle.code.csharp.frameworks.system.security.Cryptography +private import github.hardcoded +private import github.crypto + +module WeakIterations { + abstract class Source extends DataFlow::ExprNode { } + + abstract class Sink extends DataFlow::ExprNode { } + + abstract class Sanitizer extends DataFlow::ExprNode { } + + abstract class SanitizerGuard extends DataFlow::BarrierGuard { } + + /* + * Sources + */ + + class Hardcoded extends Source { + Hardcoded() { this.getExpr().(IntLiteral).getValue().toInt() < 100000 } + } + + class DefaultSettings extends Source { + // TODO: This needs to be FAR better + DefaultSettings() { + exists(Crypto::HashingAlgorithms hash | + hash.getExpr().(ObjectCreation).getNumberOfArguments() <= 2 and + hash.defaultIterations() < 10000 and + this.asExpr() = hash.getExpr() + ) + } + } + + /* + * Sinks + */ + + class HashAlgSalts extends Sink { + HashAlgSalts() { exists(Crypto::HashingAlgorithms hash | this = hash.getIterations()) } + } + + /* + * Config + */ + + class TaintTrackingConfiguration extends TaintTracking::Configuration { + TaintTrackingConfiguration() { this = "WeakIteration" } + + override predicate isSource(DataFlow::Node source) { source instanceof WeakIterations::Source } + + override predicate isSink(DataFlow::Node sink) { + sink instanceof WeakIterations::Sink and + not any(ReturnedByMockObject mock).getAMemberInitializationValue() = sink.asExpr() and + not any(ReturnedByMockObject mock).getAnArgument() = sink.asExpr() + } + + override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer } + + override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { + guard instanceof SanitizerGuard + } + } +} + +from + WeakIterations::TaintTrackingConfiguration config, DataFlow::PathNode source, + DataFlow::PathNode sink +where config.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "Use of $@.", source.getNode(), "hardcoded weak iterations" diff --git a/csharp/github/crypto.qll b/csharp/github/crypto.qll new file mode 100644 index 0000000000..5fb4fb5c5f --- /dev/null +++ b/csharp/github/crypto.qll @@ -0,0 +1,169 @@ +import csharp + +module Crypto { + class HashingAlgorithm extends DataFlow::ExprNode { + abstract DataFlow::ExprNode getHashValue(); + + abstract DataFlow::ExprNode getSalt(); + + abstract int defaultIterations(); + + abstract DataFlow::ExprNode getIterations(); + } + + class HMacSigningAlgorithm extends DataFlow::ExprNode { + abstract string algorithm(); + + abstract DataFlow::ExprNode key(); + } + + class AsymmetricAlgorithm extends DataFlow::ExprNode { + abstract int maxKeySize(); + + abstract int minKeySize(); + + abstract int getKeySize(); + } + + // Abstraction classes + abstract class HashingAlgorithms extends HashingAlgorithm { } + + abstract class HMacSigningAlgorithms extends HMacSigningAlgorithm { } + + abstract class AsymmetricAlgorithms extends AsymmetricAlgorithm { } + + // Content + class CryptoRfc2898DeriveBytes extends HashingAlgorithms { + CryptoRfc2898DeriveBytes() { + exists(ObjectCreation object | + object.getType().getQualifiedName() = "System.Security.Cryptography.Rfc2898DeriveBytes" and + this.asExpr() = object + ) + } + + override DataFlow::ExprNode getHashValue() { + result.asExpr() = this.asExpr().(ObjectCreation).getArgument(0) + } + + override DataFlow::ExprNode getSalt() { + result.asExpr() = this.asExpr().(ObjectCreation).getArgument(1) + } + + override int defaultIterations() { result = 1000 } + + override DataFlow::ExprNode getIterations() { + result.asExpr() = this.asExpr().(ObjectCreation).getArgument(2) + or + // TODO: It this the best way? We need a better way of determinding + // iterations isn't set. + this.getExpr().(ObjectCreation).getNumberOfArguments() <= 2 and + this.defaultIterations() < 100000 and + result.asExpr() = this.getExpr() + } + } + + class DSACryptoServiceProvider extends AsymmetricAlgorithms { + DSACryptoServiceProvider() { + exists(ObjectCreation object | + object + .getType() + .hasQualifiedName("System.Security.Cryptography", "DSACryptoServiceProvider") and + this.asExpr() = object + ) + } + + override int maxKeySize() { result = 1024 } + + override int minKeySize() { result = 1024 } + + override int getKeySize() { + this.asExpr().(ObjectCreation).hasNoArguments() and + result = 1024 + or + // https://docs.microsoft.com/en-us/dotnet/api/system.security.cryptography.dsacryptoserviceprovider.-ctor?view=net-6.0#system-security-cryptography-dsacryptoserviceprovider-ctor(system-int32) + result = this.asExpr().(ObjectCreation).getArgument(0).getValue().toInt() + } + } + + class RC2CryptoServiceProvider extends AsymmetricAlgorithms { + RC2CryptoServiceProvider() { + exists(ObjectCreation object | + object + .getType() + .hasQualifiedName("System.Security.Cryptography", "RC2CryptoServiceProvider") and + this.asExpr() = object + ) + } + + override int maxKeySize() { result = 128 } + + override int minKeySize() { result = 128 } + + override int getKeySize() { + this.asExpr().(ObjectCreation).hasNoArguments() and + result = 128 // default + or + // https://docs.microsoft.com/en-us/dotnet/api/system.security.cryptography.dsacryptoserviceprovider.-ctor?view=net-6.0#system-security-cryptography-dsacryptoserviceprovider-ctor(system-int32) + result = this.asExpr().(ObjectCreation).getArgument(0).getValue().toInt() + } + } + + class RSA extends AsymmetricAlgorithms { + RSA() { + exists(ObjectCreation object | + object + .getType() + .hasQualifiedName("System.Security.Cryptography", ["RSACryptoServiceProvider", "RSACng"]) and + this.asExpr() = object + ) + or + exists(MethodCall call | + call.getType().hasQualifiedName("System.Security.Cryptography", ["RSA"]) and + call.getTarget().hasName("Create") and + this.asExpr() = call + ) + } + + override int maxKeySize() { result = 2048 } + + override int minKeySize() { result = 2048 } + + override int getKeySize() { + ( + // https://docs.microsoft.com/en-us/dotnet/api/system.security.cryptography.dsacryptoserviceprovider.-ctor?view=net-6.0#system-security-cryptography-dsacryptoserviceprovider-ctor(system-int32) + this.asExpr().(ObjectCreation).hasNoArguments() and + result = 1024 + or + result = this.asExpr().(ObjectCreation).getArgument(0).getValue().toInt() + ) + or + ( + // https://docs.microsoft.com/en-us/dotnet/api/system.security.cryptography.rsa.create?view=net-6.0#system-security-cryptography-rsa-create(system-int32) + this.asExpr().(MethodCall).hasNoArguments() and + result = 1024 + or + result = this.asExpr().(MethodCall).getArgument(0).getValue().toInt() + ) + } + } + + class HMac extends HMacSigningAlgorithms { + HMac() { + exists(ObjectCreation object | + object + .getType() + .hasQualifiedName("System.Security.Cryptography", + ["HMACMD5", "HMACSHA1", "HMACSHA256", "HMACSHA384", "HMACSHA512", "HMACRIPEMD160"]) and + this.asExpr() = object + ) + } + + override string algorithm() { + result = this.getType().getName().toUpperCase().replaceAll("HMAC", "") + } + + override DataFlow::ExprNode key() { + result.asExpr() = this.asExpr().(ObjectCreation).getArgument(0) + } + } +} diff --git a/csharp/github/hardcoded.qll b/csharp/github/hardcoded.qll new file mode 100644 index 0000000000..63856db305 --- /dev/null +++ b/csharp/github/hardcoded.qll @@ -0,0 +1,27 @@ +import csharp + +abstract class HardcodedValues extends DataFlow::ExprNode { } + +class NonEmptyStringLiteral extends HardcodedValues { + NonEmptyStringLiteral() { this.getExpr().(StringLiteral).getValue().length() > 1 } +} + +class ByteArrayLiteral extends HardcodedValues { + ByteArrayLiteral() { + this.getExpr() = + any(ArrayCreation ac | + ac.getArrayType().getElementType() instanceof ByteType and + ac.hasInitializer() + ) + } +} + +class CharArrayLiteral extends HardcodedValues { + CharArrayLiteral() { + this.getExpr() = + any(ArrayCreation ac | + ac.getArrayType().getElementType() instanceof CharType and + ac.hasInitializer() + ) + } +} diff --git a/csharp/suites/codeql-csharp.qls b/csharp/suites/codeql-csharp.qls index 2ab0ad73ad..40f9e50570 100644 --- a/csharp/suites/codeql-csharp.qls +++ b/csharp/suites/codeql-csharp.qls @@ -3,6 +3,11 @@ - description: "GitHub's Field Team CodeQL CSharp extended Suite" +- qlpack: github-queries-csharp +- exclude: + tags contain: + - static + - import: codeql-suites/csharp-security-extended.qls from: codeql/csharp-queries @@ -13,6 +18,7 @@ - cs/webclient-path-injection - cs/web/cookie-httponly-not-set - cs/web/cookie-secure-not-set + - cs/hash-without-salt # Backdoor queries - cs/backdoor/dangerous-native-functions - cs/backdoor/potential-time-bomb diff --git a/tests/.gitignore b/tests/.gitignore new file mode 100644 index 0000000000..42070f13d9 --- /dev/null +++ b/tests/.gitignore @@ -0,0 +1,2 @@ +**/*.testproj +**/*.actual diff --git a/tests/csharp-tests/CWE-326/EncryptionInsuffientKeySize/KeySize.cs b/tests/csharp-tests/CWE-326/EncryptionInsuffientKeySize/KeySize.cs new file mode 100644 index 0000000000..11c7c291b8 --- /dev/null +++ b/tests/csharp-tests/CWE-326/EncryptionInsuffientKeySize/KeySize.cs @@ -0,0 +1,42 @@ +using System; +using System.IO; +using System.Text; +using System.Runtime.Serialization; +using System.Security.Cryptography; + +public class StaticSalt +{ + // public void Test1() + // { + // // + // var dsa1 = new DSACryptoServiceProvider(); + + // // + // var dsa2 = new DSACryptoServiceProvider(1024); + + // } + + public void TestRSA() + { + // BAD: Default is insecure + var rsa1 = new RSACryptoServiceProvider(); + var rsa2 = new RSACryptoServiceProvider(1024); + + int key_size = 1024; + var rsa3 = new RSACryptoServiceProvider(key_size); + + // Good + var rsa_good = new RSACryptoServiceProvider(2048); + } + + public void TestRSACng() + { + + // BAD: Default is insecure + var rsacng = new RSACng(); + var rsacng2 = new RSACng(1024); + + // GOOD: High + var rsacng_good2 = new RSACng(2048); + } +} \ No newline at end of file diff --git a/tests/csharp-tests/CWE-326/EncryptionInsuffientKeySize/KeySize.expected b/tests/csharp-tests/CWE-326/EncryptionInsuffientKeySize/KeySize.expected new file mode 100644 index 0000000000..3eecc9384d --- /dev/null +++ b/tests/csharp-tests/CWE-326/EncryptionInsuffientKeySize/KeySize.expected @@ -0,0 +1,4 @@ +| KeySize.cs:22:20:22:49 | object creation of type RSACryptoServiceProvider | Key size 1024 is to small (min: 2048) | +| KeySize.cs:23:20:23:53 | object creation of type RSACryptoServiceProvider | Key size 1024 is to small (min: 2048) | +| KeySize.cs:36:22:36:33 | object creation of type RSACng | Key size 1024 is to small (min: 2048) | +| KeySize.cs:37:23:37:38 | object creation of type RSACng | Key size 1024 is to small (min: 2048) | diff --git a/tests/csharp-tests/CWE-326/EncryptionInsuffientKeySize/KeySize.qlref b/tests/csharp-tests/CWE-326/EncryptionInsuffientKeySize/KeySize.qlref new file mode 100644 index 0000000000..92bf21e7b2 --- /dev/null +++ b/tests/csharp-tests/CWE-326/EncryptionInsuffientKeySize/KeySize.qlref @@ -0,0 +1 @@ +CWE-326/EncryptionInsufficientKeySize.ql \ No newline at end of file diff --git a/tests/csharp-tests/CWE-326/EncryptionInsuffientKeySize/options b/tests/csharp-tests/CWE-326/EncryptionInsuffientKeySize/options new file mode 100644 index 0000000000..be5e69bbe8 --- /dev/null +++ b/tests/csharp-tests/CWE-326/EncryptionInsuffientKeySize/options @@ -0,0 +1 @@ +semmle-extractor-options: /r:System.Security.Cryptography.Csp.dll /r:System.Security.Cryptography.Algorithms.dll /r:System.Security.Cryptography.Cng.dll diff --git a/tests/csharp-tests/CWE-326/EncryptionKeySizeToLarge/KeySizeLarge.cs b/tests/csharp-tests/CWE-326/EncryptionKeySizeToLarge/KeySizeLarge.cs new file mode 100644 index 0000000000..3c72fa046b --- /dev/null +++ b/tests/csharp-tests/CWE-326/EncryptionKeySizeToLarge/KeySizeLarge.cs @@ -0,0 +1,18 @@ +using System; +using System.IO; +using System.Text; +using System.Runtime.Serialization; +using System.Security.Cryptography; + +public class StaticSalt +{ + public void Test1() + { + // BAD + var dsa1 = new DSACryptoServiceProvider(2042); + + // GOOD + var dsa2 = new DSACryptoServiceProvider(1024); + + } +} \ No newline at end of file diff --git a/tests/csharp-tests/CWE-326/EncryptionKeySizeToLarge/KeySizeLarge.expected b/tests/csharp-tests/CWE-326/EncryptionKeySizeToLarge/KeySizeLarge.expected new file mode 100644 index 0000000000..3cdece03d8 --- /dev/null +++ b/tests/csharp-tests/CWE-326/EncryptionKeySizeToLarge/KeySizeLarge.expected @@ -0,0 +1 @@ +| KeySizeLarge.cs:12:20:12:53 | object creation of type DSACryptoServiceProvider | Key size 2042 is to large for algorithm (max: 1024) | diff --git a/tests/csharp-tests/CWE-326/EncryptionKeySizeToLarge/KeySizeLarge.qlref b/tests/csharp-tests/CWE-326/EncryptionKeySizeToLarge/KeySizeLarge.qlref new file mode 100644 index 0000000000..d68a4a4b46 --- /dev/null +++ b/tests/csharp-tests/CWE-326/EncryptionKeySizeToLarge/KeySizeLarge.qlref @@ -0,0 +1 @@ +CWE-326/EncryptionKeySizeToLarge.ql \ No newline at end of file diff --git a/tests/csharp-tests/CWE-326/EncryptionKeySizeToLarge/options b/tests/csharp-tests/CWE-326/EncryptionKeySizeToLarge/options new file mode 100644 index 0000000000..be5e69bbe8 --- /dev/null +++ b/tests/csharp-tests/CWE-326/EncryptionKeySizeToLarge/options @@ -0,0 +1 @@ +semmle-extractor-options: /r:System.Security.Cryptography.Csp.dll /r:System.Security.Cryptography.Algorithms.dll /r:System.Security.Cryptography.Cng.dll diff --git a/tests/csharp-tests/CWE-326/EncryptionKeySizeUseGetter/KeySizeGetter.cs b/tests/csharp-tests/CWE-326/EncryptionKeySizeUseGetter/KeySizeGetter.cs new file mode 100644 index 0000000000..4d7b621ee3 --- /dev/null +++ b/tests/csharp-tests/CWE-326/EncryptionKeySizeUseGetter/KeySizeGetter.cs @@ -0,0 +1,23 @@ +using System; +using System.IO; +using System.Text; +using System.Runtime.Serialization; +using System.Security.Cryptography; + +public class StaticSalt +{ + public void Test1() + { + // BAD + var dsa1 = new DSACryptoServiceProvider(); + dsa1.KeySize = 2048; + + // BAD + var rsa1 = new RSACryptoServiceProvider(); + rsa1.KeySize = 2048; + + // GOOD + var dsa2 = new DSACryptoServiceProvider(1024); + + } +} \ No newline at end of file diff --git a/tests/csharp-tests/CWE-326/EncryptionKeySizeUseGetter/KeySizeGetter.expected b/tests/csharp-tests/CWE-326/EncryptionKeySizeUseGetter/KeySizeGetter.expected new file mode 100644 index 0000000000..b3bd1fa3f1 --- /dev/null +++ b/tests/csharp-tests/CWE-326/EncryptionKeySizeUseGetter/KeySizeGetter.expected @@ -0,0 +1,2 @@ +| KeySizeGetter.cs:13:24:13:27 | 2048 | Cannot use Getter to set key size | +| KeySizeGetter.cs:17:24:17:27 | 2048 | Cannot use Getter to set key size | diff --git a/tests/csharp-tests/CWE-326/EncryptionKeySizeUseGetter/KeySizeGetter.qlref b/tests/csharp-tests/CWE-326/EncryptionKeySizeUseGetter/KeySizeGetter.qlref new file mode 100644 index 0000000000..e3d861365f --- /dev/null +++ b/tests/csharp-tests/CWE-326/EncryptionKeySizeUseGetter/KeySizeGetter.qlref @@ -0,0 +1 @@ +CWE-326/EncryptionKeySizeUseGetter.ql \ No newline at end of file diff --git a/tests/csharp-tests/CWE-326/EncryptionKeySizeUseGetter/options b/tests/csharp-tests/CWE-326/EncryptionKeySizeUseGetter/options new file mode 100644 index 0000000000..be5e69bbe8 --- /dev/null +++ b/tests/csharp-tests/CWE-326/EncryptionKeySizeUseGetter/options @@ -0,0 +1 @@ +semmle-extractor-options: /r:System.Security.Cryptography.Csp.dll /r:System.Security.Cryptography.Algorithms.dll /r:System.Security.Cryptography.Cng.dll diff --git a/tests/csharp-tests/CWE-327/WeakHmac.cs b/tests/csharp-tests/CWE-327/WeakHmac.cs new file mode 100644 index 0000000000..68a926e2df --- /dev/null +++ b/tests/csharp-tests/CWE-327/WeakHmac.cs @@ -0,0 +1,29 @@ +using System; +using System.IO; +using System.Text; +using System.Runtime.Serialization; +using System.Security.Cryptography; +using System.Security.Permissions; + +public class StaticSalt +{ + public void Test1() + { + var key = Encoding.UTF8.GetBytes("TestPassword"); + + // BAD: MD5 + var md5 = new HMACMD5(); + var md5_1 = new HMACMD5(key); + + // BAD: SHA1 (not really but worth reporting) + var sha1 = new HMACSHA1(); + var sha1_2 = new HMACSHA1(key); + + // GOOD: SHA256 + var sha2 = new HMACSHA256(key); + // GOOD: SHA384 + var sha3 = new HMACSHA384(key); + // GOOD: SHA512 + var sha5 = new HMACSHA512(key); + } +} \ No newline at end of file diff --git a/tests/csharp-tests/CWE-327/WeakHmac.expected b/tests/csharp-tests/CWE-327/WeakHmac.expected new file mode 100644 index 0000000000..54a473dd41 --- /dev/null +++ b/tests/csharp-tests/CWE-327/WeakHmac.expected @@ -0,0 +1,8 @@ +edges +nodes +subpaths +#select +| WeakHmac.cs:15:19:15:31 | object creation of type HMACMD5 | Algs | +| WeakHmac.cs:16:21:16:36 | object creation of type HMACMD5 | Algs | +| WeakHmac.cs:19:20:19:33 | object creation of type HMACSHA1 | Algs | +| WeakHmac.cs:20:22:20:38 | object creation of type HMACSHA1 | Algs | diff --git a/tests/csharp-tests/CWE-327/WeakHmac.qlref b/tests/csharp-tests/CWE-327/WeakHmac.qlref new file mode 100644 index 0000000000..4e13afb2e2 --- /dev/null +++ b/tests/csharp-tests/CWE-327/WeakHmac.qlref @@ -0,0 +1 @@ +CWE-327/WeakHMac.ql \ No newline at end of file diff --git a/tests/csharp-tests/CWE-327/options b/tests/csharp-tests/CWE-327/options new file mode 100644 index 0000000000..f2f776d118 --- /dev/null +++ b/tests/csharp-tests/CWE-327/options @@ -0,0 +1 @@ +semmle-extractor-options: /r:System.Security.Cryptography.Csp.dll /r:System.Security.Cryptography.Algorithms.dll diff --git a/tests/csharp-tests/CWE-760/HardcodedSalt.expected b/tests/csharp-tests/CWE-760/HardcodedSalt.expected new file mode 100644 index 0000000000..bdd849652e --- /dev/null +++ b/tests/csharp-tests/CWE-760/HardcodedSalt.expected @@ -0,0 +1,18 @@ +edges +| Test.cs:13:16:13:55 | call to method GetBytes : Byte[] | Test.cs:14:49:14:52 | access to local variable salt | +| Test.cs:13:39:13:54 | "Hardcoded Salt" : String | Test.cs:13:16:13:55 | call to method GetBytes : Byte[] | +| Test.cs:23:12:23:28 | "Hardcoded Salt2" : String | Test.cs:29:39:29:63 | call to method generateSalt : String | +| Test.cs:29:16:29:64 | call to method GetBytes : Byte[] | Test.cs:30:49:30:52 | access to local variable salt | +| Test.cs:29:39:29:63 | call to method generateSalt : String | Test.cs:29:16:29:64 | call to method GetBytes : Byte[] | +nodes +| Test.cs:13:16:13:55 | call to method GetBytes : Byte[] | semmle.label | call to method GetBytes : Byte[] | +| Test.cs:13:39:13:54 | "Hardcoded Salt" : String | semmle.label | "Hardcoded Salt" : String | +| Test.cs:14:49:14:52 | access to local variable salt | semmle.label | access to local variable salt | +| Test.cs:23:12:23:28 | "Hardcoded Salt2" : String | semmle.label | "Hardcoded Salt2" : String | +| Test.cs:29:16:29:64 | call to method GetBytes : Byte[] | semmle.label | call to method GetBytes : Byte[] | +| Test.cs:29:39:29:63 | call to method generateSalt : String | semmle.label | call to method generateSalt : String | +| Test.cs:30:49:30:52 | access to local variable salt | semmle.label | access to local variable salt | +subpaths +#select +| Test.cs:14:49:14:52 | access to local variable salt | Test.cs:13:39:13:54 | "Hardcoded Salt" : String | Test.cs:14:49:14:52 | access to local variable salt | Use of $@. | Test.cs:13:39:13:54 | "Hardcoded Salt" | hardcoded salt | +| Test.cs:30:49:30:52 | access to local variable salt | Test.cs:23:12:23:28 | "Hardcoded Salt2" : String | Test.cs:30:49:30:52 | access to local variable salt | Use of $@. | Test.cs:23:12:23:28 | "Hardcoded Salt2" | hardcoded salt | diff --git a/tests/csharp-tests/CWE-760/HardcodedSalt.qlref b/tests/csharp-tests/CWE-760/HardcodedSalt.qlref new file mode 100644 index 0000000000..f8e5d2cdc8 --- /dev/null +++ b/tests/csharp-tests/CWE-760/HardcodedSalt.qlref @@ -0,0 +1 @@ +CWE-760/HardcodedSalt.ql \ No newline at end of file diff --git a/tests/csharp-tests/CWE-760/Test.cs b/tests/csharp-tests/CWE-760/Test.cs new file mode 100644 index 0000000000..a68507404f --- /dev/null +++ b/tests/csharp-tests/CWE-760/Test.cs @@ -0,0 +1,32 @@ +using System; +using System.IO; +using System.Text; +using System.Runtime.Serialization; +using System.Security.Cryptography; +using System.Security.Permissions; + +public class StaticSalt { + public void Test1() { + string password = "TestPassword"; + + // BAD: Static String + var salt = Encoding.UTF8.GetBytes("Hardcoded Salt"); + var hash = new Rfc2898DeriveBytes(password, salt); + + // Good: Randomly generated byte array + var randonSalt = new byte[16]; + RandomNumberGenerator.Create().GetBytes(randonSalt); + var hash_safe = new Rfc2898DeriveBytes(password, randonSalt); + } + + public static string generateSalt() { + return "Hardcoded Salt2"; + } + public void Test2() { + string password = "TestPassword2"; + + // BAD: Static String + var salt = Encoding.UTF8.GetBytes(StaticSalt.generateSalt()); + var hash = new Rfc2898DeriveBytes(password, salt); + } +} \ No newline at end of file diff --git a/tests/csharp-tests/CWE-760/options b/tests/csharp-tests/CWE-760/options new file mode 100644 index 0000000000..f2f776d118 --- /dev/null +++ b/tests/csharp-tests/CWE-760/options @@ -0,0 +1 @@ +semmle-extractor-options: /r:System.Security.Cryptography.Csp.dll /r:System.Security.Cryptography.Algorithms.dll diff --git a/tests/csharp-tests/CWE-916/Test.cs b/tests/csharp-tests/CWE-916/Test.cs new file mode 100644 index 0000000000..dbccc1bebf --- /dev/null +++ b/tests/csharp-tests/CWE-916/Test.cs @@ -0,0 +1,32 @@ +using System; +using System.IO; +using System.Text; +using System.Runtime.Serialization; +using System.Security.Cryptography; +using System.Security.Permissions; + +public class StaticSalt +{ + public void Test1() + { + string password = "TestPassword"; + var randonSalt = new byte[16]; + RandomNumberGenerator.Create().GetBytes(randonSalt); + + + // BAD: Default (1000) + var hash = new Rfc2898DeriveBytes(password, randonSalt); + + // BAD: Static int + var hash2 = new Rfc2898DeriveBytes(password, randonSalt, 1000); + + // BAD: Static int + int interations = 1000; + var hash3 = new Rfc2898DeriveBytes(password, randonSalt, interations); + + + + // Good: High interations + var hash_safe = new Rfc2898DeriveBytes(password, randonSalt, 100000); + } +} \ No newline at end of file diff --git a/tests/csharp-tests/CWE-916/WeakIterations.expected b/tests/csharp-tests/CWE-916/WeakIterations.expected new file mode 100644 index 0000000000..f8cc1d094d --- /dev/null +++ b/tests/csharp-tests/CWE-916/WeakIterations.expected @@ -0,0 +1,12 @@ +edges +| Test.cs:24:27:24:30 | 1000 : Int32 | Test.cs:25:66:25:76 | access to local variable interations | +nodes +| Test.cs:18:20:18:63 | object creation of type Rfc2898DeriveBytes | semmle.label | object creation of type Rfc2898DeriveBytes | +| Test.cs:21:66:21:69 | 1000 | semmle.label | 1000 | +| Test.cs:24:27:24:30 | 1000 : Int32 | semmle.label | 1000 : Int32 | +| Test.cs:25:66:25:76 | access to local variable interations | semmle.label | access to local variable interations | +subpaths +#select +| Test.cs:18:20:18:63 | object creation of type Rfc2898DeriveBytes | Test.cs:18:20:18:63 | object creation of type Rfc2898DeriveBytes | Test.cs:18:20:18:63 | object creation of type Rfc2898DeriveBytes | Use of $@. | Test.cs:18:20:18:63 | object creation of type Rfc2898DeriveBytes | hardcoded weak iterations | +| Test.cs:21:66:21:69 | 1000 | Test.cs:21:66:21:69 | 1000 | Test.cs:21:66:21:69 | 1000 | Use of $@. | Test.cs:21:66:21:69 | 1000 | hardcoded weak iterations | +| Test.cs:25:66:25:76 | access to local variable interations | Test.cs:24:27:24:30 | 1000 : Int32 | Test.cs:25:66:25:76 | access to local variable interations | Use of $@. | Test.cs:24:27:24:30 | 1000 | hardcoded weak iterations | diff --git a/tests/csharp-tests/CWE-916/WeakIterations.qlref b/tests/csharp-tests/CWE-916/WeakIterations.qlref new file mode 100644 index 0000000000..194cb7eaf3 --- /dev/null +++ b/tests/csharp-tests/CWE-916/WeakIterations.qlref @@ -0,0 +1 @@ +CWE-916/WeakIterations.ql \ No newline at end of file diff --git a/tests/csharp-tests/CWE-916/options b/tests/csharp-tests/CWE-916/options new file mode 100644 index 0000000000..f2f776d118 --- /dev/null +++ b/tests/csharp-tests/CWE-916/options @@ -0,0 +1 @@ +semmle-extractor-options: /r:System.Security.Cryptography.Csp.dll /r:System.Security.Cryptography.Algorithms.dll diff --git a/tests/csharp-tests/qlpack.lock.yml b/tests/csharp-tests/qlpack.lock.yml new file mode 100644 index 0000000000..a046f6d978 --- /dev/null +++ b/tests/csharp-tests/qlpack.lock.yml @@ -0,0 +1,4 @@ +--- +dependencies: {} +compiled: false +lockVersion: 1.0.0 \ No newline at end of file diff --git a/tests/csharp-tests/qlpack.yml b/tests/csharp-tests/qlpack.yml new file mode 100644 index 0000000000..76b4d626f6 --- /dev/null +++ b/tests/csharp-tests/qlpack.yml @@ -0,0 +1,8 @@ +name: github-queries-csharp-tests +groups: [csharp, test] +dependencies: + codeql/csharp-all: "*" + github-queries-csharp: "*" + +extractor: csharp +tests: . \ No newline at end of file