Skip to content

Java: Remove source dispatch when there's an exact match from a manual model. #16552

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 26 additions & 15 deletions java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll
Original file line number Diff line number Diff line change
Expand Up @@ -413,25 +413,28 @@ private string paramsStringQualified(Callable c) {
}

private Element interpretElement0(
string package, string type, boolean subtypes, string name, string signature
string package, string type, boolean subtypes, string name, string signature, boolean isExact
) {
elementSpec(package, type, subtypes, name, signature, _) and
(
exists(Member m |
(
result = m
result = m and isExact = true
or
subtypes = true and result.(SrcMethod).overridesOrInstantiates+(m)
subtypes = true and result.(SrcMethod).overridesOrInstantiates+(m) and isExact = false
) and
m.hasQualifiedName(package, type, name)
|
signature = "" or
paramsStringQualified(m) = signature or
signature = ""
or
paramsStringQualified(m) = signature
or
paramsString(m) = signature
)
or
exists(RefType t |
t.hasQualifiedName(package, type) and
isExact = false and
(if subtypes = true then result.(SrcRefType).getASourceSupertype*() = t else result = t) and
name = "" and
signature = ""
Expand All @@ -442,13 +445,16 @@ private Element interpretElement0(
/** Gets the source/sink/summary/neutral element corresponding to the supplied parameters. */
cached
Element interpretElement(
string package, string type, boolean subtypes, string name, string signature, string ext
string package, string type, boolean subtypes, string name, string signature, string ext,
boolean isExact
) {
elementSpec(package, type, subtypes, name, signature, ext) and
exists(Element e | e = interpretElement0(package, type, subtypes, name, signature) |
ext = "" and result = e
exists(Element e, boolean isExact0 |
e = interpretElement0(package, type, subtypes, name, signature, isExact0)
|
ext = "" and result = e and isExact = isExact0
or
ext = "Annotated" and result.(Annotatable).getAnAnnotation().getType() = e
ext = "Annotated" and result.(Annotatable).getAnAnnotation().getType() = e and isExact = false
)
}

Expand Down Expand Up @@ -538,13 +544,13 @@ predicate sinkNode(Node node, string kind) { sinkNode(node, kind, _) }

// adapter class for converting Mad summaries to `SummarizedCallable`s
private class SummarizedCallableAdapter extends SummarizedCallable {
SummarizedCallableAdapter() { summaryElement(this, _, _, _, _, _) }
SummarizedCallableAdapter() { summaryElement(this, _, _, _, _, _, _) }

private predicate relevantSummaryElementManual(
string input, string output, string kind, string model
) {
exists(Provenance provenance |
summaryElement(this, input, output, kind, provenance, model) and
summaryElement(this, input, output, kind, provenance, model, _) and
provenance.isManual()
)
}
Expand All @@ -553,11 +559,11 @@ private class SummarizedCallableAdapter extends SummarizedCallable {
string input, string output, string kind, string model
) {
exists(Provenance provenance |
summaryElement(this, input, output, kind, provenance, model) and
summaryElement(this, input, output, kind, provenance, model, _) and
provenance.isGenerated()
) and
not exists(Provenance provenance |
neutralElement(this, "summary", provenance) and
neutralElement(this, "summary", provenance, _) and
provenance.isManual()
)
}
Expand All @@ -576,18 +582,23 @@ private class SummarizedCallableAdapter extends SummarizedCallable {
}

override predicate hasProvenance(Provenance provenance) {
summaryElement(this, _, _, _, provenance, _)
summaryElement(this, _, _, _, provenance, _, _)
}

override predicate hasExactModel() { summaryElement(this, _, _, _, _, _, true) }
}

// adapter class for converting Mad neutrals to `NeutralCallable`s
private class NeutralCallableAdapter extends NeutralCallable {
string kind;
string provenance_;
boolean exact;

NeutralCallableAdapter() { neutralElement(this, kind, provenance_) }
NeutralCallableAdapter() { neutralElement(this, kind, provenance_, exact) }

override string getKind() { result = kind }

override predicate hasProvenance(Provenance provenance) { provenance = provenance_ }

override predicate hasExactModel() { exact = true }
}
2 changes: 2 additions & 0 deletions java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ private class SummarizedSyntheticCallableAdapter extends SummarizedCallable, TSy
model = sc
)
}

override predicate hasExactModel() { any() }
}

deprecated class RequiredSummaryComponentStack = Impl::Private::RequiredSummaryComponentStack;
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,21 @@ private module DispatchImpl {
)
}

private predicate hasExactManualModel(Call c, Callable tgt) {
tgt = c.getCallee().getSourceDeclaration() and
(
exists(Impl::Public::SummarizedCallable sc |
sc.getACall() = c and sc.hasExactModel() and sc.hasManualModel()
)
or
exists(Impl::Public::NeutralSummaryCallable nc |
nc.getACall() = c and nc.hasExactModel() and nc.hasManualModel()
)
)
}

private Callable sourceDispatch(Call c) {
not hasExactManualModel(c, result) and
result = VirtualDispatch::viableCallable(c) and
if VirtualDispatch::lowConfidenceDispatchTarget(c, result)
then not hasHighConfidenceTarget(c)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private predicate relatedArgSpec(Callable c, string spec) {
sourceModel(namespace, type, subtypes, name, signature, ext, spec, _, _, _) or
sinkModel(namespace, type, subtypes, name, signature, ext, spec, _, _, _)
|
c = interpretElement(namespace, type, subtypes, name, signature, ext)
c = interpretElement(namespace, type, subtypes, name, signature, ext, _)
)
}

Expand Down Expand Up @@ -202,7 +202,7 @@ module SourceSinkInterpretationInput implements
sourceModel(namespace, type, subtypes, name, signature, ext, originalOutput, kind, provenance,
madId) and
model = "MaD:" + madId.toString() and
baseSource = interpretElement(namespace, type, subtypes, name, signature, ext) and
baseSource = interpretElement(namespace, type, subtypes, name, signature, ext, _) and
(
e = baseSource and output = originalOutput
or
Expand All @@ -221,7 +221,7 @@ module SourceSinkInterpretationInput implements
sinkModel(namespace, type, subtypes, name, signature, ext, originalInput, kind, provenance,
madId) and
model = "MaD:" + madId.toString() and
baseSink = interpretElement(namespace, type, subtypes, name, signature, ext) and
baseSink = interpretElement(namespace, type, subtypes, name, signature, ext, _) and
(
e = baseSink and originalInput = input
or
Expand Down Expand Up @@ -310,7 +310,7 @@ module Private {
*/
predicate summaryElement(
Input::SummarizedCallableBase c, string input, string output, string kind, string provenance,
string model
string model, boolean isExact
) {
exists(
string namespace, string type, boolean subtypes, string name, string signature, string ext,
Expand All @@ -320,7 +320,7 @@ module Private {
summaryModel(namespace, type, subtypes, name, signature, ext, originalInput, originalOutput,
kind, provenance, madId) and
model = "MaD:" + madId.toString() and
baseCallable = interpretElement(namespace, type, subtypes, name, signature, ext) and
baseCallable = interpretElement(namespace, type, subtypes, name, signature, ext, isExact) and
(
c.asCallable() = baseCallable and input = originalInput and output = originalOutput
or
Expand All @@ -336,10 +336,12 @@ module Private {
* Holds if a neutral model exists for `c` of kind `kind`
* and with provenance `provenance`.
*/
predicate neutralElement(Input::SummarizedCallableBase c, string kind, string provenance) {
predicate neutralElement(
Input::SummarizedCallableBase c, string kind, string provenance, boolean isExact
) {
exists(string namespace, string type, string name, string signature |
neutralModel(namespace, type, name, signature, kind, provenance) and
c.asCallable() = interpretElement(namespace, type, false, name, signature, "")
c.asCallable() = interpretElement(namespace, type, false, name, signature, "", isExact)
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion java/ql/src/utils/modeleditor/ModelEditor.qll
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class Endpoint extends Callable {
predicate isNeutral() {
exists(string namespace, string type, string name, string signature |
neutralModel(namespace, type, name, signature, _, _) and
this = interpretElement(namespace, type, false, name, signature, "")
this = interpretElement(namespace, type, false, name, signature, "", _)
)
}

Expand Down
14 changes: 14 additions & 0 deletions shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,13 @@ module Make<
* that has provenance `provenance`.
*/
predicate hasProvenance(Provenance provenance) { provenance = "manual" }

/**
* Holds if there exists a model for which this callable is an exact
* match, that is, no overriding was used to identify this callable from
* the model.
*/
predicate hasExactModel() { none() }
}

final private class NeutralCallableFinal = NeutralCallable;
Expand Down Expand Up @@ -292,6 +299,13 @@ module Make<
* Gets the kind of the neutral.
*/
abstract string getKind();

/**
* Holds if there exists a model for which this callable is an exact
* match, that is, no overriding was used to identify this callable from
* the model.
*/
predicate hasExactModel() { none() }
}
}

Expand Down