Skip to content

Commit 85fdbda

Browse files
aschackmullaschackmull
authored
Merge pull request #7002 from aschackmull/java/field-node
Java: Add FieldValueNode to break up cartesian step relation.
2 parents e0b121c + 35b6cbe commit 85fdbda

File tree

10 files changed

+99
-47
lines changed

10 files changed

+99
-47
lines changed

‎java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ private import semmle.code.java.dispatch.VirtualDispatch as VirtualDispatch
88
private module DispatchImpl {
99
/** Gets a viable implementation of the target of the given `Call`. */
1010
DataFlowCallable viableCallable(DataFlowCall c) {
11-
result = VirtualDispatch::viableCallable(c.asCall())
11+
result.asCallable() = VirtualDispatch::viableCallable(c.asCall())
1212
or
13-
result.(SummarizedCallable) = c.asCall().getCallee().getSourceDeclaration()
13+
result.(SummarizedCallable).asCallable() = c.asCall().getCallee().getSourceDeclaration()
1414
}
1515

1616
/**
@@ -93,31 +93,32 @@ private module DispatchImpl {
9393
* qualifier is a parameter of the enclosing callable `c`.
9494
*/
9595
predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) {
96-
mayBenefitFromCallContext(call.asCall(), c, _)
96+
mayBenefitFromCallContext(call.asCall(), c.asCallable(), _)
9797
}
9898

9999
/**
100100
* Gets a viable dispatch target of `call` in the context `ctx`. This is
101101
* restricted to those `call`s for which a context might make a difference.
102102
*/
103-
Method viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
103+
DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
104104
result = viableCallable(call) and
105105
exists(int i, Callable c, Method def, RefType t, boolean exact, MethodAccess ma |
106106
ma = call.asCall() and
107107
mayBenefitFromCallContext(ma, c, i) and
108-
c = viableCallable(ctx) and
108+
c = viableCallable(ctx).asCallable() and
109109
contextArgHasType(ctx.asCall(), i, t, exact) and
110110
ma.getMethod().getSourceDeclaration() = def
111111
|
112-
exact = true and result = VirtualDispatch::exactMethodImpl(def, t.getSourceDeclaration())
112+
exact = true and
113+
result.asCallable() = VirtualDispatch::exactMethodImpl(def, t.getSourceDeclaration())
113114
or
114115
exact = false and
115116
exists(RefType t2 |
116-
result = VirtualDispatch::viableMethodImpl(def, t.getSourceDeclaration(), t2) and
117+
result.asCallable() = VirtualDispatch::viableMethodImpl(def, t.getSourceDeclaration(), t2) and
117118
not failsUnification(t, t2)
118119
)
119120
or
120-
result = def and def instanceof SummarizedCallable
121+
result.asCallable() = def and result instanceof SummarizedCallable
121122
)
122123
}
123124

‎java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@ newtype TNode =
1414
not e.getParent*() instanceof Annotation
1515
} or
1616
TExplicitParameterNode(Parameter p) {
17-
exists(p.getCallable().getBody()) or p.getCallable() instanceof SummarizedCallable
17+
exists(p.getCallable().getBody()) or p.getCallable() = any(SummarizedCallable sc).asCallable()
1818
} or
1919
TImplicitVarargsArray(Call c) {
2020
c.getCallee().isVarargs() and
2121
not exists(Argument arg | arg.getCall() = c and arg.isExplicitVarargsArray())
2222
} or
2323
TInstanceParameterNode(Callable c) {
24-
(exists(c.getBody()) or c instanceof SummarizedCallable) and
24+
(exists(c.getBody()) or c = any(SummarizedCallable sc).asCallable()) and
2525
not c.isStatic()
2626
} or
2727
TImplicitInstanceAccess(InstanceAccessExt ia) { not ia.isExplicit(_) } or
@@ -44,7 +44,8 @@ newtype TNode =
4444
} or
4545
TSummaryInternalNode(SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) {
4646
FlowSummaryImpl::Private::summaryNodeRange(c, state)
47-
}
47+
} or
48+
TFieldValueNode(Field f)
4849

4950
private predicate explicitInstanceArgument(Call call, Expr instarg) {
5051
call instanceof MethodAccess and
@@ -94,19 +95,12 @@ module Public {
9495
result = this.(MallocNode).getClassInstanceExpr().getType()
9596
or
9697
result = this.(ImplicitPostUpdateNode).getPreUpdateNode().getType()
98+
or
99+
result = this.(FieldValueNode).getField().getType()
97100
}
98101

99102
/** Gets the callable in which this node occurs. */
100-
Callable getEnclosingCallable() {
101-
result = this.asExpr().getEnclosingCallable() or
102-
result = this.asParameter().getCallable() or
103-
result = this.(ImplicitVarargsArray).getCall().getEnclosingCallable() or
104-
result = this.(InstanceParameterNode).getCallable() or
105-
result = this.(ImplicitInstanceAccess).getInstanceAccess().getEnclosingCallable() or
106-
result = this.(MallocNode).getClassInstanceExpr().getEnclosingCallable() or
107-
result = this.(ImplicitPostUpdateNode).getPreUpdateNode().getEnclosingCallable() or
108-
this = TSummaryInternalNode(result, _)
109-
}
103+
Callable getEnclosingCallable() { result = nodeGetEnclosingCallable(this).asCallable() }
110104

111105
private Type getImprovedTypeBound() {
112106
exprTypeFlow(this.asExpr(), result, _) or
@@ -257,6 +251,18 @@ module Public {
257251
abstract Node getPreUpdateNode();
258252
}
259253

254+
/**
255+
* A node representing the value of a field.
256+
*/
257+
class FieldValueNode extends Node, TFieldValueNode {
258+
/** Gets the field corresponding to this node. */
259+
Field getField() { this = TFieldValueNode(result) }
260+
261+
override string toString() { result = getField().toString() }
262+
263+
override Location getLocation() { result = getField().getLocation() }
264+
}
265+
260266
/**
261267
* Gets the node that occurs as the qualifier of `fa`.
262268
*/
@@ -305,11 +311,21 @@ private class ImplicitExprPostUpdate extends ImplicitPostUpdateNode, TImplicitEx
305311

306312
module Private {
307313
/** Gets the callable in which this node occurs. */
308-
DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.getEnclosingCallable() }
314+
DataFlowCallable nodeGetEnclosingCallable(Node n) {
315+
result.asCallable() = n.asExpr().getEnclosingCallable() or
316+
result.asCallable() = n.asParameter().getCallable() or
317+
result.asCallable() = n.(ImplicitVarargsArray).getCall().getEnclosingCallable() or
318+
result.asCallable() = n.(InstanceParameterNode).getCallable() or
319+
result.asCallable() = n.(ImplicitInstanceAccess).getInstanceAccess().getEnclosingCallable() or
320+
result.asCallable() = n.(MallocNode).getClassInstanceExpr().getEnclosingCallable() or
321+
result = nodeGetEnclosingCallable(n.(ImplicitPostUpdateNode).getPreUpdateNode()) or
322+
n = TSummaryInternalNode(result, _) or
323+
result.asFieldScope() = n.(FieldValueNode).getField()
324+
}
309325

310326
/** Holds if `p` is a `ParameterNode` of `c` with position `pos`. */
311327
predicate isParameterNode(ParameterNode p, DataFlowCallable c, int pos) {
312-
p.isParameterOf(c, pos)
328+
p.isParameterOf(c.asCallable(), pos)
313329
}
314330

315331
/**

‎java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,18 @@ OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) {
3232
/**
3333
* Holds if data can flow from `node1` to `node2` through a static field.
3434
*/
35-
private predicate staticFieldStep(ExprNode node1, ExprNode node2) {
35+
private predicate staticFieldStep(Node node1, Node node2) {
36+
exists(Field f |
37+
f.isStatic() and
38+
f.getAnAssignedValue() = node1.asExpr() and
39+
node2.(FieldValueNode).getField() = f
40+
)
41+
or
3642
exists(Field f, FieldRead fr |
3743
f.isStatic() and
38-
f.getAnAssignedValue() = node1.getExpr() and
44+
node1.(FieldValueNode).getField() = f and
3945
fr.getField() = f and
40-
fr = node2.getExpr() and
46+
fr = node2.asExpr() and
4147
hasNonlocalValue(fr)
4248
)
4349
}
@@ -205,7 +211,30 @@ class CastNode extends ExprNode {
205211
CastNode() { this.getExpr() instanceof CastExpr }
206212
}
207213

208-
class DataFlowCallable = Callable;
214+
private newtype TDataFlowCallable =
215+
TCallable(Callable c) or
216+
TFieldScope(Field f)
217+
218+
class DataFlowCallable extends TDataFlowCallable {
219+
Callable asCallable() { this = TCallable(result) }
220+
221+
Field asFieldScope() { this = TFieldScope(result) }
222+
223+
RefType getDeclaringType() {
224+
result = asCallable().getDeclaringType() or
225+
result = asFieldScope().getDeclaringType()
226+
}
227+
228+
string toString() {
229+
result = asCallable().toString() or
230+
result = "Field scope: " + asFieldScope().toString()
231+
}
232+
233+
Location getLocation() {
234+
result = asCallable().getLocation() or
235+
result = asFieldScope().getLocation()
236+
}
237+
}
209238

210239
class DataFlowExpr = Expr;
211240

@@ -251,7 +280,9 @@ class SrcCall extends DataFlowCall, TCall {
251280

252281
SrcCall() { this = TCall(call) }
253282

254-
override DataFlowCallable getEnclosingCallable() { result = call.getEnclosingCallable() }
283+
override DataFlowCallable getEnclosingCallable() {
284+
result.asCallable() = call.getEnclosingCallable()
285+
}
255286

256287
override string toString() { result = call.toString() }
257288

@@ -345,10 +376,10 @@ class LambdaCallKind = Method; // the "apply" method in the functional interface
345376
predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c) {
346377
exists(ClassInstanceExpr func, Interface t, FunctionalInterface interface |
347378
creation.asExpr() = func and
348-
func.getAnonymousClass().getAMethod() = c and
379+
func.getAnonymousClass().getAMethod() = c.asCallable() and
349380
func.getConstructedType().extendsOrImplements+(t) and
350381
t.getSourceDeclaration() = interface and
351-
c.(Method).overridesOrInstantiates+(pragma[only_bind_into](kind)) and
382+
c.asCallable().(Method).overridesOrInstantiates+(pragma[only_bind_into](kind)) and
352383
pragma[only_bind_into](kind) = interface.getRunMethod().getSourceDeclaration()
353384
)
354385
}

‎java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImplSpecific.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ DataFlowType getContentType(Content c) { result = c.getType() }
3030

3131
/** Gets the return type of kind `rk` for callable `c`. */
3232
DataFlowType getReturnType(SummarizedCallable c, ReturnKind rk) {
33-
result = getErasedRepr(c.getReturnType()) and
33+
result = getErasedRepr(c.asCallable().getReturnType()) and
3434
exists(rk)
3535
}
3636

@@ -62,7 +62,7 @@ predicate summaryElement(DataFlowCallable c, string input, string output, string
6262
string namespace, string type, boolean subtypes, string name, string signature, string ext
6363
|
6464
summaryModel(namespace, type, subtypes, name, signature, ext, input, output, kind) and
65-
c = interpretElement(namespace, type, subtypes, name, signature, ext)
65+
c.asCallable() = interpretElement(namespace, type, subtypes, name, signature, ext)
6666
)
6767
}
6868

@@ -119,7 +119,7 @@ class InterpretNode extends TInterpretNode {
119119
DataFlowCall asCall() { result.asCall() = this.asElement() }
120120

121121
/** Gets the callable that this node corresponds to, if any. */
122-
DataFlowCallable asCallable() { result = this.asElement() }
122+
DataFlowCallable asCallable() { result.asCallable() = this.asElement() }
123123

124124
/** Gets the target of this call, if any. */
125125
Callable getCallTarget() { result = this.asCall().asCall().getCallee().getSourceDeclaration() }

‎java/ql/lib/semmle/code/java/dispatch/DispatchFlow.qll

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -189,10 +189,9 @@ private predicate flowStep(RelevantNode n1, RelevantNode n2) {
189189
n2.(ImplicitInstanceAccess).getInstanceAccess().(OwnInstanceAccess).getEnclosingCallable() = c
190190
)
191191
or
192-
exists(Field f |
193-
f.getAnAssignedValue() = n1.asExpr() and
194-
n2.asExpr().(FieldRead).getField() = f
195-
)
192+
n2.(FieldValueNode).getField().getAnAssignedValue() = n1.asExpr()
193+
or
194+
n2.asExpr().(FieldRead).getField() = n1.(FieldValueNode).getField()
196195
or
197196
exists(EnumType enum, Method getValue |
198197
enum.getAnEnumConstant().getAnAssignedValue() = n1.asExpr() and

‎java/ql/lib/semmle/code/java/dispatch/ObjFlow.qll

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,9 @@ private predicate step(Node n1, Node n2) {
9494
n2.(ImplicitInstanceAccess).getInstanceAccess().(OwnInstanceAccess).getEnclosingCallable() = c
9595
)
9696
or
97-
exists(Field f |
98-
f.getAnAssignedValue() = n1.asExpr() and
99-
n2.asExpr().(FieldRead).getField() = f
100-
)
97+
n2.(FieldValueNode).getField().getAnAssignedValue() = n1.asExpr()
98+
or
99+
n2.asExpr().(FieldRead).getField() = n1.(FieldValueNode).getField()
101100
or
102101
n2.asExpr().(CastExpr).getExpr() = n1.asExpr()
103102
or
@@ -132,7 +131,7 @@ private predicate step(Node n1, Node n2) {
132131
or
133132
exists(Field v |
134133
containerStep(n1.asExpr(), v.getAnAccess()) and
135-
n2.asExpr() = v.getAnAccess()
134+
n2.(FieldValueNode).getField() = v
136135
)
137136
}
138137

‎java/ql/src/Telemetry/ExternalAPI.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ class ExternalAPI extends Callable {
6262

6363
/** Holds if this API has a supported summary. */
6464
predicate hasSummary() {
65-
this instanceof SummarizedCallable or
65+
this = any(SummarizedCallable sc).asCallable() or
6666
TaintTracking::localAdditionalTaintStep(this.getAnInput(), _)
6767
}
6868

‎java/ql/test/query-tests/security/CWE-297/UnsafeHostnameVerification.expected

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
edges
22
| UnsafeHostnameVerification.java:66:37:80:9 | new (...) : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:81:55:81:62 | verifier |
33
| UnsafeHostnameVerification.java:88:37:93:9 | new (...) : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:94:55:94:62 | verifier |
4-
| UnsafeHostnameVerification.java:97:72:102:5 | new (...) : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:34:59:34:85 | ALLOW_ALL_HOSTNAME_VERIFIER |
4+
| UnsafeHostnameVerification.java:97:42:97:68 | ALLOW_ALL_HOSTNAME_VERIFIER : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:34:59:34:85 | ALLOW_ALL_HOSTNAME_VERIFIER |
5+
| UnsafeHostnameVerification.java:97:72:102:5 | new (...) : new HostnameVerifier(...) { ... } | UnsafeHostnameVerification.java:97:42:97:68 | ALLOW_ALL_HOSTNAME_VERIFIER : new HostnameVerifier(...) { ... } |
56
nodes
67
| UnsafeHostnameVerification.java:14:55:19:9 | new (...) | semmle.label | new (...) |
78
| UnsafeHostnameVerification.java:26:55:26:71 | ...->... | semmle.label | ...->... |
@@ -12,6 +13,7 @@ nodes
1213
| UnsafeHostnameVerification.java:81:55:81:62 | verifier | semmle.label | verifier |
1314
| UnsafeHostnameVerification.java:88:37:93:9 | new (...) : new HostnameVerifier(...) { ... } | semmle.label | new (...) : new HostnameVerifier(...) { ... } |
1415
| UnsafeHostnameVerification.java:94:55:94:62 | verifier | semmle.label | verifier |
16+
| UnsafeHostnameVerification.java:97:42:97:68 | ALLOW_ALL_HOSTNAME_VERIFIER : new HostnameVerifier(...) { ... } | semmle.label | ALLOW_ALL_HOSTNAME_VERIFIER : new HostnameVerifier(...) { ... } |
1517
| UnsafeHostnameVerification.java:97:72:102:5 | new (...) : new HostnameVerifier(...) { ... } | semmle.label | new (...) : new HostnameVerifier(...) { ... } |
1618
subpaths
1719
#select

‎java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsApiCall.expected

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
edges
2-
| CredentialsTest.java:7:34:7:41 | "123456" : String | CredentialsTest.java:13:39:13:39 | p |
3-
| CredentialsTest.java:7:34:7:41 | "123456" : String | CredentialsTest.java:14:16:14:16 | p : String |
2+
| CredentialsTest.java:7:30:7:30 | p : String | CredentialsTest.java:13:39:13:39 | p |
3+
| CredentialsTest.java:7:30:7:30 | p : String | CredentialsTest.java:14:16:14:16 | p : String |
4+
| CredentialsTest.java:7:34:7:41 | "123456" : String | CredentialsTest.java:7:30:7:30 | p : String |
45
| CredentialsTest.java:11:14:11:20 | "admin" : String | CredentialsTest.java:13:36:13:36 | u |
56
| CredentialsTest.java:11:14:11:20 | "admin" : String | CredentialsTest.java:14:13:14:13 | u : String |
67
| CredentialsTest.java:14:13:14:13 | u : String | CredentialsTest.java:17:38:17:45 | v : String |
@@ -44,6 +45,7 @@ edges
4445
| Test.java:29:38:29:48 | user : String | Test.java:30:36:30:39 | user |
4546
| Test.java:29:51:29:65 | password : String | Test.java:30:42:30:49 | password |
4647
nodes
48+
| CredentialsTest.java:7:30:7:30 | p : String | semmle.label | p : String |
4749
| CredentialsTest.java:7:34:7:41 | "123456" : String | semmle.label | "123456" : String |
4850
| CredentialsTest.java:11:14:11:20 | "admin" : String | semmle.label | "admin" : String |
4951
| CredentialsTest.java:13:36:13:36 | u | semmle.label | u |

‎java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsSourceCall.expected

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ edges
1212
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [clientSecret] : String | HardcodedAzureCredentials.java:15:14:15:42 | parameter this [clientSecret] : String |
1313
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [username] : String | HardcodedAzureCredentials.java:15:14:15:42 | parameter this [username] : String |
1414
| Test.java:10:17:10:24 | "123456" : String | Test.java:26:17:26:20 | pass |
15-
| User.java:2:43:2:50 | "123456" : String | User.java:5:15:5:24 | DEFAULT_PW |
15+
| User.java:2:30:2:39 | DEFAULT_PW : String | User.java:5:15:5:24 | DEFAULT_PW |
16+
| User.java:2:43:2:50 | "123456" : String | User.java:2:30:2:39 | DEFAULT_PW : String |
1617
nodes
1718
| HardcodedAzureCredentials.java:8:14:8:38 | this <.method> [post update] [clientSecret] : String | semmle.label | this <.method> [post update] [clientSecret] : String |
1819
| HardcodedAzureCredentials.java:8:14:8:38 | this <.method> [post update] [username] : String | semmle.label | this <.method> [post update] [username] : String |
@@ -30,6 +31,7 @@ nodes
3031
| HardcodedAzureCredentials.java:61:3:61:33 | new HardcodedAzureCredentials(...) [username] : String | semmle.label | new HardcodedAzureCredentials(...) [username] : String |
3132
| Test.java:10:17:10:24 | "123456" : String | semmle.label | "123456" : String |
3233
| Test.java:26:17:26:20 | pass | semmle.label | pass |
34+
| User.java:2:30:2:39 | DEFAULT_PW : String | semmle.label | DEFAULT_PW : String |
3335
| User.java:2:43:2:50 | "123456" : String | semmle.label | "123456" : String |
3436
| User.java:5:15:5:24 | DEFAULT_PW | semmle.label | DEFAULT_PW |
3537
subpaths

0 commit comments

Comments
 (0)