Skip to content

Commit a95f412

Browse files
max-schaefermax-schaefer
authored
Merge pull request #15554 from github/max-schaefer/automodel-candidate-fixes
Automodel: Improve handling of varargs and overriding in extraction queries
2 parents c6f4a20 + 652b6bb commit a95f412

File tree

5 files changed

+116
-49
lines changed

5 files changed

+116
-49
lines changed

‎java/ql/automodel/src/AutomodelApplicationModeCharacteristics.qll‎

Lines changed: 49 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,10 @@ newtype TApplicationModeEndpoint =
3535
arg = DataFlow::getInstanceArgument(call) and
3636
not call instanceof ConstructorCall
3737
} or
38-
TImplicitVarargsArray(Call call, DataFlow::Node arg, int idx) {
38+
TImplicitVarargsArray(Call call, DataFlow::ImplicitVarargsArray arg, int idx) {
3939
AutomodelJavaUtil::isFromSource(call) and
40-
exists(Argument argExpr |
41-
arg.asExpr() = argExpr and
42-
call.getArgument(idx) = argExpr and
43-
argExpr.isVararg() and
44-
not exists(int i | i < idx and call.getArgument(i).(Argument).isVararg())
45-
)
40+
call = arg.getCall() and
41+
idx = call.getCallee().getVaragsParameterIndex()
4642
} or
4743
TMethodReturnValue(Call call) {
4844
AutomodelJavaUtil::isFromSource(call) and
@@ -255,45 +251,74 @@ module ApplicationCandidatesImpl implements SharedCharacteristics::CandidateSig
255251
predicate isKnownKind = AutomodelJavaUtil::isKnownKind/2;
256252

257253
predicate isSink(Endpoint e, string kind, string provenance) {
258-
exists(string package, string type, string name, string signature, string ext, string input |
259-
sinkSpec(e, package, type, name, signature, ext, input) and
260-
ExternalFlow::sinkModel(package, type, _, name, [signature, ""], ext, input, kind, provenance)
254+
exists(
255+
string package, string type, boolean subtypes, string name, string signature, string ext,
256+
string input
257+
|
258+
sinkSpec(e, package, type, subtypes, name, signature, ext, input) and
259+
ExternalFlow::sinkModel(package, type, subtypes, name, [signature, ""], ext, input, kind,
260+
provenance)
261261
)
262262
or
263263
isCustomSink(e, kind) and provenance = "custom-sink"
264264
}
265265

266266
predicate isSource(Endpoint e, string kind, string provenance) {
267-
exists(string package, string type, string name, string signature, string ext, string output |
268-
sourceSpec(e, package, type, name, signature, ext, output) and
269-
ExternalFlow::sourceModel(package, type, _, name, [signature, ""], ext, output, kind,
267+
exists(
268+
string package, string type, boolean subtypes, string name, string signature, string ext,
269+
string output
270+
|
271+
sourceSpec(e, package, type, subtypes, name, signature, ext, output) and
272+
ExternalFlow::sourceModel(package, type, subtypes, name, [signature, ""], ext, output, kind,
270273
provenance)
271274
)
272275
}
273276

274277
predicate isNeutral(Endpoint e) {
275-
exists(string package, string type, string name, string signature |
276-
sinkSpec(e, package, type, name, signature, _, _) and
277-
ExternalFlow::neutralModel(package, type, name, [signature, ""], "sink", _)
278+
exists(string package, string type, string name, string signature, string endpointType |
279+
sinkSpec(e, package, type, _, name, signature, _, _) and
280+
endpointType = "sink"
281+
or
282+
sourceSpec(e, package, type, _, name, signature, _, _) and
283+
endpointType = "source"
284+
|
285+
ExternalFlow::neutralModel(package, type, name, [signature, ""], endpointType, _)
286+
)
287+
}
288+
289+
/**
290+
* Holds if the endpoint concerns a callable with the given package, type, name and signature.
291+
*
292+
* If `subtypes` is `false`, only the exact callable is considered. If `true`, the callable and
293+
* all its overrides are considered.
294+
*/
295+
additional predicate endpointCallable(
296+
Endpoint e, string package, string type, boolean subtypes, string name, string signature
297+
) {
298+
exists(Callable c |
299+
c = e.getCallable() and subtypes in [true, false]
300+
or
301+
e.getCallable().(Method).getSourceDeclaration().overrides+(c) and subtypes = true
302+
|
303+
c.hasQualifiedName(package, type, name) and
304+
signature = ExternalFlow::paramsString(c)
278305
)
279306
}
280307

281-
// XXX how to extend to support sources?
282308
additional predicate sinkSpec(
283-
Endpoint e, string package, string type, string name, string signature, string ext, string input
309+
Endpoint e, string package, string type, boolean subtypes, string name, string signature,
310+
string ext, string input
284311
) {
285-
e.getCallable().hasQualifiedName(package, type, name) and
286-
signature = ExternalFlow::paramsString(e.getCallable()) and
312+
endpointCallable(e, package, type, subtypes, name, signature) and
287313
ext = "" and
288314
input = e.getMaDInput()
289315
}
290316

291317
additional predicate sourceSpec(
292-
Endpoint e, string package, string type, string name, string signature, string ext,
293-
string output
318+
Endpoint e, string package, string type, boolean subtypes, string name, string signature,
319+
string ext, string output
294320
) {
295-
e.getCallable().hasQualifiedName(package, type, name) and
296-
signature = ExternalFlow::paramsString(e.getCallable()) and
321+
endpointCallable(e, package, type, subtypes, name, signature) and
297322
ext = "" and
298323
output = e.getMaDOutput()
299324
}

‎java/ql/automodel/src/AutomodelFrameworkModeCharacteristics.qll‎

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -209,46 +209,72 @@ module FrameworkCandidatesImpl implements SharedCharacteristics::CandidateSig {
209209
predicate isKnownKind = AutomodelJavaUtil::isKnownKind/2;
210210

211211
predicate isSink(Endpoint e, string kind, string provenance) {
212-
exists(string package, string type, string name, string signature, string ext, string input |
213-
sinkSpec(e, package, type, name, signature, ext, input) and
214-
ExternalFlow::sinkModel(package, type, _, name, [signature, ""], ext, input, kind, provenance)
212+
exists(
213+
string package, string type, boolean subtypes, string name, string signature, string ext,
214+
string input
215+
|
216+
sinkSpec(e, package, type, subtypes, name, signature, ext, input) and
217+
ExternalFlow::sinkModel(package, type, subtypes, name, [signature, ""], ext, input, kind,
218+
provenance)
215219
)
216220
}
217221

218222
predicate isSource(Endpoint e, string kind, string provenance) {
219-
exists(string package, string type, string name, string signature, string ext, string output |
220-
sourceSpec(e, package, type, name, signature, ext, output) and
221-
ExternalFlow::sourceModel(package, type, _, name, [signature, ""], ext, output, kind,
223+
exists(
224+
string package, string type, boolean subtypes, string name, string signature, string ext,
225+
string output
226+
|
227+
sourceSpec(e, package, type, subtypes, name, signature, ext, output) and
228+
ExternalFlow::sourceModel(package, type, subtypes, name, [signature, ""], ext, output, kind,
222229
provenance)
223230
)
224231
}
225232

226233
predicate isNeutral(Endpoint e) {
227-
exists(string package, string type, string name, string signature |
228-
(
229-
sinkSpec(e, package, type, name, signature, _, _)
230-
or
231-
sourceSpec(e, package, type, name, signature, _, _)
232-
) and
233-
ExternalFlow::neutralModel(package, type, name, [signature, ""], "sink", _)
234+
exists(string package, string type, string name, string signature, string endpointType |
235+
sinkSpec(e, package, type, _, name, signature, _, _) and
236+
endpointType = "sink"
237+
or
238+
sourceSpec(e, package, type, _, name, signature, _, _) and
239+
endpointType = "source"
240+
|
241+
ExternalFlow::neutralModel(package, type, name, [signature, ""], endpointType, _)
242+
)
243+
}
244+
245+
/**
246+
* Holds if the endpoint concerns a callable with the given package, type, name and signature.
247+
*
248+
* If `subtypes` is `false`, only the exact callable is considered. If `true`, the callable and
249+
* all its overrides are considered.
250+
*/
251+
additional predicate endpointCallable(
252+
Endpoint e, string package, string type, boolean subtypes, string name, string signature
253+
) {
254+
exists(Callable c |
255+
c = e.getCallable() and subtypes in [true, false]
256+
or
257+
e.getCallable().(Method).getSourceDeclaration().overrides+(c) and subtypes = true
258+
|
259+
c.hasQualifiedName(package, type, name) and
260+
signature = ExternalFlow::paramsString(c)
234261
)
235262
}
236263

237264
additional predicate sinkSpec(
238-
Endpoint e, string package, string type, string name, string signature, string ext, string input
265+
Endpoint e, string package, string type, boolean subtypes, string name, string signature,
266+
string ext, string input
239267
) {
240-
e.getCallable().hasQualifiedName(package, type, name) and
241-
signature = ExternalFlow::paramsString(e.getCallable()) and
268+
endpointCallable(e, package, type, subtypes, name, signature) and
242269
ext = "" and
243270
input = e.getMaDInput()
244271
}
245272

246273
additional predicate sourceSpec(
247-
Endpoint e, string package, string type, string name, string signature, string ext,
248-
string output
274+
Endpoint e, string package, string type, boolean subtypes, string name, string signature,
275+
string ext, string output
249276
) {
250-
e.getCallable().hasQualifiedName(package, type, name) and
251-
signature = ExternalFlow::paramsString(e.getCallable()) and
277+
endpointCallable(e, package, type, subtypes, name, signature) and
252278
ext = "" and
253279
output = e.getMaDOutput()
254280
}

‎java/ql/automodel/test/AutomodelApplicationModeExtraction/Test.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,12 @@ public static InputStream getInputStream(Path openPath) throws Exception {
4040
); // $ sourceModelCandidate=newInputStream(Path,OpenOption[]):ReturnValue
4141
}
4242

43-
public static InputStream getInputStream(String openPath) throws Exception {
43+
public static InputStream getInputStream(String openPath, String otherPath) throws Exception {
4444
return Test.getInputStream( // the call is not a source candidate (argument to local call)
4545
Paths.get(
46-
openPath // $ negativeSinkExample=get(String,String[]):Argument[0] // modeled as a flow step
47-
) // $ sourceModelCandidate=get(String,String[]):ReturnValue
46+
openPath, // $ negativeSinkExample=get(String,String[]):Argument[0] // modeled as a flow step
47+
otherPath
48+
) // $ sourceModelCandidate=get(String,String[]):ReturnValue negativeSinkExample=get(String,String[]):Argument[1]
4849
);
4950
}
5051

‎java/ql/automodel/test/AutomodelFrameworkModeExtraction/com/github/codeql/test/MyWriter.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package com.github.codeql.test;
2+
3+
public class MyWriter extends java.io.Writer {
4+
@Override
5+
public void write(char[] cbuf, int off, int len) { // $ sinkModelCandidate=write(char[],int,int):Argument[this] sourceModelCandidate=write(char[],int,int):Parameter[this] sourceModelCandidate=write(char[],int,int):Parameter[0]
6+
}
7+
8+
@Override
9+
public void close() { // $ sinkModelCandidate=close():Argument[this] sourceModelCandidate=close():Parameter[this]
10+
}
11+
12+
@Override
13+
public void flush() { // $ sinkModelCandidate=flush():Argument[this] sourceModelCandidate=flush():Parameter[this]
14+
}
15+
}

‎java/ql/automodel/test/AutomodelFrameworkModeExtraction/java/io/File.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package java.io;
22

33
public class File {
4-
public int compareTo( // $ negativeSinkExample=compareTo(File):Argument[this] negativeSourceExample=compareTo(File):Parameter[this] // modeled as neutral
5-
File pathname // $ negativeSinkExample=compareTo(File):Argument[0] negativeSourceExample=compareTo(File):Parameter[0] // modeled as neutral
4+
public int compareTo( // $ negativeSinkExample=compareTo(File):Argument[this] sourceModelCandidate=compareTo(File):Parameter[this] // modeled as neutral for sinks
5+
File pathname // $ negativeSinkExample=compareTo(File):Argument[0] sourceModelCandidate=compareTo(File):Parameter[0] // modeled as neutral for sinks
66
) {
77
return 0;
88
}

0 commit comments

Comments
 (0)