mirror of
https://github.com/github/codeql.git
synced 2025-12-24 04:36:35 +01:00
clean up code somewhat
This commit is contained in:
@@ -3,11 +3,6 @@ import semmle.code.java.dataflow.TaintTracking2
|
|||||||
import semmle.code.java.dataflow.TaintTracking
|
import semmle.code.java.dataflow.TaintTracking
|
||||||
import semmle.code.java.dataflow.DataFlow
|
import semmle.code.java.dataflow.DataFlow
|
||||||
|
|
||||||
// TODO:
|
|
||||||
// todo #1: make representation of source that can be shared across the configs
|
|
||||||
// todo #2: make representation of sink that can be shared across the configs
|
|
||||||
// todo #3: finish adding tracking for algo type/name... need flow/taint-tracking for across methods??
|
|
||||||
// todo #3a: 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.)
|
|
||||||
// ******* DATAFLOW BELOW *************************************************************************
|
// ******* DATAFLOW BELOW *************************************************************************
|
||||||
/**
|
/**
|
||||||
* Asymmetric (RSA, DSA, DH) key length data flow tracking configuration.
|
* Asymmetric (RSA, DSA, DH) key length data flow tracking configuration.
|
||||||
@@ -16,8 +11,10 @@ class AsymmetricKeyTrackingConfiguration extends TaintTracking2::Configuration {
|
|||||||
AsymmetricKeyTrackingConfiguration() { this = "AsymmetricKeyTrackingConfiguration" }
|
AsymmetricKeyTrackingConfiguration() { this = "AsymmetricKeyTrackingConfiguration" }
|
||||||
|
|
||||||
override predicate isSource(DataFlow::Node source) {
|
override predicate isSource(DataFlow::Node source) {
|
||||||
|
// ! may need to change below to still use `keysize` variable as the source, not the spec
|
||||||
|
// ! also need to look into specs for DSA and DH more
|
||||||
exists(ClassInstanceExpr rsaGenParamSpec |
|
exists(ClassInstanceExpr rsaGenParamSpec |
|
||||||
rsaGenParamSpec.getConstructedType() instanceof RSAGenParameterSpec and // ! double-check if should just use getType() instead
|
rsaGenParamSpec.getConstructedType() instanceof RSAGenParameterSpec and
|
||||||
rsaGenParamSpec.getArgument(0).(IntegerLiteral).getIntValue() < 2048 and
|
rsaGenParamSpec.getArgument(0).(IntegerLiteral).getIntValue() < 2048 and
|
||||||
source.asExpr() = rsaGenParamSpec
|
source.asExpr() = rsaGenParamSpec
|
||||||
)
|
)
|
||||||
@@ -26,18 +23,8 @@ class AsymmetricKeyTrackingConfiguration extends TaintTracking2::Configuration {
|
|||||||
}
|
}
|
||||||
|
|
||||||
override predicate isSink(DataFlow::Node sink) {
|
override predicate isSink(DataFlow::Node sink) {
|
||||||
exists(MethodAccess ma, VarAccess va |
|
exists(MethodAccess ma |
|
||||||
ma.getMethod() instanceof KeyPairGeneratorInitMethod and
|
ma.getMethod() instanceof KeyPairGeneratorInitMethod and
|
||||||
//ma.getFile().getBaseName().matches("SignatureTest.java") and
|
|
||||||
// va.getVariable()
|
|
||||||
// .getAnAssignedValue()
|
|
||||||
// .(JavaSecurityKeyPairGenerator)
|
|
||||||
// .getAlgoSpec()
|
|
||||||
// .(StringLiteral)
|
|
||||||
// .getValue()
|
|
||||||
// .toUpperCase()
|
|
||||||
// .matches(["RSA", "DSA", "DH"]) and
|
|
||||||
// ma.getQualifier() = va and
|
|
||||||
exists(
|
exists(
|
||||||
JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kpgConfig,
|
JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kpgConfig,
|
||||||
DataFlow::PathNode source, DataFlow::PathNode dest
|
DataFlow::PathNode source, DataFlow::PathNode dest
|
||||||
@@ -52,13 +39,6 @@ class AsymmetricKeyTrackingConfiguration extends TaintTracking2::Configuration {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// predicate hasInsufficientKeySize(string msg) { hasShortAsymmetricKeyPair(msg) }
|
|
||||||
// predicate hasShortAsymmetricKeyPair(string msg) {
|
|
||||||
// exists(AsymmetricKeyTrackingConfiguration config1, DataFlow::Node source, DataFlow::Node sink |
|
|
||||||
// config1.hasFlow(source, sink)
|
|
||||||
// ) and
|
|
||||||
// msg = "Key size should be at least 2048 bits for " + "___" + " encryption."
|
|
||||||
// }
|
|
||||||
/**
|
/**
|
||||||
* Asymmetric (EC) key length data flow tracking configuration.
|
* Asymmetric (EC) key length data flow tracking configuration.
|
||||||
*/
|
*/
|
||||||
@@ -66,8 +46,9 @@ class AsymmetricECCKeyTrackingConfiguration extends TaintTracking2::Configuratio
|
|||||||
AsymmetricECCKeyTrackingConfiguration() { this = "AsymmetricECCKeyTrackingConfiguration" }
|
AsymmetricECCKeyTrackingConfiguration() { this = "AsymmetricECCKeyTrackingConfiguration" }
|
||||||
|
|
||||||
override predicate isSource(DataFlow::Node source) {
|
override predicate isSource(DataFlow::Node source) {
|
||||||
|
// ! may need to change below to still use `keysize` variable as the source, not the spec
|
||||||
exists(ClassInstanceExpr ecGenParamSpec |
|
exists(ClassInstanceExpr ecGenParamSpec |
|
||||||
getECKeySize(ecGenParamSpec.getArgument(0).(StringLiteral).getValue()) < 256 and // ! can generate EC with just the keysize and not the curve apparently... (based on netty/netty FP example)
|
getECKeySize(ecGenParamSpec.getArgument(0).(StringLiteral).getValue()) < 256 and
|
||||||
source.asExpr() = ecGenParamSpec
|
source.asExpr() = ecGenParamSpec
|
||||||
)
|
)
|
||||||
or
|
or
|
||||||
@@ -75,18 +56,8 @@ class AsymmetricECCKeyTrackingConfiguration extends TaintTracking2::Configuratio
|
|||||||
}
|
}
|
||||||
|
|
||||||
override predicate isSink(DataFlow::Node sink) {
|
override predicate isSink(DataFlow::Node sink) {
|
||||||
exists(MethodAccess ma, VarAccess va |
|
exists(MethodAccess ma |
|
||||||
ma.getMethod() instanceof KeyPairGeneratorInitMethod and
|
ma.getMethod() instanceof KeyPairGeneratorInitMethod and
|
||||||
//ma.getArgument(0).getType() instanceof ECGenParameterSpec and // ! can generate EC with just the keysize and not the curve apparently... (based on netty/netty FP example)
|
|
||||||
// va.getVariable()
|
|
||||||
// .getAnAssignedValue()
|
|
||||||
// .(JavaSecurityKeyPairGenerator)
|
|
||||||
// .getAlgoSpec()
|
|
||||||
// .(StringLiteral)
|
|
||||||
// .getValue()
|
|
||||||
// .toUpperCase()
|
|
||||||
// .matches(["EC%"]) and
|
|
||||||
// ma.getQualifier() = va and
|
|
||||||
exists(
|
exists(
|
||||||
JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kpgConfig,
|
JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kpgConfig,
|
||||||
DataFlow::PathNode source, DataFlow::PathNode dest
|
DataFlow::PathNode source, DataFlow::PathNode dest
|
||||||
@@ -112,17 +83,8 @@ class SymmetricKeyTrackingConfiguration extends TaintTracking2::Configuration {
|
|||||||
}
|
}
|
||||||
|
|
||||||
override predicate isSink(DataFlow::Node sink) {
|
override predicate isSink(DataFlow::Node sink) {
|
||||||
exists(MethodAccess ma, VarAccess va |
|
exists(MethodAccess ma |
|
||||||
ma.getMethod() instanceof KeyGeneratorInitMethod and
|
ma.getMethod() instanceof KeyGeneratorInitMethod and
|
||||||
// va.getVariable()
|
|
||||||
// .getAnAssignedValue()
|
|
||||||
// .(JavaxCryptoKeyGenerator)
|
|
||||||
// .getAlgoSpec()
|
|
||||||
// .(StringLiteral)
|
|
||||||
// .getValue()
|
|
||||||
// .toUpperCase()
|
|
||||||
// .matches(["AES"]) and
|
|
||||||
// ma.getQualifier() = va and
|
|
||||||
exists(
|
exists(
|
||||||
JavaxCryptoKeyGenerator jcg, KeyGeneratorInitConfiguration kgConfig,
|
JavaxCryptoKeyGenerator jcg, KeyGeneratorInitConfiguration kgConfig,
|
||||||
DataFlow::PathNode source, DataFlow::PathNode dest
|
DataFlow::PathNode source, DataFlow::PathNode dest
|
||||||
@@ -137,71 +99,13 @@ class SymmetricKeyTrackingConfiguration extends TaintTracking2::Configuration {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// ! below doesn't work for some reason...
|
// ! below predicate doesn't work
|
||||||
// predicate hasInsufficientKeySize2(DataFlow::PathNode source, DataFlow::PathNode sink) {
|
// predicate hasInsufficientKeySize2(DataFlow::PathNode source, DataFlow::PathNode sink) {
|
||||||
// exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink))
|
// exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink))
|
||||||
// or
|
// or
|
||||||
// exists(SymmetricKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink))
|
// exists(SymmetricKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink))
|
||||||
// }
|
// }
|
||||||
// ******** Need the below for the above ********
|
// ******** Need the below models for the above configs ********
|
||||||
// ! move to Encryption.qll?
|
|
||||||
/** The Java class `java.security.spec.ECGenParameterSpec`. */
|
|
||||||
private class ECGenParameterSpec extends RefType {
|
|
||||||
ECGenParameterSpec() { this.hasQualifiedName("java.security.spec", "ECGenParameterSpec") }
|
|
||||||
}
|
|
||||||
|
|
||||||
/** The Java class `java.security.spec.ECGenParameterSpec`. */
|
|
||||||
private class RSAGenParameterSpec extends RefType {
|
|
||||||
RSAGenParameterSpec() { this.hasQualifiedName("java.security.spec", "RSAKeyGenParameterSpec") }
|
|
||||||
}
|
|
||||||
|
|
||||||
// ! move to Encryption.qll?
|
|
||||||
/** Returns the key size in the EC algorithm string */
|
|
||||||
bindingset[algorithm]
|
|
||||||
private int getECKeySize(string algorithm) {
|
|
||||||
algorithm.matches("sec%") and // specification such as "secp256r1"
|
|
||||||
result = algorithm.regexpCapture("sec[p|t](\\d+)[a-zA-Z].*", 1).toInt()
|
|
||||||
or
|
|
||||||
algorithm.matches("X9.62%") and //specification such as "X9.62 prime192v2"
|
|
||||||
result = algorithm.regexpCapture("X9\\.62 .*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt()
|
|
||||||
or
|
|
||||||
(algorithm.matches("prime%") or algorithm.matches("c2tnb%")) and //specification such as "prime192v2"
|
|
||||||
result = algorithm.regexpCapture(".*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt()
|
|
||||||
}
|
|
||||||
|
|
||||||
// ! move to Encryption.qll?
|
|
||||||
/** The `init` method declared in `javax.crypto.KeyGenerator`. */
|
|
||||||
private class KeyGeneratorInitMethod extends Method {
|
|
||||||
KeyGeneratorInitMethod() {
|
|
||||||
this.getDeclaringType() instanceof KeyGenerator and
|
|
||||||
this.hasName("init")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// ! move to Encryption.qll?
|
|
||||||
/** The `initialize` method declared in `java.security.KeyPairGenerator`. */
|
|
||||||
private class KeyPairGeneratorInitMethod extends Method {
|
|
||||||
KeyPairGeneratorInitMethod() {
|
|
||||||
this.getDeclaringType() instanceof KeyPairGenerator and
|
|
||||||
this.hasName("initialize")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// ******* DATAFLOW ABOVE *************************************************************************
|
|
||||||
// ************************************************************************************************
|
|
||||||
// ************************************************************************************************
|
|
||||||
// ************************************************************************************************
|
|
||||||
// ************************************************************************************************
|
|
||||||
// ************************************************************************************************
|
|
||||||
// ******* OLD/UNUSED OR EXPERIMENTAL CODE BELOW **************************************************
|
|
||||||
class UnsafeSymmetricKeySize extends IntegerLiteral {
|
|
||||||
UnsafeSymmetricKeySize() { this.getIntValue() < 128 }
|
|
||||||
}
|
|
||||||
|
|
||||||
class UnsafeAsymmetricKeySize extends IntegerLiteral {
|
|
||||||
UnsafeAsymmetricKeySize() { this.getIntValue() < 2048 }
|
|
||||||
}
|
|
||||||
|
|
||||||
/** Taint configuration tracking flow from a key generator to a `init` method call. */
|
/** Taint configuration tracking flow from a key generator to a `init` method call. */
|
||||||
private class KeyGeneratorInitConfiguration extends TaintTracking::Configuration {
|
private class KeyGeneratorInitConfiguration extends TaintTracking::Configuration {
|
||||||
KeyGeneratorInitConfiguration() { this = "KeyGeneratorInitConfiguration" }
|
KeyGeneratorInitConfiguration() { this = "KeyGeneratorInitConfiguration" }
|
||||||
@@ -236,3 +140,62 @@ private class KeyPairGeneratorInitConfiguration extends TaintTracking::Configura
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ! move some/all of below to Encryption.qll or elsewhere?
|
||||||
|
/** The Java class `java.security.spec.ECGenParameterSpec`. */
|
||||||
|
private class ECGenParameterSpec extends RefType {
|
||||||
|
ECGenParameterSpec() { this.hasQualifiedName("java.security.spec", "ECGenParameterSpec") }
|
||||||
|
}
|
||||||
|
|
||||||
|
/** The Java class `java.security.spec.ECGenParameterSpec`. */
|
||||||
|
private class RSAGenParameterSpec extends RefType {
|
||||||
|
RSAGenParameterSpec() { this.hasQualifiedName("java.security.spec", "RSAKeyGenParameterSpec") }
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Returns the key size in the EC algorithm string */
|
||||||
|
bindingset[algorithm]
|
||||||
|
private int getECKeySize(string algorithm) {
|
||||||
|
algorithm.matches("sec%") and // specification such as "secp256r1"
|
||||||
|
result = algorithm.regexpCapture("sec[p|t](\\d+)[a-zA-Z].*", 1).toInt()
|
||||||
|
or
|
||||||
|
algorithm.matches("X9.62%") and //specification such as "X9.62 prime192v2"
|
||||||
|
result = algorithm.regexpCapture("X9\\.62 .*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt()
|
||||||
|
or
|
||||||
|
(algorithm.matches("prime%") or algorithm.matches("c2tnb%")) and //specification such as "prime192v2"
|
||||||
|
result = algorithm.regexpCapture(".*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt()
|
||||||
|
}
|
||||||
|
|
||||||
|
/** The `init` method declared in `javax.crypto.KeyGenerator`. */
|
||||||
|
private class KeyGeneratorInitMethod extends Method {
|
||||||
|
KeyGeneratorInitMethod() {
|
||||||
|
this.getDeclaringType() instanceof KeyGenerator and
|
||||||
|
this.hasName("init")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** The `initialize` method declared in `java.security.KeyPairGenerator`. */
|
||||||
|
private class KeyPairGeneratorInitMethod extends Method {
|
||||||
|
KeyPairGeneratorInitMethod() {
|
||||||
|
this.getDeclaringType() instanceof KeyPairGenerator and
|
||||||
|
this.hasName("initialize")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ******* DATAFLOW ABOVE *************************************************************************
|
||||||
|
// ************************************************************************************************
|
||||||
|
// ************************************************************************************************
|
||||||
|
// ******* OLD/UNUSED OR EXPERIMENTAL CODE BELOW **************************************************
|
||||||
|
class UnsafeSymmetricKeySize extends IntegerLiteral {
|
||||||
|
UnsafeSymmetricKeySize() { this.getIntValue() < 128 }
|
||||||
|
}
|
||||||
|
|
||||||
|
class UnsafeAsymmetricKeySize extends IntegerLiteral {
|
||||||
|
UnsafeAsymmetricKeySize() { this.getIntValue() < 2048 }
|
||||||
|
}
|
||||||
|
// TODO:
|
||||||
|
// ! todo #0a: find a better way to combine the two needed taint-tracking configs so can go back to having a path-graph...
|
||||||
|
// ! todo #0b: possible to combine the 3 dataflow configs?
|
||||||
|
// todo #1: make representation of source that can be shared across the configs
|
||||||
|
// todo #2: make representation of sink that can be shared across the configs
|
||||||
|
// 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 #4: refactor to be more like the Python version? (or not possible because of lack of DataFlow::Node for void method in Java?)
|
||||||
|
|||||||
@@ -0,0 +1,239 @@
|
|||||||
|
import semmle.code.java.security.Encryption
|
||||||
|
import semmle.code.java.dataflow.TaintTracking2
|
||||||
|
import semmle.code.java.dataflow.TaintTracking
|
||||||
|
import semmle.code.java.dataflow.DataFlow
|
||||||
|
|
||||||
|
// TODO:
|
||||||
|
// todo #0: find a better way to combine the two needed taint-tracking configs so can go back to having a path-graph...
|
||||||
|
// todo #1: make representation of source that can be shared across the configs
|
||||||
|
// todo #2: make representation of sink that can be shared across the configs
|
||||||
|
// 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 #4: refactor to be more like the Python version? (or not possible because of lack of DataFlow::Node for void method in Java?)
|
||||||
|
// ******* DATAFLOW BELOW *************************************************************************
|
||||||
|
/**
|
||||||
|
* Asymmetric (RSA, DSA, DH) key length data flow tracking configuration.
|
||||||
|
*/
|
||||||
|
class AsymmetricKeyTrackingConfiguration extends TaintTracking2::Configuration {
|
||||||
|
AsymmetricKeyTrackingConfiguration() { this = "AsymmetricKeyTrackingConfiguration" }
|
||||||
|
|
||||||
|
override predicate isSource(DataFlow::Node source) {
|
||||||
|
exists(ClassInstanceExpr rsaGenParamSpec |
|
||||||
|
rsaGenParamSpec.getConstructedType() instanceof RSAGenParameterSpec and // ! double-check if should just use getType() instead
|
||||||
|
rsaGenParamSpec.getArgument(0).(IntegerLiteral).getIntValue() < 2048 and
|
||||||
|
source.asExpr() = rsaGenParamSpec
|
||||||
|
)
|
||||||
|
or
|
||||||
|
source.asExpr().(IntegerLiteral).getIntValue() < 2048
|
||||||
|
}
|
||||||
|
|
||||||
|
override predicate isSink(DataFlow::Node sink) {
|
||||||
|
exists(MethodAccess ma, VarAccess va |
|
||||||
|
ma.getMethod() instanceof KeyPairGeneratorInitMethod and
|
||||||
|
//ma.getFile().getBaseName().matches("SignatureTest.java") and
|
||||||
|
// va.getVariable()
|
||||||
|
// .getAnAssignedValue()
|
||||||
|
// .(JavaSecurityKeyPairGenerator)
|
||||||
|
// .getAlgoSpec()
|
||||||
|
// .(StringLiteral)
|
||||||
|
// .getValue()
|
||||||
|
// .toUpperCase()
|
||||||
|
// .matches(["RSA", "DSA", "DH"]) and
|
||||||
|
// ma.getQualifier() = va and
|
||||||
|
exists(
|
||||||
|
JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kpgConfig,
|
||||||
|
DataFlow::PathNode source, DataFlow::PathNode dest
|
||||||
|
|
|
||||||
|
jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches(["RSA", "DSA", "DH"]) and
|
||||||
|
source.getNode().asExpr() = jpg and
|
||||||
|
dest.getNode().asExpr() = ma.getQualifier() and
|
||||||
|
kpgConfig.hasFlowPath(source, dest)
|
||||||
|
) and
|
||||||
|
sink.asExpr() = ma.getArgument(0)
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// predicate hasInsufficientKeySize(string msg) { hasShortAsymmetricKeyPair(msg) }
|
||||||
|
// predicate hasShortAsymmetricKeyPair(string msg) {
|
||||||
|
// exists(AsymmetricKeyTrackingConfiguration config1, DataFlow::Node source, DataFlow::Node sink |
|
||||||
|
// config1.hasFlow(source, sink)
|
||||||
|
// ) and
|
||||||
|
// msg = "Key size should be at least 2048 bits for " + "___" + " encryption."
|
||||||
|
// }
|
||||||
|
/**
|
||||||
|
* Asymmetric (EC) key length data flow tracking configuration.
|
||||||
|
*/
|
||||||
|
class AsymmetricECCKeyTrackingConfiguration extends TaintTracking2::Configuration {
|
||||||
|
AsymmetricECCKeyTrackingConfiguration() { this = "AsymmetricECCKeyTrackingConfiguration" }
|
||||||
|
|
||||||
|
override predicate isSource(DataFlow::Node source) {
|
||||||
|
exists(ClassInstanceExpr ecGenParamSpec |
|
||||||
|
getECKeySize(ecGenParamSpec.getArgument(0).(StringLiteral).getValue()) < 256 and // ! can generate EC with just the keysize and not the curve apparently... (based on netty/netty FP example)
|
||||||
|
source.asExpr() = ecGenParamSpec
|
||||||
|
)
|
||||||
|
or
|
||||||
|
source.asExpr().(IntegerLiteral).getIntValue() < 256
|
||||||
|
}
|
||||||
|
|
||||||
|
override predicate isSink(DataFlow::Node sink) {
|
||||||
|
exists(MethodAccess ma, VarAccess va |
|
||||||
|
ma.getMethod() instanceof KeyPairGeneratorInitMethod and
|
||||||
|
//ma.getArgument(0).getType() instanceof ECGenParameterSpec and // ! can generate EC with just the keysize and not the curve apparently... (based on netty/netty FP example)
|
||||||
|
// va.getVariable()
|
||||||
|
// .getAnAssignedValue()
|
||||||
|
// .(JavaSecurityKeyPairGenerator)
|
||||||
|
// .getAlgoSpec()
|
||||||
|
// .(StringLiteral)
|
||||||
|
// .getValue()
|
||||||
|
// .toUpperCase()
|
||||||
|
// .matches(["EC%"]) and
|
||||||
|
// ma.getQualifier() = va and
|
||||||
|
exists(
|
||||||
|
JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kpgConfig,
|
||||||
|
DataFlow::PathNode source, DataFlow::PathNode dest
|
||||||
|
|
|
||||||
|
jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches("EC%") and
|
||||||
|
source.getNode().asExpr() = jpg and
|
||||||
|
dest.getNode().asExpr() = ma.getQualifier() and
|
||||||
|
kpgConfig.hasFlowPath(source, dest)
|
||||||
|
) and
|
||||||
|
sink.asExpr() = ma.getArgument(0)
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Symmetric (AES) key length data flow tracking configuration.
|
||||||
|
*/
|
||||||
|
class SymmetricKeyTrackingConfiguration extends TaintTracking2::Configuration {
|
||||||
|
SymmetricKeyTrackingConfiguration() { this = "SymmetricKeyTrackingConfiguration2" }
|
||||||
|
|
||||||
|
override predicate isSource(DataFlow::Node source) {
|
||||||
|
source.asExpr().(IntegerLiteral).getIntValue() < 128
|
||||||
|
}
|
||||||
|
|
||||||
|
override predicate isSink(DataFlow::Node sink) {
|
||||||
|
exists(MethodAccess ma, VarAccess va |
|
||||||
|
ma.getMethod() instanceof KeyGeneratorInitMethod and
|
||||||
|
// va.getVariable()
|
||||||
|
// .getAnAssignedValue()
|
||||||
|
// .(JavaxCryptoKeyGenerator)
|
||||||
|
// .getAlgoSpec()
|
||||||
|
// .(StringLiteral)
|
||||||
|
// .getValue()
|
||||||
|
// .toUpperCase()
|
||||||
|
// .matches(["AES"]) and
|
||||||
|
// ma.getQualifier() = va and
|
||||||
|
exists(
|
||||||
|
JavaxCryptoKeyGenerator jcg, KeyGeneratorInitConfiguration kgConfig,
|
||||||
|
DataFlow::PathNode source, DataFlow::PathNode dest
|
||||||
|
|
|
||||||
|
jcg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches("AES") and
|
||||||
|
source.getNode().asExpr() = jcg and
|
||||||
|
dest.getNode().asExpr() = ma.getQualifier() and
|
||||||
|
kgConfig.hasFlowPath(source, dest)
|
||||||
|
) and
|
||||||
|
sink.asExpr() = ma.getArgument(0)
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ! below doesn't work for some reason...
|
||||||
|
// predicate hasInsufficientKeySize2(DataFlow::PathNode source, DataFlow::PathNode sink) {
|
||||||
|
// exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink))
|
||||||
|
// or
|
||||||
|
// exists(SymmetricKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink))
|
||||||
|
// }
|
||||||
|
// ******** Need the below for the above ********
|
||||||
|
// ! move to Encryption.qll?
|
||||||
|
/** The Java class `java.security.spec.ECGenParameterSpec`. */
|
||||||
|
private class ECGenParameterSpec extends RefType {
|
||||||
|
ECGenParameterSpec() { this.hasQualifiedName("java.security.spec", "ECGenParameterSpec") }
|
||||||
|
}
|
||||||
|
|
||||||
|
/** The Java class `java.security.spec.ECGenParameterSpec`. */
|
||||||
|
private class RSAGenParameterSpec extends RefType {
|
||||||
|
RSAGenParameterSpec() { this.hasQualifiedName("java.security.spec", "RSAKeyGenParameterSpec") }
|
||||||
|
}
|
||||||
|
|
||||||
|
// ! move to Encryption.qll?
|
||||||
|
/** Returns the key size in the EC algorithm string */
|
||||||
|
bindingset[algorithm]
|
||||||
|
private int getECKeySize(string algorithm) {
|
||||||
|
algorithm.matches("sec%") and // specification such as "secp256r1"
|
||||||
|
result = algorithm.regexpCapture("sec[p|t](\\d+)[a-zA-Z].*", 1).toInt()
|
||||||
|
or
|
||||||
|
algorithm.matches("X9.62%") and //specification such as "X9.62 prime192v2"
|
||||||
|
result = algorithm.regexpCapture("X9\\.62 .*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt()
|
||||||
|
or
|
||||||
|
(algorithm.matches("prime%") or algorithm.matches("c2tnb%")) and //specification such as "prime192v2"
|
||||||
|
result = algorithm.regexpCapture(".*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt()
|
||||||
|
}
|
||||||
|
|
||||||
|
// ! move to Encryption.qll?
|
||||||
|
/** The `init` method declared in `javax.crypto.KeyGenerator`. */
|
||||||
|
private class KeyGeneratorInitMethod extends Method {
|
||||||
|
KeyGeneratorInitMethod() {
|
||||||
|
this.getDeclaringType() instanceof KeyGenerator and
|
||||||
|
this.hasName("init")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ! move to Encryption.qll?
|
||||||
|
/** The `initialize` method declared in `java.security.KeyPairGenerator`. */
|
||||||
|
private class KeyPairGeneratorInitMethod extends Method {
|
||||||
|
KeyPairGeneratorInitMethod() {
|
||||||
|
this.getDeclaringType() instanceof KeyPairGenerator and
|
||||||
|
this.hasName("initialize")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ******* DATAFLOW ABOVE *************************************************************************
|
||||||
|
// ************************************************************************************************
|
||||||
|
// ************************************************************************************************
|
||||||
|
// ************************************************************************************************
|
||||||
|
// ************************************************************************************************
|
||||||
|
// ************************************************************************************************
|
||||||
|
// ******* OLD/UNUSED OR EXPERIMENTAL CODE BELOW **************************************************
|
||||||
|
class UnsafeSymmetricKeySize extends IntegerLiteral {
|
||||||
|
UnsafeSymmetricKeySize() { this.getIntValue() < 128 }
|
||||||
|
}
|
||||||
|
|
||||||
|
class UnsafeAsymmetricKeySize extends IntegerLiteral {
|
||||||
|
UnsafeAsymmetricKeySize() { this.getIntValue() < 2048 }
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Taint configuration tracking flow from a key generator to a `init` method call. */
|
||||||
|
private class KeyGeneratorInitConfiguration extends TaintTracking::Configuration {
|
||||||
|
KeyGeneratorInitConfiguration() { this = "KeyGeneratorInitConfiguration" }
|
||||||
|
|
||||||
|
override predicate isSource(DataFlow::Node source) {
|
||||||
|
source.asExpr() instanceof JavaxCryptoKeyGenerator
|
||||||
|
}
|
||||||
|
|
||||||
|
override predicate isSink(DataFlow::Node sink) {
|
||||||
|
exists(MethodAccess ma |
|
||||||
|
ma.getMethod() instanceof KeyGeneratorInitMethod and
|
||||||
|
sink.asExpr() = ma.getQualifier()
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Taint configuration tracking flow from a keypair generator to
|
||||||
|
* an `initialize` method call.
|
||||||
|
*/
|
||||||
|
private class KeyPairGeneratorInitConfiguration extends TaintTracking::Configuration {
|
||||||
|
KeyPairGeneratorInitConfiguration() { this = "KeyPairGeneratorInitConfiguration" }
|
||||||
|
|
||||||
|
override predicate isSource(DataFlow::Node source) {
|
||||||
|
source.asExpr() instanceof JavaSecurityKeyPairGenerator
|
||||||
|
}
|
||||||
|
|
||||||
|
override predicate isSink(DataFlow::Node sink) {
|
||||||
|
exists(MethodAccess ma |
|
||||||
|
ma.getMethod() instanceof KeyPairGeneratorInitMethod and
|
||||||
|
sink.asExpr() = ma.getQualifier()
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -14,9 +14,11 @@
|
|||||||
import java
|
import java
|
||||||
import semmle.code.java.security.InsufficientKeySizeQuery
|
import semmle.code.java.security.InsufficientKeySizeQuery
|
||||||
|
|
||||||
|
// * Original:
|
||||||
//import DataFlow::PathGraph
|
//import DataFlow::PathGraph
|
||||||
// from Expr e, string msg
|
// from Expr e, string msg
|
||||||
// where hasInsufficientKeySize(e, msg)
|
// where hasInsufficientKeySize(e, msg)
|
||||||
|
// * Test data-flow config with just Asymmetric:
|
||||||
// select e, msg
|
// select e, msg
|
||||||
// from
|
// from
|
||||||
// AsymmetricKeyTrackingConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink,
|
// AsymmetricKeyTrackingConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink,
|
||||||
@@ -26,19 +28,17 @@ import semmle.code.java.security.InsufficientKeySizeQuery
|
|||||||
// cfg2.hasFlowPath(source, sink)
|
// cfg2.hasFlowPath(source, sink)
|
||||||
// select sink.getNode(), source, sink, "The $@ of an asymmetric key should be at least 2048 bits.",
|
// select sink.getNode(), source, sink, "The $@ of an asymmetric key should be at least 2048 bits.",
|
||||||
// sink.getNode(), "size"
|
// sink.getNode(), "size"
|
||||||
// * Use Below
|
// * Data-Flow path-graph with All configs: (but doesn't track algo name properly...)
|
||||||
// from DataFlow::PathNode source, DataFlow::PathNode sink
|
// from DataFlow::PathNode source, DataFlow::PathNode sink
|
||||||
// where exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) //or
|
// where exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) //or
|
||||||
// //exists(AsymmetricECCKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink)) //or
|
// //exists(AsymmetricECCKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink)) //or
|
||||||
// //exists(SymmetricKeyTrackingConfiguration config3 | config3.hasFlowPath(source, sink))
|
// //exists(SymmetricKeyTrackingConfiguration config3 | config3.hasFlowPath(source, sink))
|
||||||
// select sink.getNode(), source, sink, "This $@ is too small, and flows to $@.", source.getNode(),
|
// select sink.getNode(), source, sink, "This $@ is too small, and flows to $@.", source.getNode(),
|
||||||
// "key size", sink.getNode(), "here"
|
// "key size", sink.getNode(), "here"
|
||||||
// * Use Above
|
// * Taint-tracking with kpg to track algo names
|
||||||
// * Use Below for taint-tracking with kpg
|
|
||||||
from DataFlow::Node source, DataFlow::Node sink
|
from DataFlow::Node source, DataFlow::Node sink
|
||||||
where
|
where
|
||||||
exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlow(source, sink)) or
|
exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlow(source, sink)) or
|
||||||
exists(AsymmetricECCKeyTrackingConfiguration config2 | config2.hasFlow(source, sink)) or
|
exists(AsymmetricECCKeyTrackingConfiguration config2 | config2.hasFlow(source, sink)) or
|
||||||
exists(SymmetricKeyTrackingConfiguration config3 | config3.hasFlow(source, sink))
|
exists(SymmetricKeyTrackingConfiguration config3 | config3.hasFlow(source, sink))
|
||||||
select sink, "This $@ is too small and creates a key $@.", source, "key size", sink, "here"
|
select sink, "This $@ is too small and creates a key $@.", source, "key size", sink, "here"
|
||||||
// * Use Above for taint-tracking with kpg
|
|
||||||
|
|||||||
@@ -243,4 +243,12 @@ public class InsufficientKeySizeTest {
|
|||||||
ECGenParameterSpec ecSpec = new ECGenParameterSpec("secp112r1");
|
ECGenParameterSpec ecSpec = new ECGenParameterSpec("secp112r1");
|
||||||
kpg.initialize(ecSpec); // $ hasInsufficientKeySize
|
kpg.initialize(ecSpec); // $ hasInsufficientKeySize
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ToDo testing:
|
||||||
|
// todo #1: add tests for keysize variable passed to specs
|
||||||
|
// ? todo #2: add tests with DH and DSA specs? (or do those specs not make dev specify keysize?)
|
||||||
|
// ? todo #3: add test for retrieving a key from elsewhere?
|
||||||
|
// todo #4: add barrier-guard tests (see FP from OpenIdentityPlatform/OpenAM)
|
||||||
|
// ? todo #5: add tests for updated keysize variable?: e.g. keysize = 1024; keysize += 1024; so when it's used it is correctly 2048.
|
||||||
|
// ? todo #6: consider if some flow paths for keysize variables will be too hard to track how the keysize is updated (e.g. if calling some other method to get keysize, etc....)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user