diff --git a/csharp/ql/src/utils/modelgenerator/CaptureDiscardedSummaryModels.ql b/csharp/ql/src/utils/modelgenerator/CaptureDiscardedSummaryModels.ql deleted file mode 100644 index bbd6e887643f..000000000000 --- a/csharp/ql/src/utils/modelgenerator/CaptureDiscardedSummaryModels.ql +++ /dev/null @@ -1,15 +0,0 @@ -/** - * @name Capture discarded summary models. - * @description Finds summary models that are discarded as handwritten counterparts exist. - * @id cs/utils/modelgenerator/discarded-summary-models - */ - -import semmle.code.csharp.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl -import internal.CaptureModels -import internal.CaptureSummaryFlowQuery - -from DataFlowTargetApi api, string flow -where - flow = captureFlow(api) and - api.(FlowSummaryImpl::Public::SummarizedCallable).applyManualModel() -select flow order by flow diff --git a/csharp/ql/src/utils/modelgenerator/CaptureNeutralModels.ql b/csharp/ql/src/utils/modelgenerator/CaptureNeutralModels.ql index 5b41c5f26c7f..574d93c8a614 100644 --- a/csharp/ql/src/utils/modelgenerator/CaptureNeutralModels.ql +++ b/csharp/ql/src/utils/modelgenerator/CaptureNeutralModels.ql @@ -11,7 +11,5 @@ import internal.CaptureModels import internal.CaptureSummaryFlowQuery from DataFlowTargetApi api, string noflow -where - noflow = captureNoFlow(api) and - not api.(FlowSummaryImpl::Public::SummarizedCallable).applyManualModel() +where noflow = captureNoFlow(api) select noflow order by noflow diff --git a/csharp/ql/src/utils/modelgenerator/CaptureSummaryModels.ql b/csharp/ql/src/utils/modelgenerator/CaptureSummaryModels.ql index b8248f5a2b50..ac6e3b3227d5 100644 --- a/csharp/ql/src/utils/modelgenerator/CaptureSummaryModels.ql +++ b/csharp/ql/src/utils/modelgenerator/CaptureSummaryModels.ql @@ -11,7 +11,5 @@ import internal.CaptureModels import internal.CaptureSummaryFlowQuery from DataFlowTargetApi api, string flow -where - flow = captureFlow(api) and - not api.(FlowSummaryImpl::Public::SummarizedCallable).applyManualModel() +where flow = captureFlow(api) select flow order by flow diff --git a/csharp/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll b/csharp/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll index 60748c57b087..618cd1141711 100644 --- a/csharp/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll +++ b/csharp/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll @@ -7,6 +7,7 @@ private import dotnet private import semmle.code.csharp.commons.Util as Util private import semmle.code.csharp.commons.Collections as Collections private import semmle.code.csharp.dataflow.internal.DataFlowDispatch +private import semmle.code.csharp.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl private import semmle.code.csharp.frameworks.system.linq.Expressions import semmle.code.csharp.dataflow.internal.ExternalFlow as ExternalFlow import semmle.code.csharp.dataflow.internal.DataFlowImplCommon as DataFlowImplCommon @@ -37,7 +38,10 @@ private predicate isRelevantForModels(CS::Callable api) { not api instanceof Util::MainMethod and not api instanceof CS::Destructor and not api instanceof CS::AnonymousFunctionExpr and - not api.(CS::Constructor).isParameterless() + not api.(CS::Constructor).isParameterless() and + // Disregard all APIs that have a manual model. + not api = any(FlowSummaryImpl::Public::SummarizedCallable sc | sc.applyManualModel()) and + not api = any(FlowSummaryImpl::Public::NeutralSummaryCallable sc | sc.hasManualModel()) } /** diff --git a/csharp/ql/test/utils/modelgenerator/dataflow/CaptureNeutralModels.ext.yml b/csharp/ql/test/utils/modelgenerator/dataflow/CaptureNeutralModels.ext.yml new file mode 100644 index 000000000000..319d0a5ac621 --- /dev/null +++ b/csharp/ql/test/utils/modelgenerator/dataflow/CaptureNeutralModels.ext.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/csharp-all + extensible: neutralModel + data: + - [ "NoSummaries", "ManuallyModelled", "HasNeutralSummaryNoFlow", "(System.Object)", "summary", "manual"] diff --git a/csharp/ql/test/utils/modelgenerator/dataflow/CaptureSummaryModels.ext.yml b/csharp/ql/test/utils/modelgenerator/dataflow/CaptureSummaryModels.ext.yml new file mode 100644 index 000000000000..05e3b67313ce --- /dev/null +++ b/csharp/ql/test/utils/modelgenerator/dataflow/CaptureSummaryModels.ext.yml @@ -0,0 +1,12 @@ +extensions: + - addsTo: + pack: codeql/csharp-all + extensible: summaryModel + data: + - [ "NoSummaries", "ManuallyModelled", False, "HasSummary", "(System.Object)", "", "Argument[0]", "ReturnValue", "value", "manual"] + + - addsTo: + pack: codeql/csharp-all + extensible: neutralModel + data: + - [ "NoSummaries", "ManuallyModelled", "HasNeutralSummary", "(System.Object)", "summary", "manual"] diff --git a/csharp/ql/test/utils/modelgenerator/dataflow/NoSummaries.cs b/csharp/ql/test/utils/modelgenerator/dataflow/NoSummaries.cs index b2c21b579177..b9f08329480f 100644 --- a/csharp/ql/test/utils/modelgenerator/dataflow/NoSummaries.cs +++ b/csharp/ql/test/utils/modelgenerator/dataflow/NoSummaries.cs @@ -143,3 +143,22 @@ public ParameterlessConstructor() IsInitialized = true; } } + +// No models should be created, if there exist either a manual summary or neutral summary. +public class ManuallyModelled +{ + public object HasSummary(object o) + { + return o; + } + + public object HasNeutralSummary(object o) + { + return o; + } + + public object HasNeutralSummaryNoFlow(object o) + { + return null; + } +} diff --git a/csharp/ql/test/utils/modelgenerator/typebasedflow/CaptureTypeBasedSummaryModels.expected b/csharp/ql/test/utils/modelgenerator/typebasedflow/CaptureTypeBasedSummaryModels.expected index 45b4980ea43c..28fe5d4d93f3 100644 --- a/csharp/ql/test/utils/modelgenerator/typebasedflow/CaptureTypeBasedSummaryModels.expected +++ b/csharp/ql/test/utils/modelgenerator/typebasedflow/CaptureTypeBasedSummaryModels.expected @@ -124,7 +124,6 @@ | Summaries;TypeBasedCollection;false;Add;(T);;Argument[0];Argument[this].Element;value;tb-generated | | Summaries;TypeBasedCollection;false;AddMany;(System.Collections.Generic.IEnumerable);;Argument[0].Element;Argument[this].Element;value;tb-generated | | Summaries;TypeBasedCollection;false;First;();;Argument[this].Element;ReturnValue;value;tb-generated | -| Summaries;TypeBasedCollection;false;GetEnumerator;();;Argument[this].Element;ReturnValue.SyntheticField[ArgType0];value;tb-generated | | Summaries;TypeBasedCollection;false;GetMany;();;Argument[this].Element;ReturnValue.Element;value;tb-generated | | Summaries;TypeBasedComplex;false;AddMany;(System.Collections.Generic.IEnumerable);;Argument[0].Element;Argument[this].SyntheticField[ArgType0];value;tb-generated | | Summaries;TypeBasedComplex;false;Apply;(System.Func);;Argument[this].SyntheticField[ArgType0];Argument[0].Parameter[0];value;tb-generated | diff --git a/java/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll b/java/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll index 7877594519a9..2b69c7d0aff5 100644 --- a/java/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll +++ b/java/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll @@ -5,6 +5,7 @@ private import java as J private import semmle.code.java.dataflow.internal.DataFlowPrivate private import semmle.code.java.dataflow.internal.ContainerFlow as ContainerFlow +private import semmle.code.java.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl private import semmle.code.java.dataflow.internal.ModelExclusions private import semmle.code.java.dataflow.DataFlow as Df private import semmle.code.java.dataflow.SSA as Ssa @@ -21,6 +22,8 @@ class Type = J::Type; class Unit = J::Unit; +class Callable = J::Callable; + private J::Method superImpl(J::Method m) { result = m.getAnOverride() and not exists(result.getAnOverride()) and @@ -35,15 +38,19 @@ private predicate isInfrequentlyUsed(J::CompilationUnit cu) { /** * Holds if it is relevant to generate models for `api`. */ -private predicate isRelevantForModels(J::Callable api) { +private predicate isRelevantForModels(Callable api) { not isUninterestingForModels(api) and - not isInfrequentlyUsed(api.getCompilationUnit()) + not isInfrequentlyUsed(api.getCompilationUnit()) and + // Disregard all APIs that have a manual model. + not api = any(FlowSummaryImpl::Public::SummarizedCallable sc | sc.applyManualModel()).asCallable() and + not api = + any(FlowSummaryImpl::Public::NeutralSummaryCallable sc | sc.hasManualModel()).asCallable() } /** * Holds if it is relevant to generate models for `api` based on data flow analysis. */ -predicate isRelevantForDataFlowModels(J::Callable api) { +predicate isRelevantForDataFlowModels(Callable api) { isRelevantForModels(api) and (not api.getDeclaringType() instanceof J::Interface or exists(api.getBody())) } @@ -56,7 +63,7 @@ predicate isRelevantForTypeBasedFlowModels = isRelevantForModels/1; * In the Standard library and 3rd party libraries it the Callables that can be called * from outside the library itself. */ -class TargetApiSpecific extends J::Callable { +class TargetApiSpecific extends Callable { TargetApiSpecific() { this.isPublic() and this.fromSource() and @@ -66,6 +73,15 @@ class TargetApiSpecific extends J::Callable { ) and isRelevantForModels(this) } + + /** + * Gets the callable that a model will be lifted to, if any. + */ + Callable lift() { + exists(Method m | m = superImpl(this) and m.fromSource() | result = m) + or + not exists(superImpl(this)) and result = this + } } private string isExtensible(J::RefType ref) { @@ -79,9 +95,7 @@ private string typeAsModel(J::RefType type) { } private J::RefType bestTypeForModel(TargetApiSpecific api) { - if exists(superImpl(api)) - then superImpl(api).fromSource() and result = superImpl(api).getDeclaringType() - else result = api.getDeclaringType() + result = api.lift().getDeclaringType() } /** @@ -195,7 +209,7 @@ string returnNodeAsOutput(DataFlowImplCommon::ReturnNodeExt node) { /** * Gets the enclosing callable of `ret`. */ -J::Callable returnNodeEnclosingCallable(DataFlowImplCommon::ReturnNodeExt ret) { +Callable returnNodeEnclosingCallable(DataFlowImplCommon::ReturnNodeExt ret) { result = DataFlowImplCommon::getNodeEnclosingCallable(ret).asCallable() } diff --git a/java/ql/src/utils/modelgenerator/internal/CaptureSummaryFlowQuery.qll b/java/ql/src/utils/modelgenerator/internal/CaptureSummaryFlowQuery.qll index 33b18acf0c52..40190711e402 100644 --- a/java/ql/src/utils/modelgenerator/internal/CaptureSummaryFlowQuery.qll +++ b/java/ql/src/utils/modelgenerator/internal/CaptureSummaryFlowQuery.qll @@ -73,10 +73,11 @@ string captureFlow(DataFlowTargetApi api) { } /** - * Gets the neutral summary for `api`, if any. - * A neutral model is generated, if there does not exist any summary model. + * Gets the neutral summary model for `api`, if any. + * A neutral summary model is generated, if we are not generating + * a summary model that applies to `api`. */ string captureNoFlow(DataFlowTargetApi api) { - not exists(captureFlow(api)) and + not exists(DataFlowTargetApi api0 | exists(captureFlow(api0)) and api0.lift() = api.lift()) and result = ModelPrinting::asNeutralSummaryModel(api) } diff --git a/java/ql/test/utils/modelgenerator/dataflow/CaptureSummaryModels.expected b/java/ql/test/utils/modelgenerator/dataflow/CaptureSummaryModels.expected index 24eb6d63e0d9..014d47f25dd3 100644 --- a/java/ql/test/utils/modelgenerator/dataflow/CaptureSummaryModels.expected +++ b/java/ql/test/utils/modelgenerator/dataflow/CaptureSummaryModels.expected @@ -22,6 +22,7 @@ | p;Joiner;false;setEmptyValue;(CharSequence);;Argument[0];Argument[this];taint;df-generated | | p;Joiner;false;setEmptyValue;(CharSequence);;Argument[this];ReturnValue;value;df-generated | | p;Joiner;false;toString;();;Argument[this];ReturnValue;taint;df-generated | +| p;MultipleImpl2$IInterface;true;m;(Object);;Argument[0];ReturnValue;taint;df-generated | | p;MultipleImpls$Strat2;true;getValue;();;Argument[this];ReturnValue;taint;df-generated | | p;MultipleImpls$Strategy;true;doSomething;(String);;Argument[0];Argument[this];taint;df-generated | | p;MultipleImpls$Strategy;true;doSomething;(String);;Argument[0];ReturnValue;taint;df-generated | diff --git a/java/ql/test/utils/modelgenerator/dataflow/p/MultipleImpl2.java b/java/ql/test/utils/modelgenerator/dataflow/p/MultipleImpl2.java new file mode 100644 index 000000000000..9de2d59b2e4f --- /dev/null +++ b/java/ql/test/utils/modelgenerator/dataflow/p/MultipleImpl2.java @@ -0,0 +1,23 @@ +package p; + +class MultipleImpl2 { + + // Multiple implementations of the same interface. + // This is used to test that we only generate a summary model and + // not neutral summary model for `IInterface.m`. + public interface IInterface { + Object m(Object value); + } + + public class Impl1 implements IInterface { + public Object m(Object value) { + return null; + } + } + + public class Impl2 implements IInterface { + public Object m(Object value) { + return value; + } + } +}