Skip to content

Commit b148e31

Browse files
smowtonsmowton
committed
Java models-as-data: infer Kotlin $default models from that of its parent function
1 parent 73f977c commit b148e31

File tree

7 files changed

+314
-12
lines changed

7 files changed

+314
-12
lines changed

‎java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/lib.kt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
class ConstructorWithDefaults(x: Int, y: Int = 1) { }
2+
3+
fun topLevelWithDefaults(x: Int, y: Int = 1) = 0
4+
fun String.extensionWithDefaults(x: Int, y: Int = 1) = 0
5+
6+
class LibClass {
7+
8+
fun memberWithDefaults(x: Int, y: Int = 1) = 0
9+
fun String.extensionMemberWithDefaults(x: Int, y: Int = 1) = 0
10+
11+
}
12+
13+
class SomeToken {}
14+
15+
fun topLevelArgSource(st: SomeToken, x: Int = 0) {}
16+
fun String.extensionArgSource(st: SomeToken, x: Int = 0) {}
17+
18+
class SourceClass {
19+
20+
fun memberArgSource(st: SomeToken, x: Int = 0) {}
21+
22+
}
23+
24+
fun topLevelSink(x: Int, y: Int = 1) {}
25+
fun String.extensionSink(x: Int, y: Int = 1) {}
26+
27+
class SinkClass(x: Int, y: Int = 1) {
28+
29+
fun memberSink(x: Int, y: Int = 1) {}
30+
fun String.extensionMemberSink(x: Int, y: Int = 1) {}
31+
32+
}
33+

‎java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/test.expected

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
edges
2+
| user.kt:7:32:7:39 | source(...) : Number | user.kt:7:8:7:43 | new ConstructorWithDefaults(...) |
3+
| user.kt:8:32:8:39 | source(...) : Number | user.kt:8:8:8:40 | new ConstructorWithDefaults(...) |
4+
| user.kt:10:29:10:36 | source(...) : Number | user.kt:10:8:10:40 | topLevelWithDefaults(...) |
5+
| user.kt:11:29:11:36 | source(...) : Number | user.kt:11:8:11:37 | topLevelWithDefaults$default(...) |
6+
| user.kt:13:44:13:51 | source(...) : Number | user.kt:13:22:13:55 | extensionWithDefaults(...) |
7+
| user.kt:14:44:14:51 | source(...) : Number | user.kt:14:22:14:52 | extensionWithDefaults$default(...) |
8+
| user.kt:16:29:16:36 | source(...) : Number | user.kt:16:10:16:40 | memberWithDefaults(...) |
9+
| user.kt:17:29:17:36 | source(...) : Number | user.kt:17:10:17:37 | memberWithDefaults$default(...) |
10+
| user.kt:20:52:20:59 | source(...) : Number | user.kt:20:24:20:63 | extensionMemberWithDefaults(...) |
11+
| user.kt:21:52:21:59 | source(...) : Number | user.kt:21:24:21:60 | extensionMemberWithDefaults$default(...) |
12+
| user.kt:26:23:26:24 | st [post update] : SomeToken | user.kt:27:10:27:11 | st |
13+
| user.kt:32:38:32:39 | st [post update] : SomeToken | user.kt:33:10:33:11 | st |
14+
| user.kt:38:29:38:30 | st [post update] : SomeToken | user.kt:39:10:39:11 | st |
15+
nodes
16+
| user.kt:7:8:7:43 | new ConstructorWithDefaults(...) | semmle.label | new ConstructorWithDefaults(...) |
17+
| user.kt:7:32:7:39 | source(...) : Number | semmle.label | source(...) : Number |
18+
| user.kt:8:8:8:40 | new ConstructorWithDefaults(...) | semmle.label | new ConstructorWithDefaults(...) |
19+
| user.kt:8:32:8:39 | source(...) : Number | semmle.label | source(...) : Number |
20+
| user.kt:10:8:10:40 | topLevelWithDefaults(...) | semmle.label | topLevelWithDefaults(...) |
21+
| user.kt:10:29:10:36 | source(...) : Number | semmle.label | source(...) : Number |
22+
| user.kt:11:8:11:37 | topLevelWithDefaults$default(...) | semmle.label | topLevelWithDefaults$default(...) |
23+
| user.kt:11:29:11:36 | source(...) : Number | semmle.label | source(...) : Number |
24+
| user.kt:13:22:13:55 | extensionWithDefaults(...) | semmle.label | extensionWithDefaults(...) |
25+
| user.kt:13:44:13:51 | source(...) : Number | semmle.label | source(...) : Number |
26+
| user.kt:14:22:14:52 | extensionWithDefaults$default(...) | semmle.label | extensionWithDefaults$default(...) |
27+
| user.kt:14:44:14:51 | source(...) : Number | semmle.label | source(...) : Number |
28+
| user.kt:16:10:16:40 | memberWithDefaults(...) | semmle.label | memberWithDefaults(...) |
29+
| user.kt:16:29:16:36 | source(...) : Number | semmle.label | source(...) : Number |
30+
| user.kt:17:10:17:37 | memberWithDefaults$default(...) | semmle.label | memberWithDefaults$default(...) |
31+
| user.kt:17:29:17:36 | source(...) : Number | semmle.label | source(...) : Number |
32+
| user.kt:20:24:20:63 | extensionMemberWithDefaults(...) | semmle.label | extensionMemberWithDefaults(...) |
33+
| user.kt:20:52:20:59 | source(...) : Number | semmle.label | source(...) : Number |
34+
| user.kt:21:24:21:60 | extensionMemberWithDefaults$default(...) | semmle.label | extensionMemberWithDefaults$default(...) |
35+
| user.kt:21:52:21:59 | source(...) : Number | semmle.label | source(...) : Number |
36+
| user.kt:26:23:26:24 | st [post update] : SomeToken | semmle.label | st [post update] : SomeToken |
37+
| user.kt:27:10:27:11 | st | semmle.label | st |
38+
| user.kt:32:38:32:39 | st [post update] : SomeToken | semmle.label | st [post update] : SomeToken |
39+
| user.kt:33:10:33:11 | st | semmle.label | st |
40+
| user.kt:38:29:38:30 | st [post update] : SomeToken | semmle.label | st [post update] : SomeToken |
41+
| user.kt:39:10:39:11 | st | semmle.label | st |
42+
| user.kt:42:13:42:20 | source(...) | semmle.label | source(...) |
43+
| user.kt:43:16:43:23 | source(...) | semmle.label | source(...) |
44+
| user.kt:44:31:44:38 | source(...) | semmle.label | source(...) |
45+
| user.kt:45:20:45:27 | source(...) | semmle.label | source(...) |
46+
| user.kt:47:39:47:46 | source(...) | semmle.label | source(...) |
47+
subpaths
48+
#select
49+
| user.kt:7:32:7:39 | source(...) : Number | user.kt:7:32:7:39 | source(...) : Number | user.kt:7:8:7:43 | new ConstructorWithDefaults(...) | flow path |
50+
| user.kt:8:32:8:39 | source(...) : Number | user.kt:8:32:8:39 | source(...) : Number | user.kt:8:8:8:40 | new ConstructorWithDefaults(...) | flow path |
51+
| user.kt:10:29:10:36 | source(...) : Number | user.kt:10:29:10:36 | source(...) : Number | user.kt:10:8:10:40 | topLevelWithDefaults(...) | flow path |
52+
| user.kt:11:29:11:36 | source(...) : Number | user.kt:11:29:11:36 | source(...) : Number | user.kt:11:8:11:37 | topLevelWithDefaults$default(...) | flow path |
53+
| user.kt:13:44:13:51 | source(...) : Number | user.kt:13:44:13:51 | source(...) : Number | user.kt:13:22:13:55 | extensionWithDefaults(...) | flow path |
54+
| user.kt:14:44:14:51 | source(...) : Number | user.kt:14:44:14:51 | source(...) : Number | user.kt:14:22:14:52 | extensionWithDefaults$default(...) | flow path |
55+
| user.kt:16:29:16:36 | source(...) : Number | user.kt:16:29:16:36 | source(...) : Number | user.kt:16:10:16:40 | memberWithDefaults(...) | flow path |
56+
| user.kt:17:29:17:36 | source(...) : Number | user.kt:17:29:17:36 | source(...) : Number | user.kt:17:10:17:37 | memberWithDefaults$default(...) | flow path |
57+
| user.kt:20:52:20:59 | source(...) : Number | user.kt:20:52:20:59 | source(...) : Number | user.kt:20:24:20:63 | extensionMemberWithDefaults(...) | flow path |
58+
| user.kt:21:52:21:59 | source(...) : Number | user.kt:21:52:21:59 | source(...) : Number | user.kt:21:24:21:60 | extensionMemberWithDefaults$default(...) | flow path |
59+
| user.kt:26:23:26:24 | st [post update] : SomeToken | user.kt:26:23:26:24 | st [post update] : SomeToken | user.kt:27:10:27:11 | st | flow path |
60+
| user.kt:32:38:32:39 | st [post update] : SomeToken | user.kt:32:38:32:39 | st [post update] : SomeToken | user.kt:33:10:33:11 | st | flow path |
61+
| user.kt:38:29:38:30 | st [post update] : SomeToken | user.kt:38:29:38:30 | st [post update] : SomeToken | user.kt:39:10:39:11 | st | flow path |
62+
| user.kt:42:13:42:20 | source(...) | user.kt:42:13:42:20 | source(...) | user.kt:42:13:42:20 | source(...) | flow path |
63+
| user.kt:43:16:43:23 | source(...) | user.kt:43:16:43:23 | source(...) | user.kt:43:16:43:23 | source(...) | flow path |
64+
| user.kt:44:31:44:38 | source(...) | user.kt:44:31:44:38 | source(...) | user.kt:44:31:44:38 | source(...) | flow path |
65+
| user.kt:45:20:45:27 | source(...) | user.kt:45:20:45:27 | source(...) | user.kt:45:20:45:27 | source(...) | flow path |
66+
| user.kt:47:39:47:46 | source(...) | user.kt:47:39:47:46 | source(...) | user.kt:47:39:47:46 | source(...) | flow path |

‎java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/test.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
from create_database_utils import *
2+
import subprocess
3+
4+
subprocess.check_call(["kotlinc", "lib.kt", "-d", "lib"])
5+
run_codeql_database_create(["kotlinc user.kt -cp lib"], lang="java")

‎java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/test.ql

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import java
2+
import semmle.code.java.dataflow.TaintTracking
3+
import DataFlow::PathGraph
4+
private import semmle.code.java.dataflow.ExternalFlow
5+
6+
private class Models extends SummaryModelCsv {
7+
override predicate row(string row) {
8+
row =
9+
[
10+
";ConstructorWithDefaults;true;ConstructorWithDefaults;(int,int);;Argument[0];Argument[-1];taint;manual",
11+
";LibKt;true;topLevelWithDefaults;(int,int);;Argument[0];ReturnValue;value;manual",
12+
";LibKt;true;extensionWithDefaults;(String,int,int);;Argument[1];ReturnValue;value;manual",
13+
";LibClass;true;memberWithDefaults;(int,int);;Argument[0];ReturnValue;value;manual",
14+
";LibClass;true;extensionMemberWithDefaults;(String,int,int);;Argument[1];ReturnValue;value;manual"
15+
]
16+
}
17+
}
18+
19+
private class SourceModels extends SourceModelCsv {
20+
override predicate row(string row) {
21+
row =
22+
[
23+
";LibKt;true;topLevelArgSource;(SomeToken,int);;Argument[0];kotlinMadFlowTest;manual",
24+
";LibKt;true;extensionArgSource;(String,SomeToken,int);;Argument[1];kotlinMadFlowTest;manual",
25+
";SourceClass;true;memberArgSource;(SomeToken,int);;Argument[0];kotlinMadFlowTest;manual"
26+
]
27+
}
28+
}
29+
30+
private class SinkModels extends SinkModelCsv {
31+
override predicate row(string row) {
32+
row =
33+
[
34+
";SinkClass;true;SinkClass;(int,int);;Argument[0];kotlinMadFlowTest;manual",
35+
";LibKt;true;topLevelSink;(int,int);;Argument[0];kotlinMadFlowTest;manual",
36+
";LibKt;true;extensionSink;(String,int,int);;Argument[1];kotlinMadFlowTest;manual",
37+
";SinkClass;true;memberSink;(int,int);;Argument[0];kotlinMadFlowTest;manual",
38+
";SinkClass;true;extensionMemberSink;(String,int,int);;Argument[1];kotlinMadFlowTest;manual"
39+
]
40+
}
41+
}
42+
43+
class Config extends TaintTracking::Configuration {
44+
Config() { this = "Config" }
45+
46+
override predicate isSource(DataFlow::Node n) {
47+
n.asExpr().(MethodAccess).getCallee().getName() = "source"
48+
or
49+
sourceNode(n, "kotlinMadFlowTest")
50+
}
51+
52+
override predicate isSink(DataFlow::Node n) {
53+
n.asExpr().(Argument).getCall().getCallee().getName() = "sink"
54+
or
55+
sinkNode(n, "kotlinMadFlowTest")
56+
}
57+
}
58+
59+
from DataFlow::PathNode source, DataFlow::PathNode sink, Config c
60+
where c.hasFlowPath(source, sink)
61+
select source, source, sink, "flow path"

‎java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/user.kt

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
fun source() = 1
2+
3+
fun sink(x: Any) { }
4+
5+
fun test(c: LibClass, sourcec: SourceClass, sinkc: SinkClass) {
6+
7+
sink(ConstructorWithDefaults(source(), 0))
8+
sink(ConstructorWithDefaults(source()))
9+
10+
sink(topLevelWithDefaults(source(), 0))
11+
sink(topLevelWithDefaults(source()))
12+
13+
sink("Hello world".extensionWithDefaults(source(), 0))
14+
sink("Hello world".extensionWithDefaults(source()))
15+
16+
sink(c.memberWithDefaults(source(), 0))
17+
sink(c.memberWithDefaults(source()))
18+
19+
with(c) {
20+
sink("Hello world".extensionMemberWithDefaults(source(), 0))
21+
sink("Hello world".extensionMemberWithDefaults(source()))
22+
};
23+
24+
run {
25+
val st = SomeToken()
26+
topLevelArgSource(st)
27+
sink(st)
28+
}
29+
30+
run {
31+
val st = SomeToken()
32+
"Hello world".extensionArgSource(st)
33+
sink(st)
34+
}
35+
36+
run {
37+
val st = SomeToken()
38+
sourcec.memberArgSource(st)
39+
sink(st)
40+
}
41+
42+
SinkClass(source())
43+
topLevelSink(source())
44+
"Hello world".extensionSink(source())
45+
sinkc.memberSink(source())
46+
with(sinkc) {
47+
"Hello world".extensionMemberSink(source())
48+
}
49+
50+
}

‎java/ql/lib/semmle/code/java/Member.qll

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,9 @@ class Callable extends StmtParent, Member, @callable {
306306
*/
307307
Callable getKotlinParameterDefaultsProxy() {
308308
this.getDeclaringType() = result.getDeclaringType() and
309-
exists(int proxyNParams, int extraLeadingParams, RefType lastParamType |
309+
exists(
310+
int proxyNParams, int extraLeadingParams, int regularParamsStartIdx, RefType lastParamType
311+
|
310312
proxyNParams = result.getNumberOfParameters() and
311313
extraLeadingParams = (proxyNParams - this.getNumberOfParameters()) - 2 and
312314
extraLeadingParams >= 0 and
@@ -316,16 +318,28 @@ class Callable extends StmtParent, Member, @callable {
316318
this instanceof Constructor and
317319
result instanceof Constructor and
318320
extraLeadingParams = 0 and
321+
regularParamsStartIdx = 0 and
319322
lastParamType.hasQualifiedName("kotlin.jvm.internal", "DefaultConstructorMarker")
320323
or
321324
this instanceof Method and
322325
result instanceof Method and
323326
this.getName() + "$default" = result.getName() and
324-
extraLeadingParams <= 2 and
327+
extraLeadingParams <= 1 and
328+
(
329+
if ktExtensionFunctions(this, _, _)
330+
then
331+
// Both extension receivers are expected to occur at arg0, with any
332+
// dispatch receiver inserted afterwards in the $default proxy's parameter list.
333+
// Check the extension receiver matches here, and note regular args
334+
// are bumped one position to the right.
335+
regularParamsStartIdx = extraLeadingParams + 1 and
336+
this.getParameterType(0).getErasure() = eraseRaw(result.getParameterType(0))
337+
else regularParamsStartIdx = extraLeadingParams
338+
) and
325339
lastParamType instanceof TypeObject
326340
)
327341
|
328-
forall(int paramIdx | paramIdx in [extraLeadingParams .. proxyNParams - 3] |
342+
forall(int paramIdx | paramIdx in [regularParamsStartIdx .. proxyNParams - 3] |
329343
this.getParameterType(paramIdx - extraLeadingParams).getErasure() =
330344
eraseRaw(result.getParameterType(paramIdx))
331345
)

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

Lines changed: 82 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,61 @@ private boolean isGenerated(string provenance) {
6565
provenance != "generated" and result = false
6666
}
6767

68+
private predicate relatedArgSpec(Callable c, string spec) {
69+
exists(
70+
string namespace, string type, boolean subtypes, string name, string signature, string ext
71+
|
72+
summaryModel(namespace, type, subtypes, name, signature, ext, spec, _, _, _) or
73+
summaryModel(namespace, type, subtypes, name, signature, ext, _, spec, _, _) or
74+
sourceModel(namespace, type, subtypes, name, signature, ext, spec, _, _) or
75+
sinkModel(namespace, type, subtypes, name, signature, ext, spec, _, _)
76+
|
77+
c = interpretElement(namespace, type, subtypes, name, signature, ext)
78+
)
79+
}
80+
81+
/**
82+
* Holds if `defaultsCallable` is a Kotlin default-parameter proxy for `originalCallable`, and
83+
* `originalCallable` has a model, and `defaultsArgSpec` is `originalArgSpec` adjusted to account
84+
* for the additional dispatch receiver parameter that occurs in the default-parameter proxy's argument
85+
* list. When no adjustment is required (e.g. for constructors, or non-argument-based specs), `defaultArgsSpec`
86+
* equals `originalArgSpec`.
87+
*/
88+
private predicate correspondingKotlinParameterDefaultsArgSpec(
89+
Callable originalCallable, Callable defaultsCallable, string originalArgSpec,
90+
string defaultsArgSpec
91+
) {
92+
relatedArgSpec(originalCallable, originalArgSpec) and
93+
defaultsCallable = originalCallable.getKotlinParameterDefaultsProxy() and
94+
(
95+
originalCallable instanceof Constructor and originalArgSpec = defaultsArgSpec
96+
or
97+
originalCallable instanceof Method and
98+
exists(string regex |
99+
// Note I use a regex and not AccessPathToken because this feeds summaryElement et al,
100+
// which would introduce mutual recursion with the definition of AccessPathToken.
101+
regex = "Argument\\[([0-9]+)\\](.*)" and
102+
(
103+
exists(string oldArgNumber, string rest, int paramOffset |
104+
oldArgNumber = originalArgSpec.regexpCapture(regex, 1) and
105+
rest = originalArgSpec.regexpCapture(regex, 2) and
106+
paramOffset =
107+
(
108+
defaultsCallable.getNumberOfParameters() -
109+
(originalCallable.getNumberOfParameters() + 2)
110+
) and
111+
if ktExtensionFunctions(originalCallable, _, _) and oldArgNumber = "0"
112+
then defaultsArgSpec = originalArgSpec
113+
else defaultsArgSpec = "Argument[" + (oldArgNumber.toInt() + paramOffset) + "]" + rest
114+
)
115+
or
116+
not originalArgSpec.regexpMatch(regex) and
117+
defaultsArgSpec = originalArgSpec
118+
)
119+
)
120+
)
121+
}
122+
68123
/**
69124
* Holds if an external flow summary exists for `c` with input specification
70125
* `input`, output specification `output`, kind `kind`, and a flag `generated`
@@ -75,11 +130,19 @@ predicate summaryElement(
75130
) {
76131
exists(
77132
string namespace, string type, boolean subtypes, string name, string signature, string ext,
78-
string provenance
133+
string provenance, string originalInput, string originalOutput, Callable baseCallable
79134
|
80-
summaryModel(namespace, type, subtypes, name, signature, ext, input, output, kind, provenance) and
135+
summaryModel(namespace, type, subtypes, name, signature, ext, originalInput, originalOutput,
136+
kind, provenance) and
81137
generated = isGenerated(provenance) and
82-
c.asCallable() = interpretElement(namespace, type, subtypes, name, signature, ext)
138+
baseCallable = interpretElement(namespace, type, subtypes, name, signature, ext) and
139+
(
140+
c.asCallable() = baseCallable and input = originalInput and output = originalOutput
141+
or
142+
correspondingKotlinParameterDefaultsArgSpec(baseCallable, c.asCallable(), originalInput, input) and
143+
correspondingKotlinParameterDefaultsArgSpec(baseCallable, c.asCallable(), originalOutput,
144+
output)
145+
)
83146
)
84147
}
85148

@@ -149,11 +212,16 @@ class SourceOrSinkElement = Top;
149212
predicate sourceElement(SourceOrSinkElement e, string output, string kind, boolean generated) {
150213
exists(
151214
string namespace, string type, boolean subtypes, string name, string signature, string ext,
152-
string provenance
215+
string provenance, SourceOrSinkElement baseSource, string originalOutput
153216
|
154-
sourceModel(namespace, type, subtypes, name, signature, ext, output, kind, provenance) and
217+
sourceModel(namespace, type, subtypes, name, signature, ext, originalOutput, kind, provenance) and
155218
generated = isGenerated(provenance) and
156-
e = interpretElement(namespace, type, subtypes, name, signature, ext)
219+
baseSource = interpretElement(namespace, type, subtypes, name, signature, ext) and
220+
(
221+
e = baseSource and output = originalOutput
222+
or
223+
correspondingKotlinParameterDefaultsArgSpec(baseSource, e, originalOutput, output)
224+
)
157225
)
158226
}
159227

@@ -165,11 +233,16 @@ predicate sourceElement(SourceOrSinkElement e, string output, string kind, boole
165233
predicate sinkElement(SourceOrSinkElement e, string input, string kind, boolean generated) {
166234
exists(
167235
string namespace, string type, boolean subtypes, string name, string signature, string ext,
168-
string provenance
236+
string provenance, SourceOrSinkElement baseSink, string originalInput
169237
|
170-
sinkModel(namespace, type, subtypes, name, signature, ext, input, kind, provenance) and
238+
sinkModel(namespace, type, subtypes, name, signature, ext, originalInput, kind, provenance) and
171239
generated = isGenerated(provenance) and
172-
e = interpretElement(namespace, type, subtypes, name, signature, ext)
240+
baseSink = interpretElement(namespace, type, subtypes, name, signature, ext) and
241+
(
242+
e = baseSink and originalInput = input
243+
or
244+
correspondingKotlinParameterDefaultsArgSpec(baseSink, e, originalInput, input)
245+
)
173246
)
174247
}
175248

0 commit comments

Comments
 (0)