hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-24 04:36:35 +01:00
Code Issues Packages Projects Releases Wiki Activity

clean up code

Browse Source
This commit is contained in:
Jami Cogswell
2022-10-14 13:03:34 -04:00
parent 0334470f33
commit da218fdbf1
4 changed files with 51 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
java/ql/lib/semmle/code/java/security/Encryption.qll
View File

@@ -83,6 +83,11 @@ class KeyGenerator extends RefType {
KeyGenerator() { this.hasQualifiedName("javax.crypto", "KeyGenerator") }
}
/** The Java class `java.security.KeyPairGenerator`. */
class KeyPairGenerator extends RefType {
KeyPairGenerator() { this.hasQualifiedName("java.security", "KeyPairGenerator") }
}
/** The `init` method declared in `javax.crypto.KeyGenerator`. */
class KeyGeneratorInitMethod extends Method {
KeyGeneratorInitMethod() {
@@ -91,11 +96,6 @@ class KeyGeneratorInitMethod extends Method {
}
}
/** The Java class `java.security.KeyPairGenerator`. */
class KeyPairGenerator extends RefType {
KeyPairGenerator() { this.hasQualifiedName("java.security", "KeyPairGenerator") }
}
/** The `initialize` method declared in `java.security.KeyPairGenerator`. */
class KeyPairGeneratorInitMethod extends Method {
KeyPairGeneratorInitMethod() {
@@ -323,7 +323,6 @@ class JavaxCryptoSecretKey extends JavaxCryptoAlgoSpec {
}
}
// TODO: consider extending JavaxCryptoAlgoSpec as above does; will need to override getAlgoSpec() method
/** The Java class `javax.crypto.spec.DHGenParameterSpec`. */
class DhGenParameterSpec extends RefType {
DhGenParameterSpec() { this.hasQualifiedName("javax.crypto.spec", "DHGenParameterSpec") }
@@ -389,19 +388,16 @@ class JavaSecuritySignature extends JavaSecurityAlgoSpec {
override Expr getAlgoSpec() { result = this.(ConstructorCall).getArgument(0) }
}
// TODO: consider extending JavaSecurityAlgoSpec as above does; will need to override getAlgoSpec() method
/** The Java class `java.security.spec.ECGenParameterSpec`. */
class EcGenParameterSpec extends RefType {
EcGenParameterSpec() { this.hasQualifiedName("java.security.spec", "ECGenParameterSpec") }
}
// TODO: consider extending JavaSecurityAlgoSpec as above does; will need to override getAlgoSpec() method
/** The Java class `java.security.spec.RSAKeyGenParameterSpec`. */
class RsaKeyGenParameterSpec extends RefType {
RsaKeyGenParameterSpec() { this.hasQualifiedName("java.security.spec", "RSAKeyGenParameterSpec") }
}
// TODO: consider extending JavaSecurityAlgoSpec as above does; will need to override getAlgoSpec() method
/** The Java class `java.security.spec.DSAGenParameterSpec`. */
class DsaGenParameterSpec extends RefType {
DsaGenParameterSpec() { this.hasQualifiedName("java.security.spec", "DSAGenParameterSpec") }

85
java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll
View File

@@ -15,36 +15,40 @@ abstract class InsufficientKeySizeSink extends DataFlow::Node {
predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty }
}
/** A source for an insufficient key size (asymmetric-non-ec: RSA, DSA, DH). */
// *********************************** SOURCES ***********************************
/** A source for an insufficient key size used in RSA, DSA, and DH algorithms. */
private class AsymmetricNonEcSource extends InsufficientKeySizeSource {
AsymmetricNonEcSource() { getNodeIntValue(this) < 2048 }
override predicate hasState(DataFlow::FlowState state) { state = "2048" }
}
/** A source for an insufficient key size (asymmetric-ec: EC%). */
/** A source for an insufficient key size used in elliptic curve (EC) algorithms. */
private class AsymmetricEcSource extends InsufficientKeySizeSource {
AsymmetricEcSource() {
getNodeIntValue(this) < 256 or
getEcKeySize(this.asExpr().(StringLiteral).getValue()) < 256 // for cases when the key size is embedded in the curve name
getNodeIntValue(this) < 256
or
// the below is needed for cases when the key size is embedded in the curve name
getEcKeySize(this.asExpr().(StringLiteral).getValue()) < 256
}
override predicate hasState(DataFlow::FlowState state) { state = "256" }
}
/** A source for an insufficient key size (symmetric: AES). */
/** A source for an insufficient key size used in AES algorithms. */
private class SymmetricSource extends InsufficientKeySizeSource {
SymmetricSource() { getNodeIntValue(this) < 128 }
override predicate hasState(DataFlow::FlowState state) { state = "128" }
}
// ************************** SOURCES HELPER PREDICATES **************************
/** Returns the integer value of a given Node. */
private int getNodeIntValue(DataFlow::Node node) {
result = node.asExpr().(IntegerLiteral).getIntValue()
}
// TODO: check if any other regex should be added based on some MRVA results.
/** Returns the key size in the EC algorithm string */
/** Returns the key size from an EC algorithm curve name string */
bindingset[algorithm]
private int getEcKeySize(string algorithm) {
algorithm.matches("sec%") and // specification such as "secp256r1"
@@ -57,31 +61,26 @@ private int getEcKeySize(string algorithm) {
result = algorithm.regexpCapture(".*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt()
}
/** A sink for an insufficient key size (asymmetric-non-ec). */
// ************************************ SINKS ************************************
/** A sink for an insufficient key size used in RSA, DSA, and DH algorithms. */
private class AsymmetricNonEcSink extends InsufficientKeySizeSink {
AsymmetricNonEcSink() {
// hasKeySizeInInitMethod(this, "asymmetric-non-ec")
// or
//hasKeySizeInSpec(this, "asymmetric-non-ec")
exists(AsymmInitMethodAccess ma, AsymmKeyGen kg |
exists(AsymmetricInitMethodAccess ma, AsymmetricKeyGenerator kg |
kg.getAlgoName().matches(["RSA", "DSA", "DH"]) and
DataFlow::localExprFlow(kg, ma.getQualifier()) and
this.asExpr() = ma.getKeySizeArg()
)
or
exists(AsymmetricNonEcSpec s | this.asExpr() = s.getKeySizeArg())
exists(AsymmetricNonEcSpec spec | this.asExpr() = spec.getKeySizeArg())
}
override predicate hasState(DataFlow::FlowState state) { state = "2048" }
}
/** A sink for an insufficient key size (asymmetric-ec). */
/** A sink for an insufficient key size used in elliptic curve (EC) algorithms. */
private class AsymmetricEcSink extends InsufficientKeySizeSink {
AsymmetricEcSink() {
// hasKeySizeInInitMethod(this, "asymmetric-ec")
// or
//hasKeySizeInSpec(this, "asymmetric-ec")
exists(AsymmInitMethodAccess ma, AsymmKeyGen kg |
exists(AsymmetricInitMethodAccess ma, AsymmetricKeyGenerator kg |
kg.getAlgoName().matches("EC%") and
DataFlow::localExprFlow(kg, ma.getQualifier()) and
this.asExpr() = ma.getKeySizeArg()
@@ -93,11 +92,11 @@ private class AsymmetricEcSink extends InsufficientKeySizeSink {
override predicate hasState(DataFlow::FlowState state) { state = "256" }
}
/** A sink for an insufficient key size (symmetric). */
/** A sink for an insufficient key size used in AES algorithms. */
private class SymmetricSink extends InsufficientKeySizeSink {
SymmetricSink() {
//hasKeySizeInInitMethod(this, "symmetric")
exists(SymmInitMethodAccess ma, SymmKeyGen kg |
exists(SymmetricInitMethodAccess ma, SymmetricKeyGenerator kg |
kg.getAlgoName() = "AES" and
DataFlow::localExprFlow(kg, ma.getQualifier()) and
this.asExpr() = ma.getKeySizeArg()
@@ -107,43 +106,49 @@ private class SymmetricSink extends InsufficientKeySizeSink {
override predicate hasState(DataFlow::FlowState state) { state = "128" }
}
abstract class InitMethodAccess extends MethodAccess {
// ********************** SINKS HELPER CLASSES & PREDICATES **********************
/** A call to a method that initializes a key generator. */
abstract class KeyGenInitMethodAccess extends MethodAccess {
/** Gets the `keysize` argument of this call. */
Argument getKeySizeArg() { result = this.getArgument(0) }
}
class AsymmInitMethodAccess extends InitMethodAccess {
AsymmInitMethodAccess() { this.getMethod() instanceof KeyPairGeneratorInitMethod }
/** A call to the `initialize` method declared in `java.security.KeyPairGenerator`. */
private class AsymmetricInitMethodAccess extends KeyGenInitMethodAccess {
AsymmetricInitMethodAccess() { this.getMethod() instanceof KeyPairGeneratorInitMethod }
}
class SymmInitMethodAccess extends InitMethodAccess {
SymmInitMethodAccess() { this.getMethod() instanceof KeyGeneratorInitMethod }
/** A call to the `init` method declared in `javax.crypto.KeyGenerator`. */
private class SymmetricInitMethodAccess extends KeyGenInitMethodAccess {
SymmetricInitMethodAccess() { this.getMethod() instanceof KeyGeneratorInitMethod }
}
abstract class KeyGen extends JavaxCryptoAlgoSpec {
/** An instance of a key generator. */
abstract class KeyGeneratorObject extends JavaxCryptoAlgoSpec {
string getAlgoName() { result = this.getAlgoSpec().(StringLiteral).getValue().toUpperCase() }
}
class AsymmKeyGen extends KeyGen {
AsymmKeyGen() { this instanceof JavaSecurityKeyPairGenerator }
/** An instance of a `java.security.KeyPairGenerator`. */
private class AsymmetricKeyGenerator extends KeyGeneratorObject {
AsymmetricKeyGenerator() { this instanceof JavaSecurityKeyPairGenerator }
// ! this override is repetitive...
override Expr getAlgoSpec() { result = this.(MethodAccess).getArgument(0) }
}
class SymmKeyGen extends KeyGen {
SymmKeyGen() { this instanceof JavaxCryptoKeyGenerator }
/** An instance of a `javax.crypto.KeyGenerator`. */
private class SymmetricKeyGenerator extends KeyGeneratorObject {
SymmetricKeyGenerator() { this instanceof JavaxCryptoKeyGenerator }
// ! this override is repetitive...
override Expr getAlgoSpec() { result = this.(MethodAccess).getArgument(0) }
}
// ! use below instead of/in above?? (actually I don't think I need any of this, can just use AsymmetricNonEcSpec and EcGenParameterSpec directly???)
// Algo spec
abstract class AsymmetricAlgoSpec extends ClassInstanceExpr {
/** An instance of an algorithm specification. */
abstract class AlgoSpec extends ClassInstanceExpr {
Argument getKeySizeArg() { result = this.getArgument(0) }
}
class AsymmetricNonEcSpec extends AsymmetricAlgoSpec {
/** An instance of an RSA, DSA, or DH algorithm specification. */
private class AsymmetricNonEcSpec extends AlgoSpec {
AsymmetricNonEcSpec() {
this.getConstructedType() instanceof RsaKeyGenParameterSpec or
this.getConstructedType() instanceof DsaGenParameterSpec or
@@ -151,11 +156,7 @@ class AsymmetricNonEcSpec extends AsymmetricAlgoSpec {
}
}
class AsymmetricEcSpec extends AsymmetricAlgoSpec {
/** An instance of an elliptic curve (EC) algorithm specification. */
private class AsymmetricEcSpec extends AlgoSpec {
AsymmetricEcSpec() { this.getConstructedType() instanceof EcGenParameterSpec }
}
// TODO:
// todo #0: look into use of specs without keygen objects; should spec not be a sink in these cases?
// todo #3: make list of algo names more easily reusable (either as constant-type variable at top of file, or model as own class to share, etc.)
// todo: add barrier guard for !=0 conditional case
// todo: only update key sizes (and key size strings) in one place in the code

2
java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll
View File

@@ -1,8 +1,6 @@
/** Provides data flow configurations to be used in queries related to insufficient key sizes. */
import semmle.code.java.security.Encryption
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.security.InsufficientKeySize
/** A data flow configuration for tracking key sizes used in cryptographic algorithms. */

9
java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql
View File

@@ -16,9 +16,6 @@ import semmle.code.java.security.InsufficientKeySizeQuery
import DataFlow::PathGraph
from DataFlow::PathNode source, DataFlow::PathNode sink
where exists(KeySizeConfiguration config1 | config1.hasFlowPath(source, sink))
//or
// exists(AsymmetricNonECKeyTrackingConfiguration cfg | cfg.hasFlowPath(source, sink)) or
// exists(AsymmetricECKeyTrackingConfiguration cfg | cfg.hasFlowPath(source, sink)) or
// exists(SymmetricKeyTrackingConfiguration cfg | cfg.hasFlowPath(source, sink))
select sink.getNode(), source, sink, "This $@ is too small.", source.getNode(), "key size"
where exists(KeySizeConfiguration cfg | cfg.hasFlowPath(source, sink))
select sink.getNode(), source, sink, "This $@ is less than the recommended key size.",
source.getNode(), "key size"