Skip to content

Commit f2dea89

Browse files
aschackmullaschackmull
committed
Java: Remove/deprecate FlowStateString-based extension points.
1 parent 8ef4821 commit f2dea89

File tree

7 files changed

+157
-84
lines changed

7 files changed

+157
-84
lines changed

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

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,40 @@ private import semmle.code.java.dataflow.TaintTracking
66
private import semmle.code.java.frameworks.android.Intent
77
private import semmle.code.java.frameworks.android.PendingIntent
88

9+
private newtype TPendingIntentState =
10+
TMutablePendingIntent() or
11+
TNoState()
12+
13+
class PendingIntentState extends TPendingIntentState {
14+
abstract string toString();
15+
}
16+
17+
class MutablePendingIntent extends PendingIntentState, TMutablePendingIntent {
18+
override string toString() { result = "MutablePendingIntent" }
19+
}
20+
21+
class NoState extends PendingIntentState, TNoState {
22+
override string toString() { result = "NoState" }
23+
}
24+
925
/** A source for an implicit `PendingIntent` flow. */
1026
abstract class ImplicitPendingIntentSource extends DataFlow::Node {
11-
/** Holds if this source has the specified `state`. */
12-
predicate hasState(DataFlow::FlowState state) { state = "" }
27+
/**
28+
* DEPRECATED: Open-ended flow state is not intended to be part of the extension points.
29+
*
30+
* Holds if this source has the specified `state`.
31+
*/
32+
deprecated predicate hasState(DataFlow::FlowState state) { state = "" }
1333
}
1434

1535
/** A sink that sends an implicit and mutable `PendingIntent` to a third party. */
1636
abstract class ImplicitPendingIntentSink extends DataFlow::Node {
17-
/** Holds if this sink has the specified `state`. */
18-
predicate hasState(DataFlow::FlowState state) { state = "" }
37+
/**
38+
* DEPRECATED: Open-ended flow state is not intended to be part of the extension points.
39+
*
40+
* Holds if this sink has the specified `state`.
41+
*/
42+
deprecated predicate hasState(DataFlow::FlowState state) { state = "" }
1943
}
2044

2145
/**
@@ -32,11 +56,19 @@ class ImplicitPendingIntentAdditionalTaintStep extends Unit {
3256
predicate step(DataFlow::Node node1, DataFlow::Node node2) { none() }
3357

3458
/**
59+
* Holds if the step from `node1` to `node2` creates a mutable `PendingIntent`.
60+
*/
61+
predicate mutablePendingIntentCreation(DataFlow::Node node1, DataFlow::Node node2) { none() }
62+
63+
/**
64+
* DEPRECATED: Open-ended flow state is not intended to be part of the extension points.
65+
* Use `mutablePendingIntentCreation` instead.
66+
*
3567
* Holds if the step from `node1` to `node2` should be considered a taint
3668
* step for flows related to the use of implicit `PendingIntent`s. This step is only applicable
3769
* in `state1` and updates the flow state to `state2`.
3870
*/
39-
predicate step(
71+
deprecated predicate step(
4072
DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
4173
DataFlow::FlowState state2
4274
) {
@@ -66,17 +98,10 @@ private class SendPendingIntent extends ImplicitPendingIntentSink {
6698
or
6799
sinkNode(this, "pending-intents")
68100
}
69-
70-
override predicate hasState(DataFlow::FlowState state) { state = "MutablePendingIntent" }
71101
}
72102

73103
private class MutablePendingIntentFlowStep extends ImplicitPendingIntentAdditionalTaintStep {
74-
override predicate step(
75-
DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
76-
DataFlow::FlowState state2
77-
) {
78-
state1 = "" and
79-
state2 = "MutablePendingIntent" and
104+
override predicate mutablePendingIntentCreation(DataFlow::Node node1, DataFlow::Node node2) {
80105
exists(PendingIntentCreation pic, Argument flagArg |
81106
node1.asExpr() = pic.getIntentArg() and
82107
node2.asExpr() = pic and

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,14 @@ deprecated class ImplicitPendingIntentStartConf extends TaintTracking::Configura
6060
* being wrapped in another implicit `Intent` that gets started.
6161
*/
6262
module ImplicitPendingIntentStartConfig implements DataFlow::StateConfigSig {
63-
class FlowState = DataFlow::FlowState;
63+
class FlowState = PendingIntentState;
6464

6565
predicate isSource(DataFlow::Node source, FlowState state) {
66-
source.(ImplicitPendingIntentSource).hasState(state)
66+
source instanceof ImplicitPendingIntentSource and state instanceof NoState
6767
}
6868

6969
predicate isSink(DataFlow::Node sink, FlowState state) {
70-
sink.(ImplicitPendingIntentSink).hasState(state)
70+
sink instanceof ImplicitPendingIntentSink and state instanceof MutablePendingIntent
7171
}
7272

7373
predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof ExplicitIntentSanitizer }
@@ -79,7 +79,9 @@ module ImplicitPendingIntentStartConfig implements DataFlow::StateConfigSig {
7979
predicate isAdditionalFlowStep(
8080
DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
8181
) {
82-
any(ImplicitPendingIntentAdditionalTaintStep c).step(node1, state1, node2, state2)
82+
any(ImplicitPendingIntentAdditionalTaintStep c).mutablePendingIntentCreation(node1, node2) and
83+
state1 instanceof NoState and
84+
state2 instanceof MutablePendingIntent
8385
}
8486

8587
predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {

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

Lines changed: 72 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,32 +3,74 @@
33
private import semmle.code.java.security.Encryption
44
private import semmle.code.java.dataflow.DataFlow
55
private import semmle.code.java.security.internal.EncryptionKeySizes
6+
import codeql.util.Either
7+
8+
/** A minimum recommended key size for some algorithm. */
9+
abstract class MinimumKeySize extends int {
10+
bindingset[this]
11+
MinimumKeySize() { any() }
12+
13+
string toString() { result = super.toString() }
14+
}
15+
16+
/**
17+
* A class of algorithms for which a key size smaller than the recommended key
18+
* size might be embedded in the algorithm name.
19+
*/
20+
abstract class AlgorithmKind extends string {
21+
bindingset[this]
22+
AlgorithmKind() { any() }
23+
24+
string toString() { result = super.toString() }
25+
}
26+
27+
/**
28+
* A key size that is greater than the tracked value and equal to the minimum
29+
* recommended key size for some algorithm, or a kind of algorithm for which the
30+
* tracked string indicates a too small key size.
31+
*/
32+
final class KeySizeState = Either<MinimumKeySize, AlgorithmKind>::Either;
633

734
/** A source for an insufficient key size. */
835
abstract class InsufficientKeySizeSource extends DataFlow::Node {
936
/** Holds if this source has the specified `state`. */
10-
predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty }
37+
abstract predicate hasState(KeySizeState state);
1138
}
1239

1340
/** A sink for an insufficient key size. */
1441
abstract class InsufficientKeySizeSink extends DataFlow::Node {
15-
/** Holds if this sink has the specified `state`. */
16-
predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty }
42+
/** Holds if this sink accepts the specified `state`. */
43+
final predicate hasState(KeySizeState state) {
44+
state.asLeft() <= this.minimumKeySize() or this.algorithmKind(state.asRight())
45+
}
46+
47+
/** Gets the minimum recommended key size. */
48+
abstract int minimumKeySize();
49+
50+
/**
51+
* Holds if this sink recommends a keysize that is greater than the value in a
52+
* source with the given algorithm kind.
53+
*/
54+
predicate algorithmKind(AlgorithmKind kind) { none() }
55+
}
56+
57+
/** A source for an insufficient key size used in some algorithm. */
58+
private class IntegerLiteralSource extends InsufficientKeySizeSource {
59+
private int value;
60+
61+
IntegerLiteralSource() { this.asExpr().(IntegerLiteral).getIntValue() = value }
62+
63+
override predicate hasState(KeySizeState state) {
64+
state.asLeft() = min(MinimumKeySize m | value < m)
65+
}
1766
}
1867

1968
/** Provides models for asymmetric cryptography. */
2069
private module Asymmetric {
2170
/** Provides models for non-elliptic-curve asymmetric cryptography. */
2271
private module NonEllipticCurve {
23-
/** A source for an insufficient key size used in RSA, DSA, and DH algorithms. */
24-
private class Source extends InsufficientKeySizeSource {
25-
string algoName;
26-
27-
Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize(algoName) }
28-
29-
override predicate hasState(DataFlow::FlowState state) {
30-
state = getMinKeySize(algoName).toString()
31-
}
72+
private class NonEllipticCurveKeySize extends MinimumKeySize {
73+
NonEllipticCurveKeySize() { this = getMinKeySize(_) }
3274
}
3375

3476
/** A sink for an insufficient key size used in RSA, DSA, and DH algorithms. */
@@ -46,9 +88,7 @@ private module Asymmetric {
4688
exists(Spec spec | this.asExpr() = spec.getKeySizeArg() and algoName = spec.getAlgoName())
4789
}
4890

49-
override predicate hasState(DataFlow::FlowState state) {
50-
state = getMinKeySize(algoName).toString()
51-
}
91+
override int minimumKeySize() { result = getMinKeySize(algoName) }
5292
}
5393

5494
/** Returns the minimum recommended key size for RSA, DSA, and DH algorithms. */
@@ -88,16 +128,24 @@ private module Asymmetric {
88128

89129
/** Provides models for elliptic-curve asymmetric cryptography. */
90130
private module EllipticCurve {
131+
private class EllipticCurveKeySize extends MinimumKeySize {
132+
EllipticCurveKeySize() { this = getMinKeySize() }
133+
}
134+
135+
private class EllipticCurveKind extends AlgorithmKind {
136+
EllipticCurveKind() { this = "EC" }
137+
}
138+
91139
/** A source for an insufficient key size used in elliptic curve (EC) algorithms. */
92140
private class Source extends InsufficientKeySizeSource {
93141
Source() {
94-
this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize()
95-
or
96142
// the below is needed for cases when the key size is embedded in the curve name
97143
getKeySize(this.asExpr().(StringLiteral).getValue()) < getMinKeySize()
98144
}
99145

100-
override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() }
146+
override predicate hasState(KeySizeState state) {
147+
state.asRight() instanceof EllipticCurveKind
148+
}
101149
}
102150

103151
/** A sink for an insufficient key size used in elliptic curve (EC) algorithms. */
@@ -112,7 +160,9 @@ private module Asymmetric {
112160
exists(Spec s | this.asExpr() = s.getKeySizeArg())
113161
}
114162

115-
override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() }
163+
override int minimumKeySize() { result = getMinKeySize() }
164+
165+
override predicate algorithmKind(AlgorithmKind kind) { kind instanceof EllipticCurveKind }
116166
}
117167

118168
/** Returns the minimum recommended key size for elliptic curve (EC) algorithms. */
@@ -176,11 +226,8 @@ private module Asymmetric {
176226

177227
/** Provides models for symmetric cryptography. */
178228
private module Symmetric {
179-
/** A source for an insufficient key size used in AES algorithms. */
180-
private class Source extends InsufficientKeySizeSource {
181-
Source() { this.asExpr().(IntegerLiteral).getIntValue() < getMinKeySize() }
182-
183-
override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() }
229+
private class SymmetricKeySize extends MinimumKeySize {
230+
SymmetricKeySize() { this = getMinKeySize() }
184231
}
185232

186233
/** A sink for an insufficient key size used in AES algorithms. */
@@ -193,7 +240,7 @@ private module Symmetric {
193240
)
194241
}
195242

196-
override predicate hasState(DataFlow::FlowState state) { state = getMinKeySize().toString() }
243+
override int minimumKeySize() { result = getMinKeySize() }
197244
}
198245

199246
/** Returns the minimum recommended key size for AES algorithms. */

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

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,36 +12,27 @@ deprecated class KeySizeConfiguration extends DataFlow::Configuration {
1212
KeySizeConfiguration() { this = "KeySizeConfiguration" }
1313

1414
override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
15-
source.(InsufficientKeySizeSource).hasState(state)
15+
exists(KeySizeState s | source.(InsufficientKeySizeSource).hasState(s) and state = s.toString())
1616
}
1717

1818
override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
19-
sink.(InsufficientKeySizeSink).hasState(state)
19+
exists(KeySizeState s | sink.(InsufficientKeySizeSink).hasState(s) and state = s.toString())
2020
}
2121
}
2222

2323
/**
2424
* A data flow configuration for tracking key sizes used in cryptographic algorithms.
2525
*/
2626
module KeySizeConfig implements DataFlow::StateConfigSig {
27-
class FlowState = DataFlow::FlowState;
27+
class FlowState = KeySizeState;
2828

29-
predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
29+
predicate isSource(DataFlow::Node source, KeySizeState state) {
3030
source.(InsufficientKeySizeSource).hasState(state)
3131
}
3232

33-
predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
33+
predicate isSink(DataFlow::Node sink, KeySizeState state) {
3434
sink.(InsufficientKeySizeSink).hasState(state)
3535
}
36-
37-
predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) { none() }
38-
39-
predicate isAdditionalFlowStep(
40-
DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
41-
DataFlow::FlowState state2
42-
) {
43-
none()
44-
}
4536
}
4637

4738
/** Tracks key sizes used in cryptographic algorithms. */

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

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,28 @@ private import semmle.code.java.dataflow.TaintTracking
99
* A source for server-side template injection (SST) vulnerabilities.
1010
*/
1111
abstract class TemplateInjectionSource extends DataFlow::Node {
12-
/** Holds if this source has the specified `state`. */
13-
predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty }
12+
/**
13+
* DEPRECATED: Open-ended flow state is not intended to be part of the extension points.
14+
*
15+
* Holds if this source has the specified `state`.
16+
*/
17+
deprecated predicate hasState(DataFlow::FlowState state) {
18+
state instanceof DataFlow::FlowStateEmpty
19+
}
1420
}
1521

1622
/**
1723
* A sink for server-side template injection (SST) vulnerabilities.
1824
*/
1925
abstract class TemplateInjectionSink extends DataFlow::Node {
20-
/** Holds if this sink has the specified `state`. */
21-
predicate hasState(DataFlow::FlowState state) { state instanceof DataFlow::FlowStateEmpty }
26+
/**
27+
* DEPRECATED: Open-ended flow state is not intended to be part of the extension points.
28+
*
29+
* Holds if this sink has the specified `state`.
30+
*/
31+
deprecated predicate hasState(DataFlow::FlowState state) {
32+
state instanceof DataFlow::FlowStateEmpty
33+
}
2234
}
2335

2436
/**
@@ -35,11 +47,13 @@ class TemplateInjectionAdditionalTaintStep extends Unit {
3547
predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { none() }
3648

3749
/**
50+
* DEPRECATED: Open-ended flow state is not intended to be part of the extension points.
51+
*
3852
* Holds if the step from `node1` to `node2` should be considered a taint
3953
* step for flows related toserver-side template injection (SST) vulnerabilities.
4054
* This step is only applicable in `state1` and updates the flow state to `state2`.
4155
*/
42-
predicate isAdditionalTaintStep(
56+
deprecated predicate isAdditionalTaintStep(
4357
DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2,
4458
DataFlow::FlowState state2
4559
) {
@@ -53,13 +67,19 @@ class TemplateInjectionAdditionalTaintStep extends Unit {
5367
abstract class TemplateInjectionSanitizer extends DataFlow::Node { }
5468

5569
/**
70+
* DEPRECATED: Open-ended flow state is not intended to be part of the extension points.
71+
*
5672
* A sanitizer for server-side template injection (SST) vulnerabilities.
5773
* This sanitizer is only applicable when `TemplateInjectionSanitizerWithState::hasState`
5874
* holds for the flow state.
5975
*/
60-
abstract class TemplateInjectionSanitizerWithState extends DataFlow::Node {
61-
/** Holds if this sanitizer has the specified `state`. */
62-
abstract predicate hasState(DataFlow::FlowState state);
76+
abstract deprecated class TemplateInjectionSanitizerWithState extends DataFlow::Node {
77+
/**
78+
* DEPRECATED: Open-ended flow state is not intended to be part of the extension points.
79+
*
80+
* Holds if this sanitizer has the specified `state`.
81+
*/
82+
abstract deprecated predicate hasState(DataFlow::FlowState state);
6383
}
6484

6585
private class DefaultTemplateInjectionSource extends TemplateInjectionSource instanceof ThreatModelFlowSource

0 commit comments

Comments
 (0)