Skip to content

Commit cfbaf5e

Browse files
jcogs33jcogs33
authored
Merge pull request github#10785 from jcogs33/insuff-key-size-globalflow-keysize
Java: Promote insufficient key size query from experimental
2 parents 53b7584 + 910eebc commit cfbaf5e

17 files changed

+681
-326
lines changed

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

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,22 @@ class KeyPairGenerator extends RefType {
8888
KeyPairGenerator() { this.hasQualifiedName("java.security", "KeyPairGenerator") }
8989
}
9090

91+
/** The `init` method declared in `javax.crypto.KeyGenerator`. */
92+
class KeyGeneratorInitMethod extends Method {
93+
KeyGeneratorInitMethod() {
94+
this.getDeclaringType() instanceof KeyGenerator and
95+
this.hasName("init")
96+
}
97+
}
98+
99+
/** The `initialize` method declared in `java.security.KeyPairGenerator`. */
100+
class KeyPairGeneratorInitMethod extends Method {
101+
KeyPairGeneratorInitMethod() {
102+
this.getDeclaringType() instanceof KeyPairGenerator and
103+
this.hasName("initialize")
104+
}
105+
}
106+
91107
/** The `verify` method of the class `javax.net.ssl.HostnameVerifier`. */
92108
class HostnameVerifierVerify extends Method {
93109
HostnameVerifierVerify() {
@@ -367,8 +383,8 @@ class JavaSecuritySignature extends JavaSecurityAlgoSpec {
367383
override Expr getAlgoSpec() { result = this.(ConstructorCall).getArgument(0) }
368384
}
369385

370-
/** A method call to the Java class `java.security.KeyPairGenerator`. */
371-
class JavaSecurityKeyPairGenerator extends JavaxCryptoAlgoSpec {
386+
/** A call to the `getInstance` method declared in `java.security.KeyPairGenerator`. */
387+
class JavaSecurityKeyPairGenerator extends JavaSecurityAlgoSpec {
372388
JavaSecurityKeyPairGenerator() {
373389
exists(Method m | m.getAReference() = this |
374390
m.getDeclaringType() instanceof KeyPairGenerator and
@@ -378,3 +394,53 @@ class JavaSecurityKeyPairGenerator extends JavaxCryptoAlgoSpec {
378394

379395
override Expr getAlgoSpec() { result = this.(MethodAccess).getArgument(0) }
380396
}
397+
398+
/** The Java class `java.security.AlgorithmParameterGenerator`. */
399+
class AlgorithmParameterGenerator extends RefType {
400+
AlgorithmParameterGenerator() {
401+
this.hasQualifiedName("java.security", "AlgorithmParameterGenerator")
402+
}
403+
}
404+
405+
/** The `init` method declared in `java.security.AlgorithmParameterGenerator`. */
406+
class AlgoParamGeneratorInitMethod extends Method {
407+
AlgoParamGeneratorInitMethod() {
408+
this.getDeclaringType() instanceof AlgorithmParameterGenerator and
409+
this.hasName("init")
410+
}
411+
}
412+
413+
/** A call to the `getInstance` method declared in `java.security.AlgorithmParameterGenerator`. */
414+
class JavaSecurityAlgoParamGenerator extends JavaSecurityAlgoSpec {
415+
JavaSecurityAlgoParamGenerator() {
416+
exists(Method m | m.getAReference() = this |
417+
m.getDeclaringType() instanceof AlgorithmParameterGenerator and
418+
m.getName() = "getInstance"
419+
)
420+
}
421+
422+
override Expr getAlgoSpec() { result = this.(MethodAccess).getArgument(0) }
423+
}
424+
425+
/** An implementation of the `java.security.spec.AlgorithmParameterSpec` interface. */
426+
abstract class AlgorithmParameterSpec extends RefType { }
427+
428+
/** The Java class `java.security.spec.ECGenParameterSpec`. */
429+
class EcGenParameterSpec extends AlgorithmParameterSpec {
430+
EcGenParameterSpec() { this.hasQualifiedName("java.security.spec", "ECGenParameterSpec") }
431+
}
432+
433+
/** The Java class `java.security.spec.RSAKeyGenParameterSpec`. */
434+
class RsaKeyGenParameterSpec extends AlgorithmParameterSpec {
435+
RsaKeyGenParameterSpec() { this.hasQualifiedName("java.security.spec", "RSAKeyGenParameterSpec") }
436+
}
437+
438+
/** The Java class `java.security.spec.DSAGenParameterSpec`. */
439+
class DsaGenParameterSpec extends AlgorithmParameterSpec {
440+
DsaGenParameterSpec() { this.hasQualifiedName("java.security.spec", "DSAGenParameterSpec") }
441+
}
442+
443+
/** The Java class `javax.crypto.spec.DHGenParameterSpec`. */
444+
class DhGenParameterSpec extends AlgorithmParameterSpec {
445+
DhGenParameterSpec() { this.hasQualifiedName("javax.crypto.spec", "DHGenParameterSpec") }
446+
}

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

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
/** Provides classes and predicates related to insufficient key sizes in Java. */
2+
3+
private import semmle.code.java.security.Encryption
4+
private import semmle.code.java.dataflow.DataFlow
5+
6+
/** A source for an insufficient key size. */
7+
abstract class InsufficientKeySizeSource extends DataFlow::Node {
8+
/** Holds if this source has the specified `state`. */
9+
predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty }
10+
}
11+
12+
/** A sink for an insufficient key size. */
13+
abstract class InsufficientKeySizeSink extends DataFlow::Node {
14+
/** Holds if this sink has the specified `state`. */
15+
predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty }
16+
}
17+
18+
/** Provides models for asymmetric cryptography. */
19+
private module Asymmetric {
20+
/** Provides models for non-elliptic-curve asymmetric cryptography. */
21+
private module NonEllipticCurve {
22+
/** A source for an insufficient key size used in RSA, DSA, and DH algorithms. */
23+
private class Source extends InsufficientKeySizeSource {
24+
Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() }
25+
26+
override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() }
27+
}
28+
29+
/** A sink for an insufficient key size used in RSA, DSA, and DH algorithms. */
30+
private class Sink extends InsufficientKeySizeSink {
31+
Sink() {
32+
exists(KeyPairGenInit kpgInit, KeyPairGen kpg |
33+
kpg.getAlgoName().matches(["RSA", "DSA", "DH"]) and
34+
DataFlow::localExprFlow(kpg, kpgInit.getQualifier()) and
35+
this.asExpr() = kpgInit.getKeySizeArg()
36+
)
37+
or
38+
exists(Spec spec | this.asExpr() = spec.getKeySizeArg())
39+
}
40+
41+
override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() }
42+
}
43+
44+
/** Returns the minimum recommended key size for RSA, DSA, and DH algorithms. */
45+
private int getMinKeySize() { result = 2048 }
46+
47+
/** An instance of an RSA, DSA, or DH algorithm specification. */
48+
private class Spec extends ClassInstanceExpr {
49+
Spec() {
50+
this.getConstructedType() instanceof RsaKeyGenParameterSpec or
51+
this.getConstructedType() instanceof DsaGenParameterSpec or
52+
this.getConstructedType() instanceof DhGenParameterSpec
53+
}
54+
55+
/** Gets the `keysize` argument of this instance. */
56+
Argument getKeySizeArg() { result = this.getArgument(0) }
57+
}
58+
}
59+
60+
/** Provides models for elliptic-curve asymmetric cryptography. */
61+
private module EllipticCurve {
62+
/** A source for an insufficient key size used in elliptic curve (EC) algorithms. */
63+
private class Source extends InsufficientKeySizeSource {
64+
Source() {
65+
this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize()
66+
or
67+
// the below is needed for cases when the key size is embedded in the curve name
68+
getKeySize(this.asExpr().(StringLiteral).getValue()) < getMinKeySize()
69+
}
70+
71+
override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() }
72+
}
73+
74+
/** A sink for an insufficient key size used in elliptic curve (EC) algorithms. */
75+
private class Sink extends InsufficientKeySizeSink {
76+
Sink() {
77+
exists(KeyPairGenInit kpgInit, KeyPairGen kpg |
78+
kpg.getAlgoName().matches("EC%") and
79+
DataFlow::localExprFlow(kpg, kpgInit.getQualifier()) and
80+
this.asExpr() = kpgInit.getKeySizeArg()
81+
)
82+
or
83+
exists(Spec s | this.asExpr() = s.getKeySizeArg())
84+
}
85+
86+
override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() }
87+
}
88+
89+
/** Returns the minimum recommended key size for elliptic curve (EC) algorithms. */
90+
private int getMinKeySize() { result = 256 }
91+
92+
/** Returns the key size from an EC algorithm's curve name string */
93+
bindingset[algorithm]
94+
private int getKeySize(string algorithm) {
95+
algorithm.matches("sec%") and // specification such as "secp256r1"
96+
result = algorithm.regexpCapture("sec[p|t](\\d+)[a-zA-Z].*", 1).toInt()
97+
or
98+
algorithm.matches("X9.62%") and // specification such as "X9.62 prime192v2"
99+
result = algorithm.regexpCapture("X9\\.62 .*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt()
100+
or
101+
algorithm.matches(["prime%", "c2tnb%"]) and // specification such as "prime192v2"
102+
result = algorithm.regexpCapture(".*[a-zA-Z](\\d+)[a-zA-Z].*", 1).toInt()
103+
}
104+
105+
/** An instance of an elliptic curve (EC) algorithm specification. */
106+
private class Spec extends ClassInstanceExpr {
107+
Spec() { this.getConstructedType() instanceof EcGenParameterSpec }
108+
109+
/** Gets the `keysize` argument of this instance. */
110+
Argument getKeySizeArg() { result = this.getArgument(0) }
111+
}
112+
}
113+
114+
/**
115+
* A call to the `initialize` method declared in `java.security.KeyPairGenerator`
116+
* or to the `init` method declared in `java.security.AlgorithmParameterGenerator`.
117+
*/
118+
private class KeyPairGenInit extends MethodAccess {
119+
KeyPairGenInit() {
120+
this.getMethod() instanceof KeyPairGeneratorInitMethod or
121+
this.getMethod() instanceof AlgoParamGeneratorInitMethod
122+
}
123+
124+
/** Gets the `keysize` argument of this call. */
125+
Argument getKeySizeArg() { result = this.getArgument(0) }
126+
}
127+
128+
/**
129+
* An instance of a `java.security.KeyPairGenerator`
130+
* or of a `java.security.AlgorithmParameterGenerator`.
131+
*/
132+
private class KeyPairGen extends GeneratorAlgoSpec {
133+
KeyPairGen() {
134+
this instanceof JavaSecurityKeyPairGenerator or
135+
this instanceof JavaSecurityAlgoParamGenerator
136+
}
137+
138+
override Expr getAlgoSpec() {
139+
result =
140+
[
141+
this.(JavaSecurityKeyPairGenerator).getAlgoSpec(),
142+
this.(JavaSecurityAlgoParamGenerator).getAlgoSpec()
143+
]
144+
}
145+
}
146+
}
147+
148+
/** Provides models for symmetric cryptography. */
149+
private module Symmetric {
150+
/** A source for an insufficient key size used in AES algorithms. */
151+
private class Source extends InsufficientKeySizeSource {
152+
Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() }
153+
154+
override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() }
155+
}
156+
157+
/** A sink for an insufficient key size used in AES algorithms. */
158+
private class Sink extends InsufficientKeySizeSink {
159+
Sink() {
160+
exists(KeyGenInit kgInit, KeyGen kg |
161+
kg.getAlgoName() = "AES" and
162+
DataFlow::localExprFlow(kg, kgInit.getQualifier()) and
163+
this.asExpr() = kgInit.getKeySizeArg()
164+
)
165+
}
166+
167+
override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() }
168+
}
169+
170+
/** Returns the minimum recommended key size for AES algorithms. */
171+
private int getMinKeySize() { result = 128 }
172+
173+
/** A call to the `init` method declared in `javax.crypto.KeyGenerator`. */
174+
private class KeyGenInit extends MethodAccess {
175+
KeyGenInit() { this.getMethod() instanceof KeyGeneratorInitMethod }
176+
177+
/** Gets the `keysize` argument of this call. */
178+
Argument getKeySizeArg() { result = this.getArgument(0) }
179+
}
180+
181+
/** An instance of a `javax.crypto.KeyGenerator`. */
182+
private class KeyGen extends GeneratorAlgoSpec instanceof JavaxCryptoKeyGenerator {
183+
override Expr getAlgoSpec() { result = JavaxCryptoKeyGenerator.super.getAlgoSpec() }
184+
}
185+
}
186+
187+
/** An instance of a generator that specifies an encryption algorithm. */
188+
abstract private class GeneratorAlgoSpec extends CryptoAlgoSpec {
189+
/** Returns an uppercase string representing the algorithm name specified by this generator object. */
190+
string getAlgoName() {
191+
result = this.getAlgoSpec().(CompileTimeConstantExpr).getStringValue().toUpperCase()
192+
}
193+
}

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/** Provides data flow configurations to be used in queries related to insufficient key sizes. */
2+
3+
import semmle.code.java.dataflow.DataFlow
4+
import semmle.code.java.security.InsufficientKeySize
5+
6+
/** A data flow configuration for tracking key sizes used in cryptographic algorithms. */
7+
class KeySizeConfiguration extends DataFlow::Configuration {
8+
KeySizeConfiguration() { this = "KeySizeConfiguration" }
9+
10+
override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
11+
source.(InsufficientKeySizeSource).hasState(state)
12+
}
13+
14+
override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
15+
sink.(InsufficientKeySizeSink).hasState(state)
16+
}
17+
}

‎java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.qhelp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Modern encryption relies on the computational infeasibility of breaking a cipher and decoding its
8+
message without the key. As computational power increases, the ability to break ciphers grows, and key
9+
sizes need to become larger as a result. Cryptographic algorithms that use too small of a key size are
10+
vulnerable to brute force attacks, which can reveal sensitive data.</p>
11+
</overview>
12+
13+
<recommendation>
14+
<p>Use a key of the recommended size or larger. The key size should be at least 128 bits for AES encryption,
15+
256 bits for elliptic-curve cryptography (ECC), and 2048 bits for RSA, DSA, or DH encryption.</p>
16+
</recommendation>
17+
18+
<example>
19+
20+
<p>
21+
The following code uses cryptographic algorithms with insufficient key sizes.
22+
</p>
23+
24+
<sample src="InsufficientKeySizeBad.java" />
25+
26+
<p>
27+
To fix the code, change the key sizes to be the recommended size or
28+
larger for each algorithm.
29+
</p>
30+
31+
</example>
32+
33+
<references>
34+
<li>
35+
Wikipedia:
36+
<a href="http://en.wikipedia.org/wiki/Key_size">Key size</a>.
37+
</li>
38+
<li>
39+
Wikipedia: <a href="https://en.wikipedia.org/wiki/Strong_cryptography">Strong cryptography</a>.
40+
</li>
41+
<li>
42+
OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html#algorithms">
43+
Cryptographic Storage Cheat Sheet</a>.
44+
</li>
45+
<li>
46+
OWASP: <a href="https://owasp.org/www-project-web-security-testing-guide/stable/4-Web_Application_Security_Testing/09-Testing_for_Weak_Cryptography/04-Testing_for_Weak_Encryption">
47+
Testing for Weak Encryption</a>.
48+
</li>
49+
<li>
50+
NIST:
51+
<a href="https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar2.pdf">
52+
Transitioning the Use of Cryptographic Algorithms and Key Lengths</a>.
53+
</li>
54+
</references>
55+
</qhelp>

‎java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* @name Use of a cryptographic algorithm with insufficient key size
3+
* @description Using cryptographic algorithms with too small a key size can
4+
* allow an attacker to compromise security.
5+
* @kind path-problem
6+
* @problem.severity warning
7+
* @security-severity 7.5
8+
* @precision high
9+
* @id java/insufficient-key-size
10+
* @tags security
11+
* external/cwe/cwe-326
12+
*/
13+
14+
import java
15+
import semmle.code.java.security.InsufficientKeySizeQuery
16+
import DataFlow::PathGraph
17+
18+
from DataFlow::PathNode source, DataFlow::PathNode sink, KeySizeConfiguration cfg
19+
where cfg.hasFlowPath(source, sink)
20+
select sink.getNode(), source, sink,
21+
"This $@ is less than the recommended key size of " + source.getState() + " bits.",
22+
source.getNode(), "key size"

‎java/ql/src/Security/CWE/CWE-326/InsufficientKeySizeBad.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
KeyPairGenerator keyPairGen1 = KeyPairGenerator.getInstance("RSA");
2+
keyPairGen1.initialize(1024); // BAD: Key size is less than 2048
3+
4+
KeyPairGenerator keyPairGen2 = KeyPairGenerator.getInstance("DSA");
5+
keyPairGen2.initialize(1024); // BAD: Key size is less than 2048
6+
7+
KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("DH");
8+
keyPairGen3.initialize(1024); // BAD: Key size is less than 2048
9+
10+
KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("EC");
11+
ECGenParameterSpec ecSpec = new ECGenParameterSpec("secp112r1"); // BAD: Key size is less than 256
12+
keyPairGen4.initialize(ecSpec);
13+
14+
KeyGenerator keyGen = KeyGenerator.getInstance("AES");
15+
keyGen.init(64); // BAD: Key size is less than 128

‎java/ql/src/change-notes/2022-10-19-insufficient-key-size.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* The query `java/insufficient-key-size` has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @luchua-bc](https://github.com/github/codeql/pull/4926).

0 commit comments

Comments
 (0)