From 0f864081cb74a959328e31fde0f58e4788529fbe Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 21 May 2024 14:51:31 +0200 Subject: [PATCH 1/2] Java: Remove source dispatch when there's an exact match from a manual model. --- .../code/java/dataflow/ExternalFlow.qll | 45 ++++++++++++------- .../semmle/code/java/dataflow/FlowSummary.qll | 2 + .../dataflow/internal/DataFlowDispatch.qll | 14 ++++++ .../dataflow/internal/FlowSummaryImpl.qll | 16 ++++--- java/ql/src/utils/modeleditor/ModelEditor.qll | 2 +- .../dataflow/internal/FlowSummaryImpl.qll | 14 ++++++ 6 files changed, 68 insertions(+), 25 deletions(-) diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll index 68b43a6a14a..08632b661b9 100644 --- a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll @@ -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 | + exists(Member m, boolean isExact0 | ( - result = m + result = m and isExact0 = true or - subtypes = true and result.(SrcMethod).overridesOrInstantiates+(m) + subtypes = true and result.(SrcMethod).overridesOrInstantiates+(m) and isExact0 = false ) and m.hasQualifiedName(package, type, name) | - signature = "" or - paramsStringQualified(m) = signature or - paramsString(m) = signature + signature = "" and isExact = false + or + paramsStringQualified(m) = signature and isExact = isExact0 + or + paramsString(m) = signature and isExact = isExact0 ) 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 = "" @@ -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 ) } @@ -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() ) } @@ -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() ) } @@ -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 } } diff --git a/java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll b/java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll index 51055e56212..acea2a10784 100644 --- a/java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll +++ b/java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll @@ -135,6 +135,8 @@ private class SummarizedSyntheticCallableAdapter extends SummarizedCallable, TSy model = sc ) } + + override predicate hasExactModel() { any() } } deprecated class RequiredSummaryComponentStack = Impl::Private::RequiredSummaryComponentStack; diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll index a7877fdf2f9..4a7e0814013 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll @@ -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) diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll index 5698d3f3477..0a994991e26 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll @@ -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, _) ) } @@ -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 @@ -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 @@ -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, @@ -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 @@ -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) ) } } diff --git a/java/ql/src/utils/modeleditor/ModelEditor.qll b/java/ql/src/utils/modeleditor/ModelEditor.qll index 2c1a56823f1..dd4d405b83e 100644 --- a/java/ql/src/utils/modeleditor/ModelEditor.qll +++ b/java/ql/src/utils/modeleditor/ModelEditor.qll @@ -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, "", _) ) } diff --git a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll index 8cc0d37f29b..eb181494d5f 100644 --- a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll @@ -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 or overloading was used to identify this + * callable from the model. + */ + predicate hasExactModel() { none() } } final private class NeutralCallableFinal = NeutralCallable; @@ -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 or overloading was used to identify this + * callable from the model. + */ + predicate hasExactModel() { none() } } } From f353065d265266833cf1dcea5458a5c97c94b7e3 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Wed, 22 May 2024 14:26:50 +0200 Subject: [PATCH 2/2] Java: Allow overloading for exact model matches. --- .../lib/semmle/code/java/dataflow/ExternalFlow.qll | 12 ++++++------ .../codeql/dataflow/internal/FlowSummaryImpl.qll | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll index 08632b661b9..2337d0282aa 100644 --- a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll @@ -417,19 +417,19 @@ private Element interpretElement0( ) { elementSpec(package, type, subtypes, name, signature, _) and ( - exists(Member m, boolean isExact0 | + exists(Member m | ( - result = m and isExact0 = true + result = m and isExact = true or - subtypes = true and result.(SrcMethod).overridesOrInstantiates+(m) and isExact0 = false + subtypes = true and result.(SrcMethod).overridesOrInstantiates+(m) and isExact = false ) and m.hasQualifiedName(package, type, name) | - signature = "" and isExact = false + signature = "" or - paramsStringQualified(m) = signature and isExact = isExact0 + paramsStringQualified(m) = signature or - paramsString(m) = signature and isExact = isExact0 + paramsString(m) = signature ) or exists(RefType t | diff --git a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll index eb181494d5f..9e0ccf82be2 100644 --- a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll @@ -256,8 +256,8 @@ module Make< /** * Holds if there exists a model for which this callable is an exact - * match, that is, no overriding or overloading was used to identify this - * callable from the model. + * match, that is, no overriding was used to identify this callable from + * the model. */ predicate hasExactModel() { none() } } @@ -302,8 +302,8 @@ module Make< /** * Holds if there exists a model for which this callable is an exact - * match, that is, no overriding or overloading was used to identify this - * callable from the model. + * match, that is, no overriding was used to identify this callable from + * the model. */ predicate hasExactModel() { none() } }