Skip to content

Commit 7a0555e

Browse files
authored
Merge pull request github#6357 from artem-smotrakov/static-iv
Java: Static initialization vector
2 parents 4e243f9 + 1dd4bf0 commit 7a0555e

File tree

8 files changed

+434
-0
lines changed

8 files changed

+434
-0
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
byte[] iv = new byte[16]; // all zeroes
2+
GCMParameterSpec params = new GCMParameterSpec(128, iv);
3+
Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING");
4+
cipher.init(Cipher.ENCRYPT_MODE, key, params);
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
byte[] iv = new byte[16];
2+
SecureRandom random = SecureRandom.getInstanceStrong();
3+
random.nextBytes(iv);
4+
GCMParameterSpec params = new GCMParameterSpec(128, iv);
5+
Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING");
6+
cipher.init(Cipher.ENCRYPT_MODE, key, params);
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
A cipher needs an initialization vector (IV) when it is used in certain modes
7+
such as CBC or GCM. Under the same secret key, IVs should be unique and ideally unpredictable.
8+
Given a secret key, if the same IV is used for encryption, the same plaintexts result in the same ciphertexts.
9+
This lets an attacker learn if the same data pieces are transferred or stored,
10+
or this can help the attacker run a dictionary attack.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
Use a random IV generated by <code>SecureRandom</code>.
17+
</p>
18+
</recommendation>
19+
20+
<example>
21+
<p>
22+
The following example initializes a cipher with a static IV which is unsafe:
23+
</p>
24+
<sample src="BadStaticInitializationVector.java" />
25+
26+
<p>
27+
The next example initializes a cipher with a random IV:
28+
</p>
29+
<sample src="GoodRandomInitializationVector.java" />
30+
</example>
31+
32+
<references>
33+
<li>
34+
Wikipedia:
35+
<a href="https://en.wikipedia.org/wiki/Initialization_vector">Initialization vector</a>.
36+
</li>
37+
<li>
38+
National Institute of Standards and Technology:
39+
<a href="https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38a.pdf">Recommendation for Block Cipher Modes of Operation</a>.
40+
</li>
41+
<li>
42+
National Institute of Standards and Technology:
43+
<a href="https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.140-2.pdf">FIPS 140-2: Security Requirements for Cryptographic Modules</a>.
44+
</li>
45+
</references>
46+
</qhelp>
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* @name Using a static initialization vector for encryption
3+
* @description A cipher needs an initialization vector (IV) in some cases,
4+
* for example, when CBC or GCM modes are used. IVs are used to randomize the encryption,
5+
* therefore they should be unique and ideally unpredictable.
6+
* Otherwise, the same plaintexts result in same ciphertexts under a given secret key.
7+
* If a static IV is used for encryption, this lets an attacker learn
8+
* if the same data pieces are transferred or stored,
9+
* or this can help the attacker run a dictionary attack.
10+
* @kind path-problem
11+
* @problem.severity warning
12+
* @precision high
13+
* @id java/static-initialization-vector
14+
* @tags security
15+
* external/cwe/cwe-329
16+
* external/cwe/cwe-1204
17+
*/
18+
19+
import java
20+
import experimental.semmle.code.java.security.StaticInitializationVectorQuery
21+
import DataFlow::PathGraph
22+
23+
from DataFlow::PathNode source, DataFlow::PathNode sink, StaticInitializationVectorConfig conf
24+
where conf.hasFlowPath(source, sink)
25+
select sink.getNode(), source, sink, "A $@ should not be used for encryption.", source.getNode(),
26+
"static initialization vector"
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
import java
2+
import semmle.code.java.dataflow.TaintTracking
3+
import semmle.code.java.dataflow.TaintTracking2
4+
5+
/**
6+
* Holds if `array` is initialized only with constants.
7+
*/
8+
private predicate initializedWithConstants(ArrayCreationExpr array) {
9+
// creating an array without an initializer, for example `new byte[8]`
10+
not exists(array.getInit())
11+
or
12+
// creating a multidimensional array with an initializer like `{ new byte[8], new byte[16] }`
13+
// This works around https://github.com/github/codeql/issues/6552 -- change me once there is
14+
// a better way to distinguish nested initializers that create zero-filled arrays
15+
// (e.g. `new byte[1]`) from those with an initializer list (`new byte[] { 1 }` or just `{ 1 }`)
16+
array.getInit().getAnInit().getAChildExpr() instanceof IntegerLiteral
17+
or
18+
// creating an array wit an initializer like `new byte[] { 1, 2 }`
19+
forex(Expr element | element = array.getInit().getAnInit() |
20+
element instanceof CompileTimeConstantExpr
21+
)
22+
}
23+
24+
/**
25+
* An expression that creates a byte array that is initialized with constants.
26+
*/
27+
private class StaticByteArrayCreation extends ArrayCreationExpr {
28+
StaticByteArrayCreation() {
29+
this.getType().(Array).getElementType().(PrimitiveType).getName() = "byte" and
30+
initializedWithConstants(this)
31+
}
32+
}
33+
34+
/** An expression that updates `array`. */
35+
private class ArrayUpdate extends Expr {
36+
Expr array;
37+
38+
ArrayUpdate() {
39+
exists(Assignment assign |
40+
assign = this and
41+
assign.getDest().(ArrayAccess).getArray() = array and
42+
not assign.getSource() instanceof CompileTimeConstantExpr
43+
)
44+
or
45+
exists(StaticMethodAccess ma |
46+
ma.getMethod().hasQualifiedName("java.lang", "System", "arraycopy") and
47+
ma = this and
48+
ma.getArgument(2) = array
49+
)
50+
or
51+
exists(MethodAccess ma, Method m |
52+
m = ma.getMethod() and
53+
ma = this and
54+
ma.getArgument(0) = array
55+
|
56+
m.hasQualifiedName("java.io", "InputStream", "read") or
57+
m.hasQualifiedName("java.nio", "ByteBuffer", "get") or
58+
m.hasQualifiedName("java.security", "SecureRandom", "nextBytes") or
59+
m.hasQualifiedName("java.util", "Random", "nextBytes")
60+
)
61+
}
62+
63+
/** Returns the updated array. */
64+
Expr getArray() { result = array }
65+
}
66+
67+
/**
68+
* A config that tracks dataflow from creating an array to an operation that updates it.
69+
*/
70+
private class ArrayUpdateConfig extends TaintTracking2::Configuration {
71+
ArrayUpdateConfig() { this = "ArrayUpdateConfig" }
72+
73+
override predicate isSource(DataFlow::Node source) {
74+
source.asExpr() instanceof StaticByteArrayCreation
75+
}
76+
77+
override predicate isSink(DataFlow::Node sink) {
78+
exists(ArrayUpdate update | update.getArray() = sink.asExpr())
79+
}
80+
}
81+
82+
/**
83+
* A source that defines an array that doesn't get updated.
84+
*/
85+
private class StaticInitializationVectorSource extends DataFlow::Node {
86+
StaticInitializationVectorSource() {
87+
exists(StaticByteArrayCreation array | array = this.asExpr() |
88+
not exists(ArrayUpdateConfig config | config.hasFlow(DataFlow2::exprNode(array), _))
89+
)
90+
}
91+
}
92+
93+
/**
94+
* A config that tracks initialization of a cipher for encryption.
95+
*/
96+
private class EncryptionModeConfig extends TaintTracking2::Configuration {
97+
EncryptionModeConfig() { this = "EncryptionModeConfig" }
98+
99+
override predicate isSource(DataFlow::Node source) {
100+
source
101+
.asExpr()
102+
.(FieldRead)
103+
.getField()
104+
.hasQualifiedName("javax.crypto", "Cipher", "ENCRYPT_MODE")
105+
}
106+
107+
override predicate isSink(DataFlow::Node sink) {
108+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
109+
m.hasQualifiedName("javax.crypto", "Cipher", "init") and
110+
ma.getArgument(0) = sink.asExpr()
111+
)
112+
}
113+
}
114+
115+
/**
116+
* A sink that initializes a cipher for encryption with unsafe parameters.
117+
*/
118+
private class EncryptionInitializationSink extends DataFlow::Node {
119+
EncryptionInitializationSink() {
120+
exists(MethodAccess ma, Method m, EncryptionModeConfig config | m = ma.getMethod() |
121+
m.hasQualifiedName("javax.crypto", "Cipher", "init") and
122+
m.getParameterType(2)
123+
.(RefType)
124+
.hasQualifiedName("java.security.spec", "AlgorithmParameterSpec") and
125+
ma.getArgument(2) = this.asExpr() and
126+
config.hasFlowToExpr(ma.getArgument(0))
127+
)
128+
}
129+
}
130+
131+
/**
132+
* Holds if `fromNode` to `toNode` is a dataflow step
133+
* that creates cipher's parameters with initialization vector.
134+
*/
135+
private predicate createInitializationVectorSpecStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
136+
exists(ConstructorCall cc, RefType type |
137+
cc = toNode.asExpr() and type = cc.getConstructedType()
138+
|
139+
type.hasQualifiedName("javax.crypto.spec", "IvParameterSpec") and
140+
cc.getArgument(0) = fromNode.asExpr()
141+
or
142+
type.hasQualifiedName("javax.crypto.spec", ["GCMParameterSpec", "RC2ParameterSpec"]) and
143+
cc.getArgument(1) = fromNode.asExpr()
144+
or
145+
type.hasQualifiedName("javax.crypto.spec", "RC5ParameterSpec") and
146+
cc.getArgument(3) = fromNode.asExpr()
147+
)
148+
}
149+
150+
/**
151+
* A config that tracks dataflow to initializing a cipher with a static initialization vector.
152+
*/
153+
class StaticInitializationVectorConfig extends TaintTracking::Configuration {
154+
StaticInitializationVectorConfig() { this = "StaticInitializationVectorConfig" }
155+
156+
override predicate isSource(DataFlow::Node source) {
157+
source instanceof StaticInitializationVectorSource
158+
}
159+
160+
override predicate isSink(DataFlow::Node sink) { sink instanceof EncryptionInitializationSink }
161+
162+
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
163+
createInitializationVectorSpecStep(fromNode, toNode)
164+
}
165+
}

0 commit comments

Comments
 (0)