Skip to content

Commit fdd09a4

Browse files
committed
Adding a new means for determining if there is nonce reuse.
1 parent 28ccc83 commit fdd09a4

File tree

3 files changed

+82
-36
lines changed

3 files changed

+82
-36
lines changed
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import java
2+
import semmle.code.java.dataflow.DataFlow
3+
import experimental.Quantum.Language
4+
5+
/**
6+
* Flow from any function that appears to return a value
7+
* to an artifact node.
8+
* NOTE: TODO: need to handle call by refernece for now. Need to re-evaluate (see notes below)
9+
* Such functions may be 'wrappers' for some derived value.
10+
*/
11+
private module WrapperConfig implements DataFlow::ConfigSig {
12+
predicate isSource(DataFlow::Node source) {
13+
exists(Call c |
14+
c = source.asExpr()
15+
// not handling references yet, I think we want to flat say references are only ok
16+
// if I know the source, otherwise, it has to be through an additional flow step, which
17+
// we filter as a source, i.e., references are only allowed as sources only,
18+
// no inferrece? Not sure if that would work
19+
//or
20+
// source.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = c.getAnArgument()
21+
) and
22+
// Filter out sources that are known additional flow steps, as these are likely not the
23+
// kind of wrapper source we are looking for.
24+
not exists(AdditionalFlowInputStep s | s.getOutput() = source)
25+
}
26+
27+
// Flow through additional flow steps
28+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
29+
node1.(AdditionalFlowInputStep).getOutput() = node2
30+
}
31+
32+
predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(Crypto::ArtifactNode i).asElement() }
33+
}
34+
35+
module WrapperFlow = DataFlow::Global<WrapperConfig>;
36+
37+
/**
38+
* Using a set approach to determine if reuse of an artifact exists.
39+
* This predicate produces a set of 'wrappers' that flow to the artifact node.
40+
* This set can be compared with the set to another artifact node to determine if they are the same.
41+
*/
42+
private DataFlow::Node getWrapperSet(Crypto::NonceArtifactNode a) {
43+
WrapperFlow::flow(result, DataFlow::exprNode(a.asElement()))
44+
or
45+
result.asExpr() = a.getSourceElement()
46+
}
47+
48+
/**
49+
* Two different artifact nodes are considered reuse if any of the following conditions are met:
50+
* 1. The source for artifact `a` and artifact `b` are the same and the source is a literal.
51+
* 2. The source for artifact `a` and artifact `b` are not the same and the source is a literal of the same value.
52+
* 3. For all 'wrappers' that return the source of artifact `a`, and that wrapper also exists for artifact `b`.
53+
* 4. For all 'wrappers' that return the source of artifact `b`, and that wrapper also exists for artifact `a`.
54+
*/
55+
predicate isArtifactReuse(Crypto::ArtifactNode a, Crypto::ArtifactNode b) {
56+
a != b and
57+
(
58+
a.getSourceElement() = b.getSourceElement() and a.getSourceElement() instanceof Literal
59+
or
60+
a.getSourceElement().(Literal).getValue() = b.getSourceElement().(Literal).getValue()
61+
or
62+
forex(DataFlow::Node e | e = getWrapperSet(a) |
63+
exists(DataFlow::Node e2 | e2 = getWrapperSet(b) | e = e2)
64+
)
65+
or
66+
forex(DataFlow::Node e | e = getWrapperSet(b) |
67+
exists(DataFlow::Node e2 | e2 = getWrapperSet(a) | e = e2)
68+
)
69+
)
70+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* @name Detects reuse of the same nonce in multiple operations
3+
* @id java/crypto_inventory_filter/nonce_reuse
4+
* @kind problem
5+
*/
6+
7+
import java
8+
import ArtifactReuse
9+
10+
from Crypto::NonceArtifactNode nonce1, Crypto::NonceArtifactNode nonce2
11+
where isArtifactReuse(nonce1, nonce2)
12+
select nonce1, "Reuse with nonce $@", nonce2, nonce2.toString()

java/ql/src/experimental/Quantum/PossibleReusedNonce.ql

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

0 commit comments

Comments
 (0)