hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-03-27 09:48:16 +01:00
Code Issues Packages Projects Releases Wiki Activity

clean-up and update configurations to have specs as sink

Browse Source
This commit is contained in:
Jami Cogswell
2022-10-10 16:18:49 -04:00
parent 0c2cff253f
commit bd76b1fcc0
4 changed files with 99 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -1,24 +1,14 @@
import semmle.code.java.security.Encryption
import semmle.code.java.dataflow.TaintTracking2
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.dataflow.DataFlow2
// ******* DATAFLOW BELOW *************************************************************************
/**
* Asymmetric (RSA, DSA, DH) key length data flow tracking configuration.
*/
class AsymmetricKeyTrackingConfiguration extends DataFlow2::Configuration {
AsymmetricKeyTrackingConfiguration() { this = "AsymmetricKeyTrackingConfiguration" }
class AsymmetricNonECKeyTrackingConfiguration extends DataFlow2::Configuration {
AsymmetricNonECKeyTrackingConfiguration() { this = "AsymmetricNonECKeyTrackingConfiguration" }
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 |
rsaGenParamSpec.getConstructedType() instanceof RSAGenParameterSpec and
rsaGenParamSpec.getArgument(0).(CompileTimeConstantExpr).getIntValue() < 2048 and
source.asExpr() = rsaGenParamSpec
)
or
source.asExpr().(IntegerLiteral).getIntValue() < 2048
}
@@ -34,7 +24,23 @@ class AsymmetricKeyTrackingConfiguration extends DataFlow2::Configuration {
dest.getNode().asExpr() = ma.getQualifier() and
kpgConfig.hasFlowPath(source, dest)
) and
sink.asExpr() = ma.getArgument(0) // ! todo: add spec as a sink
sink.asExpr() = ma.getArgument(0)
)
or
// TODO: combine below three for less duplicated code
exists(ClassInstanceExpr rsaKeyGenParamSpec |
rsaKeyGenParamSpec.getConstructedType() instanceof RSAKeyGenParameterSpec and
sink.asExpr() = rsaKeyGenParamSpec.getArgument(0)
)
or
exists(ClassInstanceExpr dsaGenParamSpec |
dsaGenParamSpec.getConstructedType() instanceof DSAGenParameterSpec and
sink.asExpr() = dsaGenParamSpec.getArgument(0)
)
or
exists(ClassInstanceExpr dhGenParamSpec |
dhGenParamSpec.getConstructedType() instanceof DHGenParameterSpec and
sink.asExpr() = dhGenParamSpec.getArgument(0)
)
}
}
@@ -42,17 +48,12 @@ class AsymmetricKeyTrackingConfiguration extends DataFlow2::Configuration {
/**
* Asymmetric (EC) key length data flow tracking configuration.
*/
class AsymmetricECCKeyTrackingConfiguration extends DataFlow2::Configuration {
AsymmetricECCKeyTrackingConfiguration() { this = "AsymmetricECCKeyTrackingConfiguration" }
class AsymmetricECKeyTrackingConfiguration extends DataFlow2::Configuration {
AsymmetricECKeyTrackingConfiguration() { this = "AsymmetricECKeyTrackingConfiguration" }
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 |
getECKeySize(ecGenParamSpec.getArgument(0).(StringLiteral).getValue()) < 256 and
source.asExpr() = ecGenParamSpec
)
or
source.asExpr().(IntegerLiteral).getIntValue() < 256
source.asExpr().(IntegerLiteral).getIntValue() < 256 or
getECKeySize(source.asExpr().(StringLiteral).getValue()) < 256 // need this for the cases when the key size is embedded in the curve name.
}
override predicate isSink(DataFlow::Node sink) {
@@ -69,6 +70,12 @@ class AsymmetricECCKeyTrackingConfiguration extends DataFlow2::Configuration {
) and
sink.asExpr() = ma.getArgument(0)
)
or
exists(ClassInstanceExpr ecGenParamSpec |
ecGenParamSpec.getConstructedType() instanceof ECGenParameterSpec and
//getECKeySize(ecGenParamSpec.getArgument(0).(StringLiteral).getValue()) < 256 and
sink.asExpr() = ecGenParamSpec.getArgument(0)
)
}
}
@@ -76,7 +83,7 @@ class AsymmetricECCKeyTrackingConfiguration extends DataFlow2::Configuration {
* Symmetric (AES) key length data flow tracking configuration.
*/
class SymmetricKeyTrackingConfiguration extends DataFlow2::Configuration {
SymmetricKeyTrackingConfiguration() { this = "SymmetricKeyTrackingConfiguration2" }
SymmetricKeyTrackingConfiguration() { this = "SymmetricKeyTrackingConfiguration" }
override predicate isSource(DataFlow::Node source) {
source.asExpr().(IntegerLiteral).getIntValue() < 128
@@ -99,14 +106,9 @@ class SymmetricKeyTrackingConfiguration extends DataFlow2::Configuration {
}
}
// ! below predicate doesn't work
// 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 models for the above configs ********
/** Taint configuration tracking flow from a key generator to a `init` method call. */
// ********************** Need the below models for the above configs **********************
// todo: move some/all of below to Encryption.qll or elsewhere?
/** Data flow configuration tracking flow from a key generator to an `init` method call. */
private class KeyGeneratorInitConfiguration extends DataFlow::Configuration {
KeyGeneratorInitConfiguration() { this = "KeyGeneratorInitConfiguration" }
@@ -122,10 +124,7 @@ private class KeyGeneratorInitConfiguration extends DataFlow::Configuration {
}
}
/**
* Taint configuration tracking flow from a keypair generator to
* an `initialize` method call.
*/
/** Data flow configuration tracking flow from a keypair generator to an `initialize` method call. */
private class KeyPairGeneratorInitConfiguration extends DataFlow::Configuration {
KeyPairGeneratorInitConfiguration() { this = "KeyPairGeneratorInitConfiguration" }
@@ -141,28 +140,24 @@ private class KeyPairGeneratorInitConfiguration extends DataFlow::Configuration
}
}
// ! 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") }
/** The Java class `java.security.spec.RSAKeyGenParameterSpec`. */
private class RSAKeyGenParameterSpec extends RefType {
RSAKeyGenParameterSpec() { 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 Java class `java.security.spec.DSAGenParameterSpec`. */
private class DSAGenParameterSpec extends RefType {
DSAGenParameterSpec() { this.hasQualifiedName("java.security.spec", "DSAGenParameterSpec") }
}
/** The Java class `javax.crypto.spec.DHGenParameterSpec`. */
private class DHGenParameterSpec extends RefType {
DHGenParameterSpec() { this.hasQualifiedName("javax.crypto.spec", "DHGenParameterSpec") }
}
/** The `init` method declared in `javax.crypto.KeyGenerator`. */
@@ -181,21 +176,21 @@ private class KeyPairGeneratorInitMethod extends Method {
}
}
/** 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()
}
// ******* 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?)
// todo #4: refactor to be more like the Python (or C#) version? (or not possible because of lack of DataFlow::Node for void method in Java?)