Skip to content

Commit cfe74b5

Browse files
artem-smotrakovartem-smotrakov
committed
Use inline-expectation tests for StaticInitializationVector.ql
1 parent 218731c commit cfe74b5

File tree

7 files changed

+190
-184
lines changed

7 files changed

+190
-184
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-1204/StaticInitializationVector.ql‎

Lines changed: 1 addition & 165 deletions
Original file line numberDiff line numberDiff line change
@@ -17,173 +17,9 @@
1717
*/
1818

1919
import java
20-
import semmle.code.java.dataflow.TaintTracking
21-
import semmle.code.java.dataflow.TaintTracking2
20+
import experimental.semmle.code.java.security.StaticInitializationVectorQuery
2221
import DataFlow::PathGraph
2322

24-
/**
25-
* Holds if `array` is initialized only with constants, for example,
26-
* `new byte[8]` or `new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 }`.
27-
*/
28-
private predicate initializedWithConstants(ArrayCreationExpr array) {
29-
not exists(array.getInit())
30-
or
31-
forex(Expr element | element = array.getInit().getAChildExpr() |
32-
element instanceof CompileTimeConstantExpr
33-
)
34-
}
35-
36-
/**
37-
* An expression that creates a byte array that is initialized with constants.
38-
*/
39-
private class StaticByteArrayCreation extends ArrayCreationExpr {
40-
StaticByteArrayCreation() {
41-
this.getType().(Array).getElementType().(PrimitiveType).getName() = "byte" and
42-
initializedWithConstants(this)
43-
}
44-
}
45-
46-
/** Defines a sub-set of expressions that update an array. */
47-
private class ArrayUpdate extends Expr {
48-
Expr array;
49-
50-
ArrayUpdate() {
51-
exists(Assignment assign, ArrayAccess arrayAccess | arrayAccess = assign.getDest() |
52-
assign = this and
53-
arrayAccess.getArray() = array and
54-
not assign.getSource() instanceof CompileTimeConstantExpr
55-
)
56-
or
57-
exists(StaticMethodAccess ma |
58-
ma.getMethod().hasQualifiedName("java.lang", "System", "arraycopy") and
59-
ma = this and
60-
ma.getArgument(2) = array
61-
)
62-
or
63-
exists(StaticMethodAccess ma |
64-
ma.getMethod().hasQualifiedName("java.util", "Arrays", "copyOf") and
65-
ma = this and
66-
ma = array
67-
)
68-
or
69-
exists(MethodAccess ma, Method m |
70-
m = ma.getMethod() and
71-
ma = this and
72-
ma.getArgument(0) = array
73-
|
74-
m.hasQualifiedName("java.io", "InputStream", "read") or
75-
m.hasQualifiedName("java.nio", "ByteBuffer", "get") or
76-
m.hasQualifiedName("java.security", "SecureRandom", "nextBytes") or
77-
m.hasQualifiedName("java.util", "Random", "nextBytes")
78-
)
79-
}
80-
81-
/** Returns the updated array. */
82-
Expr getArray() { result = array }
83-
}
84-
85-
/**
86-
* A config that tracks dataflow from creating an array to an operation that updates it.
87-
*/
88-
private class ArrayUpdateConfig extends TaintTracking2::Configuration {
89-
ArrayUpdateConfig() { this = "ArrayUpdateConfig" }
90-
91-
override predicate isSource(DataFlow::Node source) {
92-
source.asExpr() instanceof StaticByteArrayCreation
93-
}
94-
95-
override predicate isSink(DataFlow::Node sink) {
96-
exists(ArrayUpdate update | update.getArray() = sink.asExpr())
97-
}
98-
}
99-
100-
/**
101-
* A source that defines an array that doesn't get updated.
102-
*/
103-
private class StaticInitializationVectorSource extends DataFlow::Node {
104-
StaticInitializationVectorSource() {
105-
exists(StaticByteArrayCreation array | array = this.asExpr() |
106-
not exists(ArrayUpdate update, ArrayUpdateConfig config |
107-
config.hasFlow(DataFlow2::exprNode(array), DataFlow2::exprNode(update.getArray()))
108-
)
109-
)
110-
}
111-
}
112-
113-
/**
114-
* A config that tracks initialization of a cipher for encryption.
115-
*/
116-
private class EncryptionModeConfig extends TaintTracking2::Configuration {
117-
EncryptionModeConfig() { this = "EncryptionModeConfig" }
118-
119-
override predicate isSource(DataFlow::Node source) {
120-
source.asExpr().(VarAccess).getVariable().hasName("ENCRYPT_MODE")
121-
}
122-
123-
override predicate isSink(DataFlow::Node sink) {
124-
exists(MethodAccess ma, Method m | m = ma.getMethod() |
125-
m.hasQualifiedName("javax.crypto", "Cipher", "init") and
126-
ma.getArgument(0) = sink.asExpr()
127-
)
128-
}
129-
}
130-
131-
/**
132-
* A sink that initializes a cipher for encryption with unsafe parameters.
133-
*/
134-
private class EncryptionInitializationSink extends DataFlow::Node {
135-
EncryptionInitializationSink() {
136-
exists(MethodAccess ma, Method m, EncryptionModeConfig config | m = ma.getMethod() |
137-
m.hasQualifiedName("javax.crypto", "Cipher", "init") and
138-
m.getParameterType(2)
139-
.(RefType)
140-
.hasQualifiedName("java.security.spec", "AlgorithmParameterSpec") and
141-
ma.getArgument(2) = this.asExpr() and
142-
config.hasFlowToExpr(ma.getArgument(0))
143-
)
144-
}
145-
}
146-
147-
/**
148-
* Holds if `fromNode` to `toNode` is a dataflow step
149-
* that creates cipher's parameters with initialization vector.
150-
*/
151-
private predicate createInitializationVectorSpecStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
152-
exists(ConstructorCall cc, RefType type |
153-
cc = toNode.asExpr() and type = cc.getConstructedType()
154-
|
155-
type.hasQualifiedName("javax.crypto.spec", "IvParameterSpec") and
156-
cc.getArgument(0) = fromNode.asExpr()
157-
or
158-
type.hasQualifiedName("javax.crypto.spec", ["GCMParameterSpec", "RC2ParameterSpec"]) and
159-
cc.getArgument(1) = fromNode.asExpr()
160-
or
161-
type.hasQualifiedName("javax.crypto.spec", "RC5ParameterSpec") and
162-
cc.getArgument(3) = fromNode.asExpr()
163-
)
164-
}
165-
166-
/**
167-
* A config that tracks dataflow to initializing a cipher with a static initialization vector.
168-
*/
169-
private class StaticInitializationVectorConfig extends TaintTracking::Configuration {
170-
StaticInitializationVectorConfig() { this = "StaticInitializationVectorConfig" }
171-
172-
override predicate isSource(DataFlow::Node source) {
173-
source instanceof StaticInitializationVectorSource
174-
}
175-
176-
override predicate isSink(DataFlow::Node sink) { sink instanceof EncryptionInitializationSink }
177-
178-
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
179-
createInitializationVectorSpecStep(fromNode, toNode)
180-
}
181-
182-
override predicate isSanitizer(DataFlow::Node node) {
183-
exists(ArrayUpdate update | update.getArray() = node.asExpr())
184-
}
185-
}
186-
18723
from DataFlow::PathNode source, DataFlow::PathNode sink, StaticInitializationVectorConfig conf
18824
where conf.hasFlowPath(source, sink)
18925
select sink.getNode(), source, sink, "A $@ should not be used for encryption.", source.getNode(),

‎java/ql/src/experimental/semmle/code/java/security/StaticInitializationVectorQuery.qll‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
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, for example,
7+
* `new byte[8]` or `new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 }`.
8+
*/
9+
private predicate initializedWithConstants(ArrayCreationExpr array) {
10+
not exists(array.getInit())
11+
or
12+
forex(Expr element | element = array.getInit().getAChildExpr() |
13+
element instanceof CompileTimeConstantExpr
14+
)
15+
}
16+
17+
/**
18+
* An expression that creates a byte array that is initialized with constants.
19+
*/
20+
private class StaticByteArrayCreation extends ArrayCreationExpr {
21+
StaticByteArrayCreation() {
22+
this.getType().(Array).getElementType().(PrimitiveType).getName() = "byte" and
23+
initializedWithConstants(this)
24+
}
25+
}
26+
27+
/** Defines a sub-set of expressions that update an array. */
28+
private class ArrayUpdate extends Expr {
29+
Expr array;
30+
31+
ArrayUpdate() {
32+
exists(Assignment assign, ArrayAccess arrayAccess | arrayAccess = assign.getDest() |
33+
assign = this and
34+
arrayAccess.getArray() = array and
35+
not assign.getSource() instanceof CompileTimeConstantExpr
36+
)
37+
or
38+
exists(StaticMethodAccess ma |
39+
ma.getMethod().hasQualifiedName("java.lang", "System", "arraycopy") and
40+
ma = this and
41+
ma.getArgument(2) = array
42+
)
43+
or
44+
exists(StaticMethodAccess ma |
45+
ma.getMethod().hasQualifiedName("java.util", "Arrays", "copyOf") and
46+
ma = this and
47+
ma = array
48+
)
49+
or
50+
exists(MethodAccess ma, Method m |
51+
m = ma.getMethod() and
52+
ma = this and
53+
ma.getArgument(0) = array
54+
|
55+
m.hasQualifiedName("java.io", "InputStream", "read") or
56+
m.hasQualifiedName("java.nio", "ByteBuffer", "get") or
57+
m.hasQualifiedName("java.security", "SecureRandom", "nextBytes") or
58+
m.hasQualifiedName("java.util", "Random", "nextBytes")
59+
)
60+
}
61+
62+
/** Returns the updated array. */
63+
Expr getArray() { result = array }
64+
}
65+
66+
/**
67+
* A config that tracks dataflow from creating an array to an operation that updates it.
68+
*/
69+
private class ArrayUpdateConfig extends TaintTracking2::Configuration {
70+
ArrayUpdateConfig() { this = "ArrayUpdateConfig" }
71+
72+
override predicate isSource(DataFlow::Node source) {
73+
source.asExpr() instanceof StaticByteArrayCreation
74+
}
75+
76+
override predicate isSink(DataFlow::Node sink) {
77+
exists(ArrayUpdate update | update.getArray() = sink.asExpr())
78+
}
79+
}
80+
81+
/**
82+
* A source that defines an array that doesn't get updated.
83+
*/
84+
private class StaticInitializationVectorSource extends DataFlow::Node {
85+
StaticInitializationVectorSource() {
86+
exists(StaticByteArrayCreation array | array = this.asExpr() |
87+
not exists(ArrayUpdate update, ArrayUpdateConfig config |
88+
config.hasFlow(DataFlow2::exprNode(array), DataFlow2::exprNode(update.getArray()))
89+
)
90+
)
91+
}
92+
}
93+
94+
/**
95+
* A config that tracks initialization of a cipher for encryption.
96+
*/
97+
private class EncryptionModeConfig extends TaintTracking2::Configuration {
98+
EncryptionModeConfig() { this = "EncryptionModeConfig" }
99+
100+
override predicate isSource(DataFlow::Node source) {
101+
source.asExpr().(VarAccess).getVariable().hasName("ENCRYPT_MODE")
102+
}
103+
104+
override predicate isSink(DataFlow::Node sink) {
105+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
106+
m.hasQualifiedName("javax.crypto", "Cipher", "init") and
107+
ma.getArgument(0) = sink.asExpr()
108+
)
109+
}
110+
}
111+
112+
/**
113+
* A sink that initializes a cipher for encryption with unsafe parameters.
114+
*/
115+
private class EncryptionInitializationSink extends DataFlow::Node {
116+
EncryptionInitializationSink() {
117+
exists(MethodAccess ma, Method m, EncryptionModeConfig config | m = ma.getMethod() |
118+
m.hasQualifiedName("javax.crypto", "Cipher", "init") and
119+
m.getParameterType(2)
120+
.(RefType)
121+
.hasQualifiedName("java.security.spec", "AlgorithmParameterSpec") and
122+
ma.getArgument(2) = this.asExpr() and
123+
config.hasFlowToExpr(ma.getArgument(0))
124+
)
125+
}
126+
}
127+
128+
/**
129+
* Holds if `fromNode` to `toNode` is a dataflow step
130+
* that creates cipher's parameters with initialization vector.
131+
*/
132+
private predicate createInitializationVectorSpecStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
133+
exists(ConstructorCall cc, RefType type |
134+
cc = toNode.asExpr() and type = cc.getConstructedType()
135+
|
136+
type.hasQualifiedName("javax.crypto.spec", "IvParameterSpec") and
137+
cc.getArgument(0) = fromNode.asExpr()
138+
or
139+
type.hasQualifiedName("javax.crypto.spec", ["GCMParameterSpec", "RC2ParameterSpec"]) and
140+
cc.getArgument(1) = fromNode.asExpr()
141+
or
142+
type.hasQualifiedName("javax.crypto.spec", "RC5ParameterSpec") and
143+
cc.getArgument(3) = fromNode.asExpr()
144+
)
145+
}
146+
147+
/**
148+
* A config that tracks dataflow to initializing a cipher with a static initialization vector.
149+
*/
150+
class StaticInitializationVectorConfig extends TaintTracking::Configuration {
151+
StaticInitializationVectorConfig() { this = "StaticInitializationVectorConfig" }
152+
153+
override predicate isSource(DataFlow::Node source) {
154+
source instanceof StaticInitializationVectorSource
155+
}
156+
157+
override predicate isSink(DataFlow::Node sink) { sink instanceof EncryptionInitializationSink }
158+
159+
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
160+
createInitializationVectorSpecStep(fromNode, toNode)
161+
}
162+
163+
override predicate isSanitizer(DataFlow::Node node) {
164+
exists(ArrayUpdate update | update.getArray() = node.asExpr())
165+
}
166+
}

‎java/ql/test/experimental/query-tests/security/CWE-1204/StaticInitializationVector.expected‎

Lines changed: 0 additions & 15 deletions
This file was deleted.

‎java/ql/test/experimental/query-tests/security/CWE-1204/StaticInitializationVector.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public byte[] encryptWithStaticIvByteArrayWithInitializer(byte[] key, byte[] pla
1616
SecretKeySpec keySpec = new SecretKeySpec(key, "AES");
1717

1818
Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING");
19-
cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec);
19+
cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector
2020
cipher.update(plaintext);
2121
return cipher.doFinal();
2222
}
@@ -29,7 +29,7 @@ public byte[] encryptWithZeroStaticIvByteArray(byte[] key, byte[] plaintext) thr
2929
SecretKeySpec keySpec = new SecretKeySpec(key, "AES");
3030

3131
Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING");
32-
cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec);
32+
cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector
3333
cipher.update(plaintext);
3434
return cipher.doFinal();
3535
}
@@ -45,7 +45,7 @@ public byte[] encryptWithStaticIvByteArray(byte[] key, byte[] plaintext) throws
4545
SecretKeySpec keySpec = new SecretKeySpec(key, "AES");
4646

4747
Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5PADDING");
48-
cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec);
48+
cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector
4949
cipher.update(plaintext);
5050
return cipher.doFinal();
5151
}

‎java/ql/test/experimental/query-tests/security/CWE-1204/StaticInitializationVector.qlref‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎java/ql/test/experimental/query-tests/security/CWE-1204/StaticInitializationVectorTest.expected‎

Whitespace-only changes.

0 commit comments

Comments
 (0)