Skip to content

Commit 6eb58d8

Browse files
Jami CogswellJami Cogswell
Jami Cogswell
authored and
Jami Cogswell
committed
remove dependence on typeFlag
1 parent c61f23b commit 6eb58d8

File tree

1 file changed

+118
-70
lines changed

1 file changed

+118
-70
lines changed

‎java/ql/lib/semmle/code/java/security/InsufficientKeySize.qll

Lines changed: 118 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ private import semmle.code.java.dataflow.DataFlow
88
abstract class InsufficientKeySizeSource extends DataFlow::Node {
99
/** Holds if this source has the specified `state`. */
1010
predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty }
11+
//int getIntValue() { result = this.asExpr().(IntegerLiteral).getIntValue() }
1112
}
1213

1314
/** A sink for an insufficient key size. */
@@ -17,17 +18,17 @@ abstract class InsufficientKeySizeSink extends DataFlow::Node {
1718
}
1819

1920
/** A source for an insufficient key size (asymmetric-non-ec: RSA, DSA, DH). */
20-
private class AsymmetricNonECSource extends InsufficientKeySizeSource {
21-
AsymmetricNonECSource() { getNodeIntValue(this) < 2048 }
21+
private class AsymmetricNonEcSource extends InsufficientKeySizeSource {
22+
AsymmetricNonEcSource() { getNodeIntValue(this) < 2048 }
2223

2324
override predicate hasState(DataFlow::FlowState state) { state = "2048" }
2425
}
2526

2627
/** A source for an insufficient key size (asymmetric-ec: EC%). */
27-
private class AsymmetricECSource extends InsufficientKeySizeSource {
28-
AsymmetricECSource() {
28+
private class AsymmetricEcSource extends InsufficientKeySizeSource {
29+
AsymmetricEcSource() {
2930
getNodeIntValue(this) < 256 or
30-
getECKeySize(this.asExpr().(StringLiteral).getValue()) < 256 // for cases when the key size is embedded in the curve name
31+
getEcKeySize(this.asExpr().(StringLiteral).getValue()) < 256 // for cases when the key size is embedded in the curve name
3132
}
3233

3334
override predicate hasState(DataFlow::FlowState state) { state = "256" }
@@ -40,15 +41,14 @@ private class SymmetricSource extends InsufficientKeySizeSource {
4041
override predicate hasState(DataFlow::FlowState state) { state = "128" }
4142
}
4243

43-
// TODO: use `typeFlag` like with sinks below to include the size numbers in this predicate?
4444
private int getNodeIntValue(DataFlow::Node node) {
4545
result = node.asExpr().(IntegerLiteral).getIntValue()
4646
}
4747

4848
// TODO: check if any other regex should be added based on some MRVA results.
4949
/** Returns the key size in the EC algorithm string */
5050
bindingset[algorithm]
51-
private int getECKeySize(string algorithm) {
51+
private int getEcKeySize(string algorithm) {
5252
algorithm.matches("sec%") and // specification such as "secp256r1"
5353
result = algorithm.regexpCapture("sec[p|t](\\d+)[a-zA-Z].*", 1).toInt()
5454
or
@@ -60,96 +60,144 @@ private int getECKeySize(string algorithm) {
6060
}
6161

6262
/** A sink for an insufficient key size (asymmetric-non-ec). */
63-
private class AsymmetricNonECSink extends InsufficientKeySizeSink {
64-
AsymmetricNonECSink() {
65-
hasKeySizeInInitMethod(this, "asymmetric-non-ec")
63+
private class AsymmetricNonEcSink extends InsufficientKeySizeSink {
64+
AsymmetricNonEcSink() {
65+
// hasKeySizeInInitMethod(this, "asymmetric-non-ec")
66+
// or
67+
//hasKeySizeInSpec(this, "asymmetric-non-ec")
68+
exists(AsymmInitMethodAccess ma, AsymmKeyGen kg |
69+
kg.getAlgoName().matches(["RSA", "DSA", "DH"]) and
70+
DataFlow::localExprFlow(kg, ma.getQualifier()) and
71+
this.asExpr() = ma.getKeySizeArg()
72+
)
6673
or
67-
hasKeySizeInSpec(this)
74+
exists(AsymmetricNonEcSpec s | this.asExpr() = s.getKeySizeArg())
6875
}
6976

7077
override predicate hasState(DataFlow::FlowState state) { state = "2048" }
7178
}
7279

73-
private class AsymmetricNonECSpec extends RefType {
74-
AsymmetricNonECSpec() {
75-
this instanceof RsaKeyGenParameterSpec or
76-
this instanceof DsaGenParameterSpec or
77-
this instanceof DhGenParameterSpec
78-
}
79-
}
80-
8180
/** A sink for an insufficient key size (asymmetric-ec). */
82-
private class AsymmetricECSink extends InsufficientKeySizeSink {
83-
AsymmetricECSink() {
84-
hasKeySizeInInitMethod(this, "asymmetric-ec")
81+
private class AsymmetricEcSink extends InsufficientKeySizeSink {
82+
AsymmetricEcSink() {
83+
// hasKeySizeInInitMethod(this, "asymmetric-ec")
84+
// or
85+
//hasKeySizeInSpec(this, "asymmetric-ec")
86+
exists(AsymmInitMethodAccess ma, AsymmKeyGen kg |
87+
kg.getAlgoName().matches("EC%") and
88+
DataFlow::localExprFlow(kg, ma.getQualifier()) and
89+
this.asExpr() = ma.getKeySizeArg()
90+
)
8591
or
86-
hasKeySizeInSpec(this)
92+
exists(AsymmetricEcSpec s | this.asExpr() = s.getKeySizeArg())
8793
}
8894

8995
override predicate hasState(DataFlow::FlowState state) { state = "256" }
9096
}
9197

9298
/** A sink for an insufficient key size (symmetric). */
9399
private class SymmetricSink extends InsufficientKeySizeSink {
94-
SymmetricSink() { hasKeySizeInInitMethod(this, "symmetric") }
100+
SymmetricSink() {
101+
//hasKeySizeInInitMethod(this, "symmetric")
102+
exists(SymmInitMethodAccess ma, SymmKeyGen kg |
103+
kg.getAlgoName() = "AES" and
104+
DataFlow::localExprFlow(kg, ma.getQualifier()) and
105+
this.asExpr() = ma.getKeySizeArg()
106+
)
107+
}
95108

96109
override predicate hasState(DataFlow::FlowState state) { state = "128" }
97110
}
98111

99112
// 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()...
100113
// TODO: can prbly re-work way using the typeFlag to be better and less repetitive
101-
private predicate hasKeySizeInInitMethod(DataFlow::Node node, string typeFlag) {
102-
exists(MethodAccess ma, JavaxCryptoAlgoSpec jcaSpec |
103-
(
104-
ma.getMethod() instanceof KeyGeneratorInitMethod and typeFlag = "symmetric"
105-
or
106-
ma.getMethod() instanceof KeyPairGeneratorInitMethod and typeFlag.matches("asymmetric%")
107-
) and
108-
(
109-
jcaSpec instanceof JavaxCryptoKeyGenerator and typeFlag = "symmetric"
110-
or
111-
jcaSpec instanceof JavaSecurityKeyPairGenerator and typeFlag.matches("asymmetric%")
112-
) and
113-
(
114-
getAlgoName(jcaSpec) = "AES" and typeFlag = "symmetric"
115-
or
116-
getAlgoName(jcaSpec).matches(["RSA", "DSA", "DH"]) and typeFlag = "asymmetric-non-ec"
117-
or
118-
getAlgoName(jcaSpec).matches("EC%") and typeFlag = "asymmetric-ec"
119-
) and
120-
DataFlow::localExprFlow(jcaSpec, ma.getQualifier()) and
121-
node.asExpr() = ma.getArgument(0)
122-
)
123-
}
124-
125-
// TODO: this predicate is just a poc for more code condensing; redo this
126-
private string getAlgoName(JavaxCryptoAlgoSpec jca) {
127-
result = jca.getAlgoSpec().(StringLiteral).getValue().toUpperCase()
114+
// private predicate hasKeySizeInInitMethod(DataFlow::Node node, string typeFlag) {
115+
// exists(MethodAccess ma, JavaxCryptoAlgoSpec jcaSpec |
116+
// (
117+
// ma.getMethod() instanceof KeyGeneratorInitMethod and typeFlag = "symmetric"
118+
// or
119+
// ma.getMethod() instanceof KeyPairGeneratorInitMethod and typeFlag.matches("asymmetric%")
120+
// ) and
121+
// (
122+
// jcaSpec instanceof JavaxCryptoKeyGenerator and typeFlag = "symmetric"
123+
// or
124+
// jcaSpec instanceof JavaSecurityKeyPairGenerator and typeFlag.matches("asymmetric%")
125+
// ) and
126+
// (
127+
// getAlgoName(jcaSpec) = "AES" and typeFlag = "symmetric"
128+
// or
129+
// getAlgoName(jcaSpec).matches(["RSA", "DSA", "DH"]) and typeFlag = "asymmetric-non-ec"
130+
// or
131+
// getAlgoName(jcaSpec).matches("EC%") and typeFlag = "asymmetric-ec"
132+
// ) and
133+
// DataFlow::localExprFlow(jcaSpec, ma.getQualifier()) and
134+
// node.asExpr() = ma.getArgument(0)
135+
// )
136+
// }
137+
// // TODO: this predicate is just a poc for more code condensing; redo this
138+
// private string getAlgoName(JavaxCryptoAlgoSpec jca) {
139+
// result = jca.getAlgoSpec().(StringLiteral).getValue().toUpperCase()
140+
// }
141+
abstract class InitMethodAccess extends MethodAccess {
142+
Argument getKeySizeArg() { result = this.getArgument(0) }
143+
}
144+
145+
class AsymmInitMethodAccess extends InitMethodAccess {
146+
AsymmInitMethodAccess() { this.getMethod() instanceof KeyPairGeneratorInitMethod }
147+
}
148+
149+
class SymmInitMethodAccess extends InitMethodAccess {
150+
SymmInitMethodAccess() { this.getMethod() instanceof KeyGeneratorInitMethod }
151+
}
152+
153+
abstract class KeyGen extends JavaxCryptoAlgoSpec {
154+
string getAlgoName() { result = this.getAlgoSpec().(StringLiteral).getValue().toUpperCase() }
155+
}
156+
157+
class AsymmKeyGen extends KeyGen {
158+
AsymmKeyGen() { this instanceof JavaSecurityKeyPairGenerator }
159+
160+
// ! this override is repetitive...
161+
override Expr getAlgoSpec() { result = this.(MethodAccess).getArgument(0) }
162+
}
163+
164+
class SymmKeyGen extends KeyGen {
165+
SymmKeyGen() { this instanceof JavaxCryptoKeyGenerator }
166+
167+
// ! this override is repetitive...
168+
override Expr getAlgoSpec() { result = this.(MethodAccess).getArgument(0) }
128169
}
129170

130171
// 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()...
131172
// TODO: can prbly re-work way using the typeFlag to be better and less repetitive...
132-
private predicate hasKeySizeInSpec(DataFlow::Node node) {
133-
exists(ClassInstanceExpr paramSpec |
134-
(
135-
paramSpec.getConstructedType() instanceof AsymmetricNonECSpec //and
136-
or
137-
//typeFlag = "asymmetric-non-ec"
138-
paramSpec.getConstructedType() instanceof EcGenParameterSpec //and
139-
//typeFlag = "asymmetric-ec"
140-
) and
141-
node.asExpr() = paramSpec.getArgument(0)
142-
)
143-
}
144-
145-
// ! use below instead of/in above??
146-
class Spec extends ClassInstanceExpr {
147-
Spec() {
148-
this.getConstructedType() instanceof AsymmetricNonECSpec or
149-
this.getConstructedType() instanceof EcGenParameterSpec
173+
// private predicate hasKeySizeInSpec(DataFlow::Node node, string typeFlag) {
174+
// exists(ClassInstanceExpr paramSpec |
175+
// (
176+
// paramSpec.getConstructedType() instanceof AsymmetricNonEcSpec and
177+
// typeFlag = "asymmetric-non-ec"
178+
// or
179+
// paramSpec.getConstructedType() instanceof EcGenParameterSpec and
180+
// typeFlag = "asymmetric-ec"
181+
// ) and
182+
// node.asExpr() = paramSpec.getArgument(0)
183+
// )
184+
// }
185+
// ! use below instead of/in above?? (actually I don't think I need any of this, can just use AsymmetricNonEcSpec and EcGenParameterSpec directly???)
186+
// Algo spec
187+
abstract class AsymmetricAlgoSpec extends ClassInstanceExpr {
188+
Argument getKeySizeArg() { result = this.getArgument(0) }
189+
}
190+
191+
class AsymmetricNonEcSpec extends AsymmetricAlgoSpec {
192+
AsymmetricNonEcSpec() {
193+
this.getConstructedType() instanceof RsaKeyGenParameterSpec or
194+
this.getConstructedType() instanceof DsaGenParameterSpec or
195+
this.getConstructedType() instanceof DhGenParameterSpec
150196
}
197+
}
151198

152-
Argument getKeySizeArg() { result = this.getArgument(0) }
199+
class AsymmetricEcSpec extends AsymmetricAlgoSpec {
200+
AsymmetricEcSpec() { this.getConstructedType() instanceof EcGenParameterSpec }
153201
}
154202
// TODO:
155203
// todo #0: look into use of specs without keygen objects; should spec not be a sink in these cases?

0 commit comments

Comments
 (0)