Skip to content

Commit 4b9443e

Browse files
author
Max Schaefer
committed
Properly recognise existing models involving subtypes.
If an existing source/sink model specifies `subtypes=True` we should apply it to endpoints on overriding methods.
1 parent a9c0fed commit 4b9443e

3 files changed

Lines changed: 78 additions & 32 deletions

File tree

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

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -251,45 +251,68 @@ module ApplicationCandidatesImpl implements SharedCharacteristics::CandidateSig
251251
predicate isKnownKind = AutomodelJavaUtil::isKnownKind/2;
252252

253253
predicate isSink(Endpoint e, string kind, string provenance) {
254-
exists(string package, string type, string name, string signature, string ext, string input |
255-
sinkSpec(e, package, type, name, signature, ext, input) and
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
256259
ExternalFlow::sinkModel(package, type, _, name, [signature, ""], ext, input, kind, provenance)
257260
)
258261
or
259262
isCustomSink(e, kind) and provenance = "custom-sink"
260263
}
261264

262265
predicate isSource(Endpoint e, string kind, string provenance) {
263-
exists(string package, string type, string name, string signature, string ext, string output |
264-
sourceSpec(e, package, type, name, signature, ext, output) and
265-
ExternalFlow::sourceModel(package, type, _, name, [signature, ""], ext, output, kind,
266+
exists(
267+
string package, string type, boolean subtypes, string name, string signature, string ext,
268+
string output
269+
|
270+
sourceSpec(e, package, type, subtypes, name, signature, ext, output) and
271+
ExternalFlow::sourceModel(package, type, subtypes, name, [signature, ""], ext, output, kind,
266272
provenance)
267273
)
268274
}
269275

270276
predicate isNeutral(Endpoint e) {
271277
exists(string package, string type, string name, string signature |
272-
sinkSpec(e, package, type, name, signature, _, _) and
278+
sinkSpec(e, package, type, _, name, signature, _, _) and
273279
ExternalFlow::neutralModel(package, type, name, [signature, ""], "sink", _)
274280
)
275281
}
276282

277-
// XXX how to extend to support sources?
283+
/**
284+
* Holds if the endpoint concerns a callable with the given package, type, name and signature.
285+
*
286+
* If `subtypes` is `false`, only the exact callable is considered. If `true`, the callable and
287+
* all its overrides are considered.
288+
*/
289+
additional predicate endpointCallable(
290+
Endpoint e, string package, string type, boolean subtypes, string name, string signature
291+
) {
292+
exists(Callable c |
293+
c = e.getCallable() and subtypes in [true, false]
294+
or
295+
e.getCallable().(Method).getSourceDeclaration().overrides+(c) and subtypes = true
296+
|
297+
c.hasQualifiedName(package, type, name) and
298+
signature = ExternalFlow::paramsString(c)
299+
)
300+
}
301+
278302
additional predicate sinkSpec(
279-
Endpoint e, string package, string type, string name, string signature, string ext, string input
303+
Endpoint e, string package, string type, boolean subtypes, string name, string signature,
304+
string ext, string input
280305
) {
281-
e.getCallable().hasQualifiedName(package, type, name) and
282-
signature = ExternalFlow::paramsString(e.getCallable()) and
306+
endpointCallable(e, package, type, subtypes, name, signature) and
283307
ext = "" and
284308
input = e.getMaDInput()
285309
}
286310

287311
additional predicate sourceSpec(
288-
Endpoint e, string package, string type, string name, string signature, string ext,
289-
string output
312+
Endpoint e, string package, string type, boolean subtypes, string name, string signature,
313+
string ext, string output
290314
) {
291-
e.getCallable().hasQualifiedName(package, type, name) and
292-
signature = ExternalFlow::paramsString(e.getCallable()) and
315+
endpointCallable(e, package, type, subtypes, name, signature) and
293316
ext = "" and
294317
output = e.getMaDOutput()
295318
}

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

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -209,46 +209,69 @@ 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
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
214217
ExternalFlow::sinkModel(package, type, _, name, [signature, ""], ext, input, kind, provenance)
215218
)
216219
}
217220

218221
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,
222+
exists(
223+
string package, string type, boolean subtypes, string name, string signature, string ext,
224+
string output
225+
|
226+
sourceSpec(e, package, type, subtypes, name, signature, ext, output) and
227+
ExternalFlow::sourceModel(package, type, subtypes, name, [signature, ""], ext, output, kind,
222228
provenance)
223229
)
224230
}
225231

226232
predicate isNeutral(Endpoint e) {
227233
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
234+
sinkSpec(e, package, type, _, name, signature, _, _)
235+
or
236+
sourceSpec(e, package, type, _, name, signature, _, _)
237+
|
233238
ExternalFlow::neutralModel(package, type, name, [signature, ""], "sink", _)
234239
)
235240
}
236241

242+
/**
243+
* Holds if the endpoint concerns a callable with the given package, type, name and signature.
244+
*
245+
* If `subtypes` is `false`, only the exact callable is considered. If `true`, the callable and
246+
* all its overrides are considered.
247+
*/
248+
additional predicate endpointCallable(
249+
Endpoint e, string package, string type, boolean subtypes, string name, string signature
250+
) {
251+
exists(Callable c |
252+
c = e.getCallable() and subtypes in [true, false]
253+
or
254+
e.getCallable().(Method).getSourceDeclaration().overrides+(c) and subtypes = true
255+
|
256+
c.hasQualifiedName(package, type, name) and
257+
signature = ExternalFlow::paramsString(c)
258+
)
259+
}
260+
237261
additional predicate sinkSpec(
238-
Endpoint e, string package, string type, string name, string signature, string ext, string input
262+
Endpoint e, string package, string type, boolean subtypes, string name, string signature,
263+
string ext, string input
239264
) {
240-
e.getCallable().hasQualifiedName(package, type, name) and
241-
signature = ExternalFlow::paramsString(e.getCallable()) and
265+
endpointCallable(e, package, type, subtypes, name, signature) and
242266
ext = "" and
243267
input = e.getMaDInput()
244268
}
245269

246270
additional predicate sourceSpec(
247-
Endpoint e, string package, string type, string name, string signature, string ext,
248-
string output
271+
Endpoint e, string package, string type, boolean subtypes, string name, string signature,
272+
string ext, string output
249273
) {
250-
e.getCallable().hasQualifiedName(package, type, name) and
251-
signature = ExternalFlow::paramsString(e.getCallable()) and
274+
endpointCallable(e, package, type, subtypes, name, signature) and
252275
ext = "" and
253276
output = e.getMaDOutput()
254277
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
public class MyWriter extends java.io.Writer {
44
@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] SPURIOUS: sinkModelCandidate=write(char[],int,int):Argument[0]
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]
66
}
77

88
@Override

0 commit comments

Comments
 (0)