-
Notifications
You must be signed in to change notification settings - Fork 21
CSharp Crypto Queries #32
Changes from all commits
0537ad2
b005b05
8dd26cf
93afe9b
34f1287
7c2cd85
9d4c4bd
9c215ef
e0b6a3d
c6d10a1
62ac79e
c048d0f
c48db6f
1a513bc
7f61526
d3af1c0
69433ac
688e3ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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() + ")" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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() + ")" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to --> too |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we not remove this line and line 25? If this was located in a .qll file, I would understand that it offers clients the ability to add things, but in a .ql file I would just leave it out. If it is ever needed, we can add it then.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, scratch that, I guess this does something, indeed.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still, lines 25 and 27 look a bit strange in this setting, I would remove them and directly refer to |
||
|
|
||
| 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" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps, |
||
| } | ||
|
|
||
| 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" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to --> too