hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-03-26 17:28:29 +01:00
Code Issues Packages Projects Releases Wiki Activity

combine three configs into one

Browse Source
This commit is contained in:
Jami Cogswell
2022-10-13 17:57:56 -04:00
parent e0f0d554cb
commit 2daa3457d7
4 changed files with 79 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -3,30 +3,41 @@
private import semmle.code.java.security.Encryption
private import semmle.code.java.dataflow.DataFlow
// TODO: only update key sizes (and key size strings in one place in the code)
/** A source for an insufficient key size. */
abstract class InsufficientKeySizeSource extends DataFlow::Node { }
abstract class InsufficientKeySizeSource extends DataFlow::Node {
/** Holds if this source has the specified `state`. */
predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty }
}
/** A sink for an insufficient key size. */
abstract class InsufficientKeySizeSink extends DataFlow::Node { }
// TODO: Consider if below 3 sources should be private and if it's possible to only use InsufficientKeySizeSource in the configs
// TODO: add QLDocs if keeping non-private
/** A source for an insufficient key size (asymmetric-non-ec). */
class AsymmetricNonECSource extends InsufficientKeySizeSource {
AsymmetricNonECSource() { getNodeIntValue(this) < 2048 }
abstract class InsufficientKeySizeSink extends DataFlow::Node {
/** Holds if this sink has the specified `state`. */
predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty }
}
/** A source for an insufficient key size (asymmetric-ec). */
class AsymmetricECSource extends InsufficientKeySizeSource {
/** A source for an insufficient key size (asymmetric-non-ec: RSA, DSA, DH). */
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%). */
private class AsymmetricECSource extends InsufficientKeySizeSource {
AsymmetricECSource() {
getNodeIntValue(this) < 256 or
getECKeySize(this.asExpr().(StringLiteral).getValue()) < 256 // need this for the cases when the key size is embedded in the curve name.
getECKeySize(this.asExpr().(StringLiteral).getValue()) < 256 // for cases when the key size is embedded in the curve name
}
override predicate hasState(DataFlow::FlowState state) { state = "256" }
}
/** A source for an insufficient key size (symmetric). */
class SymmetricSource extends InsufficientKeySizeSource {
/** A source for an insufficient key size (symmetric: AES). */
private class SymmetricSource extends InsufficientKeySizeSource {
SymmetricSource() { getNodeIntValue(this) < 128 }
override predicate hasState(DataFlow::FlowState state) { state = "128" }
}
// TODO: use `typeFlag` like with sinks below to include the size numbers in this predicate?
@@ -34,6 +45,7 @@ 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 */
bindingset[algorithm]
private int getECKeySize(string algorithm) {
@@ -47,18 +59,17 @@ private int getECKeySize(string algorithm) {
result = algorithm.regexpCapture(".*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt()
}
// TODO: Consider if below 3 sinks should be private and if it's possible to only use InsufficientKeySizeSink in the configs
// TODO: add QLDocs if keeping non-private
/** A sink for an insufficient key size (asymmetric-non-ec). */
class AsymmetricNonECSink extends InsufficientKeySizeSink {
private class AsymmetricNonECSink extends InsufficientKeySizeSink {
AsymmetricNonECSink() {
hasKeySizeInInitMethod(this, "asymmetric-non-ec")
or
hasKeySizeInSpec(this, "asymmetric-non-ec")
}
override predicate hasState(DataFlow::FlowState state) { state = "2048" }
}
// TODO: move to Encryption.qll? or keep here since specific to this query?
private class AsymmetricNonECSpec extends RefType {
AsymmetricNonECSpec() {
this instanceof RsaKeyGenParameterSpec or
@@ -68,17 +79,21 @@ private class AsymmetricNonECSpec extends RefType {
}
/** A sink for an insufficient key size (asymmetric-ec). */
class AsymmetricECSink extends InsufficientKeySizeSink {
private class AsymmetricECSink extends InsufficientKeySizeSink {
AsymmetricECSink() {
hasKeySizeInInitMethod(this, "asymmetric-ec")
or
hasKeySizeInSpec(this, "asymmetric-ec")
}
override predicate hasState(DataFlow::FlowState state) { state = "256" }
}
/** A sink for an insufficient key size (symmetric). */
class SymmetricSink extends InsufficientKeySizeSink {
private class SymmetricSink extends InsufficientKeySizeSink {
SymmetricSink() { hasKeySizeInInitMethod(this, "symmetric") }
override predicate hasState(DataFlow::FlowState state) { state = "128" }
}
// TODO: rethink the predicate name; also think about whether this could/should be a class instead; or a predicate within the sink class so can do sink.predicate()...
@@ -113,7 +128,7 @@ private string getAlgoName(JavaxCryptoAlgoSpec jca) {
}
// TODO: rethink the predicate name; also think about whether this could/should be a class instead; or a predicate within the sink class so can do sink.predicate()...
// TODO: can prbly re-work way using the typeFlag to be better and less repetitive
// TODO: can prbly re-work way using the typeFlag to be better and less repetitive...
private predicate hasKeySizeInSpec(DataFlow::Node node, string typeFlag) {
exists(ClassInstanceExpr paramSpec |
(
@@ -126,7 +141,8 @@ private predicate hasKeySizeInSpec(DataFlow::Node node, string typeFlag) {
node.asExpr() = paramSpec.getArgument(0)
)
}
class SpecWithKeySize extends RefType { }
// 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 #4: use flowstate (or even just result predicate) to pass source int size to sink?

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

@@ -6,120 +6,43 @@ import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.security.InsufficientKeySize
/**
* A data flow configuration for tracking non-elliptic curve asymmetric algorithms
* A data flow configuration for tracking non-elliptic curve asymmetric algorithm
* (RSA, DSA, and DH) key sizes.
*/
class AsymmetricNonECKeyTrackingConfiguration extends DataFlow::Configuration {
AsymmetricNonECKeyTrackingConfiguration() { this = "AsymmetricNonECKeyTrackingConfiguration" }
class KeySizeConfiguration extends DataFlow::Configuration {
KeySizeConfiguration() { this = "KeySizeConfiguration" }
override predicate isSource(DataFlow::Node source) { source instanceof AsymmetricNonECSource }
override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
source.(InsufficientKeySizeSource).hasState(state)
//source instanceof InsufficientKeySizeSource
}
override predicate isSink(DataFlow::Node sink) { sink instanceof AsymmetricNonECSink }
override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
sink.(InsufficientKeySizeSink).hasState(state)
//sink instanceof InsufficientKeySizeSink
}
}
/**
* A data flow configuration for tracking elliptic curve (EC) asymmetric
* algorithm key sizes.
*/
class AsymmetricECKeyTrackingConfiguration extends DataFlow::Configuration {
AsymmetricECKeyTrackingConfiguration() { this = "AsymmetricECKeyTrackingConfiguration" }
override predicate isSource(DataFlow::Node source) { source instanceof AsymmetricECSource }
override predicate isSink(DataFlow::Node sink) { sink instanceof AsymmetricECSink }
}
/** A data flow configuration for tracking symmetric algorithm (AES) key sizes. */
class SymmetricKeyTrackingConfiguration extends DataFlow::Configuration {
SymmetricKeyTrackingConfiguration() { this = "SymmetricKeyTrackingConfiguration" }
override predicate isSource(DataFlow::Node source) { source instanceof SymmetricSource }
override predicate isSink(DataFlow::Node sink) { sink instanceof SymmetricSink }
}
// ******* 3 DATAFLOW CONFIGS ABOVE *************************************************************************
// ******* SINGLE CONFIG ATTEMPT BELOW *************************************************************************
// /**
// * A key length data flow tracking configuration.
// * A data flow configuration for tracking non-elliptic curve asymmetric algorithm
// * (RSA, DSA, and DH) key sizes.
// */
// class KeyTrackingConfiguration extends DataFlow::Configuration {
// KeyTrackingConfiguration() { this = "KeyTrackingConfiguration" }
// override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
// //state instanceof DataFlow::FlowStateEmpty and
// // SYMMETRIC
// source.asExpr().(IntegerLiteral).getIntValue() < 128 and state = "128"
// or
// // ASYMMETRIC
// source.asExpr().(IntegerLiteral).getIntValue() < 2048 and state = "2048"
// or
// source.asExpr().(IntegerLiteral).getIntValue() < 256 and state = "256"
// or
// getECKeySize(source.asExpr().(StringLiteral).getValue()) < 256 and state = "256" // need this for the cases when the key size is embedded in the curve name.
// }
// override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
// // SYMMETRIC
// exists(MethodAccess ma, JavaxCryptoKeyGenerator jcg |
// ma.getMethod() instanceof KeyGeneratorInitMethod and
// jcg.getAlgoSpec().(StringLiteral).getValue().toUpperCase() = "AES" and
// DataFlow::localExprFlow(jcg, ma.getQualifier()) and
// sink.asExpr() = ma.getArgument(0) and
// state = "128"
// )
// or
// // ASYMMETRIC
// exists(MethodAccess ma, JavaSecurityKeyPairGenerator jpg |
// ma.getMethod() instanceof KeyPairGeneratorInitMethod and
// (
// jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches(["RSA", "DSA", "DH"]) and
// DataFlow::localExprFlow(jpg, ma.getQualifier()) and
// sink.asExpr() = ma.getArgument(0) and
// //ma.getArgument(0).(LocalSourceNode).flowsTo(sink) and
// //ma.getArgument(0).(CompileTimeConstantExpr).getIntValue() < 2048 and
// state = "2048"
// )
// or
// jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches("EC%") and
// DataFlow::localExprFlow(jpg, ma.getQualifier()) and
// sink.asExpr() = ma.getArgument(0) and
// //ma.getArgument(0).(CompileTimeConstantExpr).getIntValue() < 256 and
// state = "256"
// )
// or
// // TODO: combine below three for less duplicated code
// exists(ClassInstanceExpr rsaKeyGenParamSpec |
// rsaKeyGenParamSpec.getConstructedType() instanceof RsaKeyGenParameterSpec and
// sink.asExpr() = rsaKeyGenParamSpec.getArgument(0) and
// state = "2048"
// )
// or
// exists(ClassInstanceExpr dsaGenParamSpec |
// dsaGenParamSpec.getConstructedType() instanceof DsaGenParameterSpec and
// sink.asExpr() = dsaGenParamSpec.getArgument(0) and
// state = "2048"
// )
// or
// exists(ClassInstanceExpr dhGenParamSpec |
// dhGenParamSpec.getConstructedType() instanceof DhGenParameterSpec and
// sink.asExpr() = dhGenParamSpec.getArgument(0) and
// state = "2048"
// )
// or
// exists(ClassInstanceExpr ecGenParamSpec |
// ecGenParamSpec.getConstructedType() instanceof EcGenParameterSpec and
// sink.asExpr() = ecGenParamSpec.getArgument(0) and
// state = "256"
// )
// }
// // ! FlowStates seem to work without even including a step like the below... hmmm
// override predicate isAdditionalFlowStep(
// DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
// DataFlow::FlowState state2
// ) {
// exists(IntegerLiteral intLiteral |
// state1 = "" and
// state2 = intLiteral.toString() and
// node1.asExpr() = intLiteral and
// node2.asExpr() = intLiteral
// )
// }
// class AsymmetricNonECKeyTrackingConfiguration extends DataFlow::Configuration {
// AsymmetricNonECKeyTrackingConfiguration() { this = "AsymmetricNonECKeyTrackingConfiguration" }
// override predicate isSource(DataFlow::Node source) { source instanceof AsymmetricNonECSource }
// override predicate isSink(DataFlow::Node sink) { sink instanceof AsymmetricNonECSink }
// }
// /**
// * A data flow configuration for tracking elliptic curve (EC) asymmetric
// * algorithm key sizes.
// */
// class AsymmetricECKeyTrackingConfiguration extends DataFlow::Configuration {
// AsymmetricECKeyTrackingConfiguration() { this = "AsymmetricECKeyTrackingConfiguration" }
// override predicate isSource(DataFlow::Node source) { source instanceof AsymmetricECSource }
// override predicate isSink(DataFlow::Node sink) { sink instanceof AsymmetricECSink }
// }
// /** A data flow configuration for tracking symmetric algorithm (AES) key sizes. */
// class SymmetricKeyTrackingConfiguration extends DataFlow::Configuration {
// SymmetricKeyTrackingConfiguration() { this = "SymmetricKeyTrackingConfiguration" }
// override predicate isSource(DataFlow::Node source) { source instanceof SymmetricSource }
// override predicate isSink(DataFlow::Node sink) { sink instanceof SymmetricSink }
// }