fix code-scanning bot problems

This commit is contained in:
Jami Cogswell
2022-10-10 23:01:55 -04:00
parent b6a8c27d48
commit e64825ff7a
4 changed files with 22 additions and 67 deletions

View File

@@ -1,9 +1,11 @@
/** Provides classes and predicates related to insufficient key sizes in Java. */
import semmle.code.java.security.Encryption
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.dataflow.DataFlow2
/**
* Asymmetric (RSA, DSA, DH) key length data flow tracking configuration.
* An Asymmetric (RSA, DSA, DH) key length data flow tracking configuration.
*/
class AsymmetricNonECKeyTrackingConfiguration extends DataFlow2::Configuration {
AsymmetricNonECKeyTrackingConfiguration() { this = "AsymmetricNonECKeyTrackingConfiguration" }
@@ -29,24 +31,24 @@ class AsymmetricNonECKeyTrackingConfiguration extends DataFlow2::Configuration {
or
// TODO: combine below three for less duplicated code
exists(ClassInstanceExpr rsaKeyGenParamSpec |
rsaKeyGenParamSpec.getConstructedType() instanceof RSAKeyGenParameterSpec and
rsaKeyGenParamSpec.getConstructedType() instanceof RsaKeyGenParameterSpec and
sink.asExpr() = rsaKeyGenParamSpec.getArgument(0)
)
or
exists(ClassInstanceExpr dsaGenParamSpec |
dsaGenParamSpec.getConstructedType() instanceof DSAGenParameterSpec and
dsaGenParamSpec.getConstructedType() instanceof DsaGenParameterSpec and
sink.asExpr() = dsaGenParamSpec.getArgument(0)
)
or
exists(ClassInstanceExpr dhGenParamSpec |
dhGenParamSpec.getConstructedType() instanceof DHGenParameterSpec and
dhGenParamSpec.getConstructedType() instanceof DhGenParameterSpec and
sink.asExpr() = dhGenParamSpec.getArgument(0)
)
}
}
/**
* Asymmetric (EC) key length data flow tracking configuration.
* An Asymmetric (EC) key length data flow tracking configuration.
*/
class AsymmetricECKeyTrackingConfiguration extends DataFlow2::Configuration {
AsymmetricECKeyTrackingConfiguration() { this = "AsymmetricECKeyTrackingConfiguration" }
@@ -72,7 +74,7 @@ class AsymmetricECKeyTrackingConfiguration extends DataFlow2::Configuration {
)
or
exists(ClassInstanceExpr ecGenParamSpec |
ecGenParamSpec.getConstructedType() instanceof ECGenParameterSpec and
ecGenParamSpec.getConstructedType() instanceof EcGenParameterSpec and
//getECKeySize(ecGenParamSpec.getArgument(0).(StringLiteral).getValue()) < 256 and
sink.asExpr() = ecGenParamSpec.getArgument(0)
)
@@ -80,7 +82,7 @@ class AsymmetricECKeyTrackingConfiguration extends DataFlow2::Configuration {
}
/**
* Symmetric (AES) key length data flow tracking configuration.
* A Symmetric (AES) key length data flow tracking configuration.
*/
class SymmetricKeyTrackingConfiguration extends DataFlow2::Configuration {
SymmetricKeyTrackingConfiguration() { this = "SymmetricKeyTrackingConfiguration" }
@@ -96,7 +98,7 @@ class SymmetricKeyTrackingConfiguration extends DataFlow2::Configuration {
JavaxCryptoKeyGenerator jcg, KeyGeneratorInitConfiguration kgConfig,
DataFlow::PathNode source, DataFlow::PathNode dest
|
jcg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches("AES") and
jcg.getAlgoSpec().(StringLiteral).getValue().toUpperCase() = "AES" and
source.getNode().asExpr() = jcg and
dest.getNode().asExpr() = ma.getQualifier() and
kgConfig.hasFlowPath(source, dest)
@@ -108,7 +110,7 @@ class SymmetricKeyTrackingConfiguration extends DataFlow2::Configuration {
// ********************** 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. */
/** A data flow configuration tracking flow from a key generator to an `init` method call. */
private class KeyGeneratorInitConfiguration extends DataFlow::Configuration {
KeyGeneratorInitConfiguration() { this = "KeyGeneratorInitConfiguration" }
@@ -124,7 +126,7 @@ private class KeyGeneratorInitConfiguration extends DataFlow::Configuration {
}
}
/** Data flow configuration tracking flow from a keypair generator to an `initialize` method call. */
/** A data flow configuration tracking flow from a keypair generator to an `initialize` method call. */
private class KeyPairGeneratorInitConfiguration extends DataFlow::Configuration {
KeyPairGeneratorInitConfiguration() { this = "KeyPairGeneratorInitConfiguration" }
@@ -141,23 +143,23 @@ private class KeyPairGeneratorInitConfiguration extends DataFlow::Configuration
}
/** The Java class `java.security.spec.ECGenParameterSpec`. */
private class ECGenParameterSpec extends RefType {
ECGenParameterSpec() { this.hasQualifiedName("java.security.spec", "ECGenParameterSpec") }
private class EcGenParameterSpec extends RefType {
EcGenParameterSpec() { this.hasQualifiedName("java.security.spec", "ECGenParameterSpec") }
}
/** The Java class `java.security.spec.RSAKeyGenParameterSpec`. */
private class RSAKeyGenParameterSpec extends RefType {
RSAKeyGenParameterSpec() { this.hasQualifiedName("java.security.spec", "RSAKeyGenParameterSpec") }
private class RsaKeyGenParameterSpec extends RefType {
RsaKeyGenParameterSpec() { this.hasQualifiedName("java.security.spec", "RSAKeyGenParameterSpec") }
}
/** The Java class `java.security.spec.DSAGenParameterSpec`. */
private class DSAGenParameterSpec extends RefType {
DSAGenParameterSpec() { this.hasQualifiedName("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") }
private class DhGenParameterSpec extends RefType {
DhGenParameterSpec() { this.hasQualifiedName("javax.crypto.spec", "DHGenParameterSpec") }
}
/** The `init` method declared in `javax.crypto.KeyGenerator`. */
@@ -190,6 +192,7 @@ private int getECKeySize(string algorithm) {
}
// ******* DATAFLOW ABOVE *************************************************************************
// TODO:
// todo #0: look into use of specs without keygens; should spec not be a sink in these cases?
// 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.)

View File

@@ -19,4 +19,4 @@ where
exists(AsymmetricNonECKeyTrackingConfiguration config1 | config1.hasFlow(source, sink)) or
exists(AsymmetricECKeyTrackingConfiguration config2 | config2.hasFlow(source, sink)) or
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.", source, "key size"

View File

@@ -1,37 +0,0 @@
public class InsufficientKeySize {
public void CryptoMethod() {
KeyGenerator keyGen1 = KeyGenerator.getInstance("AES");
// BAD: Key size is less than 128
keyGen1.init(64);
KeyGenerator keyGen2 = KeyGenerator.getInstance("AES");
// GOOD: Key size is no less than 128
keyGen2.init(128);
KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("RSA");
// BAD: Key size is less than 2048
keyPairGen1.initialize(1024);
KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("RSA");
// GOOD: Key size is no less than 2048
keyPairGen2.initialize(2048);
KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("DSA");
// BAD: Key size is less than 2048
keyPairGen3.initialize(1024);
KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("DSA");
// GOOD: Key size is no less than 2048
keyPairGen4.initialize(2048);
KeyPairGenerator keyPairGen5 = KeyPairGenerator.getInstance("EC");
// BAD: Key size is less than 256
ECGenParameterSpec ecSpec1 = new ECGenParameterSpec("secp112r1");
keyPairGen5.initialize(ecSpec1);
KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("EC");
// GOOD: Key size is no less than 256
ECGenParameterSpec ecSpec2 = new ECGenParameterSpec("secp256r1");
keyPairGen6.initialize(ecSpec2);
}
}

View File

@@ -1,11 +0,0 @@
| InsufficientKeySize.java:9:9:9:24 | init(...) | Key size should be at least 128 bits for AES encryption. |
| InsufficientKeySize.java:17:9:17:36 | initialize(...) | Key size should be at least 2048 bits for RSA encryption. |
| InsufficientKeySize.java:25:9:25:36 | initialize(...) | Key size should be at least 2048 bits for DSA encryption. |
| InsufficientKeySize.java:34:9:34:39 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
| InsufficientKeySize.java:38:9:38:67 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
| InsufficientKeySize.java:48:9:48:39 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
| InsufficientKeySize.java:53:9:53:39 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
| InsufficientKeySize.java:58:9:58:40 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
| InsufficientKeySize.java:68:9:68:40 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
| InsufficientKeySize.java:78:9:78:40 | initialize(...) | Key size should be at least 256 bits for EC encryption. |
| InsufficientKeySize.java:87:9:87:37 | initialize(...) | Key size should be at least 2048 bits for DH encryption. |