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-12 16:58:34 -04:00
parent 37d85587e0
commit bfbb6db436
3 changed files with 59 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -26,6 +26,7 @@ class SymmetricSource extends InsufficientKeySizeSource {
SymmetricSource() { getNodeIntValue(this) < 128 }
}
// TODO: use `typeFlag` like with sinks below to include the size numbers in this predicate?
private int getNodeIntValue(DataFlow::Node node) {
result = node.asExpr().(IntegerLiteral).getIntValue()
}
@@ -43,19 +44,13 @@ 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
class AsymmetricNonECSink extends InsufficientKeySizeSink {
AsymmetricNonECSink() {
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
this.asExpr() = ma.getArgument(0)
)
hasKeySizeInInitMethod(this, "asymmetric-non-ec")
or
exists(ClassInstanceExpr genParamSpec |
genParamSpec.getConstructedType() instanceof AsymmetricNonECSpec and
this.asExpr() = genParamSpec.getArgument(0)
)
hasKeySizeInSpec(this, "asymmetric-non-ec")
}
}
@@ -70,31 +65,60 @@ private class AsymmetricNonECSpec extends RefType {
class AsymmetricECSink extends InsufficientKeySizeSink {
AsymmetricECSink() {
exists(MethodAccess ma, JavaSecurityKeyPairGenerator jpg |
ma.getMethod() instanceof KeyPairGeneratorInitMethod and
jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches("EC%") and
DataFlow::localExprFlow(jpg, ma.getQualifier()) and
this.asExpr() = ma.getArgument(0)
)
hasKeySizeInInitMethod(this, "asymmetric-ec")
or
exists(ClassInstanceExpr ecGenParamSpec |
ecGenParamSpec.getConstructedType() instanceof EcGenParameterSpec and
this.asExpr() = ecGenParamSpec.getArgument(0)
)
hasKeySizeInSpec(this, "asymmetric-ec")
}
}
class SymmetricSink extends InsufficientKeySizeSink {
SymmetricSink() {
exists(MethodAccess ma, JavaxCryptoKeyGenerator jcg |
ma.getMethod() instanceof KeyGeneratorInitMethod and
jcg.getAlgoSpec().(StringLiteral).getValue().toUpperCase() = "AES" and
DataFlow::localExprFlow(jcg, ma.getQualifier()) and
this.asExpr() = ma.getArgument(0)
)
}
SymmetricSink() { hasKeySizeInInitMethod(this, "symmetric") }
}
// 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
private predicate hasKeySizeInInitMethod(DataFlow::Node node, string typeFlag) {
exists(MethodAccess ma, JavaxCryptoAlgoSpec jcaSpec |
(
ma.getMethod() instanceof KeyGeneratorInitMethod and typeFlag = "symmetric"
or
ma.getMethod() instanceof KeyPairGeneratorInitMethod and typeFlag.matches("asymmetric%")
) and
(
jcaSpec instanceof JavaxCryptoKeyGenerator and typeFlag = "symmetric"
or
jcaSpec instanceof JavaSecurityKeyPairGenerator and typeFlag.matches("asymmetric%")
) and
(
jcaSpec.getAlgoSpec().(StringLiteral).getValue().toUpperCase() = "AES" and
typeFlag = "symmetric"
or
jcaSpec.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches(["RSA", "DSA", "DH"]) and
typeFlag = "asymmetric-non-ec"
or
jcaSpec.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches("EC%") and
typeFlag = "asymmetric-ec"
) and
DataFlow::localExprFlow(jcaSpec, ma.getQualifier()) and
node.asExpr() = ma.getArgument(0)
)
}
// 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
private predicate hasKeySizeInSpec(DataFlow::Node node, string typeFlag) {
exists(ClassInstanceExpr paramSpec |
(
paramSpec.getConstructedType() instanceof AsymmetricNonECSpec and
typeFlag = "asymmetric-non-ec"
or
paramSpec.getConstructedType() instanceof EcGenParameterSpec and
typeFlag = "asymmetric-ec"
) and
node.asExpr() = paramSpec.getArgument(0)
)
}
// 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 #5:
// todo #4: use flowstate (or even just result predicate) to pass source int size to sink?

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

@@ -19,7 +19,7 @@ from DataFlow::PathNode source, DataFlow::PathNode sink
where
//exists(KeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink))
//or
exists(AsymmetricNonECKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) or
exists(AsymmetricECKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink)) or
exists(SymmetricKeyTrackingConfiguration config3 | config3.hasFlowPath(source, sink))
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"

6
java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.ql
View File

@@ -13,9 +13,9 @@ class InsufficientKeySizeTest extends InlineExpectationsTest {
exists(DataFlow::PathNode source, DataFlow::PathNode sink |
//exists(KeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink))
//or
exists(AsymmetricNonECKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) or
exists(AsymmetricECKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink)) or
exists(SymmetricKeyTrackingConfiguration config3 | config3.hasFlowPath(source, sink))
exists(AsymmetricNonECKeyTrackingConfiguration cfg | cfg.hasFlowPath(source, sink)) or
exists(AsymmetricECKeyTrackingConfiguration cfg | cfg.hasFlowPath(source, sink)) or
exists(SymmetricKeyTrackingConfiguration cfg | cfg.hasFlowPath(source, sink))
|
sink.getNode().getLocation() = location and
element = sink.getNode().toString() and