diff --git a/csharp/ql/lib/change-notes/2024-01-11-manual-neutral-model-blocks-generated-models.md b/csharp/ql/lib/change-notes/2024-01-11-manual-neutral-model-blocks-generated-models.md new file mode 100644 index 00000000000..bdc5c1b0f2d --- /dev/null +++ b/csharp/ql/lib/change-notes/2024-01-11-manual-neutral-model-blocks-generated-models.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* A manual neutral summary model for a callable now blocks all generated summary models for that callable from having any effect. diff --git a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll index fcd8a91b5e5..5c985759b78 100644 --- a/csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll +++ b/csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll @@ -529,6 +529,13 @@ private predicate interpretSummary( ) } +private predicate interpretNeutral(UnboundCallable c, string kind, string provenance) { + exists(string namespace, string type, string name, string signature | + neutralModel(namespace, type, name, signature, kind, provenance) and + c = interpretElement(namespace, type, false, name, signature, "") + ) +} + // adapter class for converting Mad summaries to `SummarizedCallable`s private class SummarizedCallableAdapter extends SummarizedCallable { SummarizedCallableAdapter() { interpretSummary(this, _, _, _, _) } @@ -544,6 +551,10 @@ private class SummarizedCallableAdapter extends SummarizedCallable { exists(Provenance provenance | interpretSummary(this, input, output, kind, provenance) and provenance.isGenerated() + ) and + not exists(Provenance provenance | + interpretNeutral(this, "summary", provenance) and + provenance.isManual() ) } @@ -568,12 +579,7 @@ private class NeutralCallableAdapter extends NeutralCallable { string kind; string provenance_; - NeutralCallableAdapter() { - exists(string namespace, string type, string name, string signature | - neutralModel(namespace, type, name, signature, kind, provenance_) and - this = interpretElement(namespace, type, false, name, signature, "") - ) - } + NeutralCallableAdapter() { interpretNeutral(this, kind, provenance_) } override string getKind() { result = kind } diff --git a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs index bc49805fc04..1013a31fa50 100644 --- a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs +++ b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs @@ -206,12 +206,25 @@ namespace My.Qltest Sink(MixedFlowArgs(null, o2)); } + void M4() + { + var o1 = new object(); + Sink(GeneratedFlowWithGeneratedNeutral(o1)); + + var o2 = new object(); + Sink(GeneratedFlowWithManualNeutral(o2)); // no flow because the modelled method has a manual neutral summary model + } + object GeneratedFlow(object o) => throw null; object GeneratedFlowArgs(object o1, object o2) => throw null; object MixedFlowArgs(object o1, object o2) => throw null; + object GeneratedFlowWithGeneratedNeutral(object o) => throw null; + + object GeneratedFlowWithManualNeutral(object o) => throw null; + static void Sink(object o) { } } diff --git a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected index 34849ad7014..ad549681753 100644 --- a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected +++ b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected @@ -63,9 +63,11 @@ edges | ExternalFlow.cs:120:18:120:18 | access to local variable b : null [element] : Object | ExternalFlow.cs:120:18:120:21 | access to array element | | ExternalFlow.cs:205:22:205:33 | object creation of type Object : Object | ExternalFlow.cs:206:38:206:39 | access to local variable o2 : Object | | ExternalFlow.cs:206:38:206:39 | access to local variable o2 : Object | ExternalFlow.cs:206:18:206:40 | call to method MixedFlowArgs | -| ExternalFlow.cs:231:21:231:28 | object creation of type HC : HC | ExternalFlow.cs:232:21:232:21 | access to local variable h : HC | -| ExternalFlow.cs:232:21:232:21 | access to local variable h : HC | ExternalFlow.cs:232:21:232:39 | call to method ExtensionMethod : HC | -| ExternalFlow.cs:232:21:232:39 | call to method ExtensionMethod : HC | ExternalFlow.cs:233:18:233:18 | access to local variable o | +| ExternalFlow.cs:211:22:211:33 | object creation of type Object : Object | ExternalFlow.cs:212:52:212:53 | access to local variable o1 : Object | +| ExternalFlow.cs:212:52:212:53 | access to local variable o1 : Object | ExternalFlow.cs:212:18:212:54 | call to method GeneratedFlowWithGeneratedNeutral | +| ExternalFlow.cs:244:21:244:28 | object creation of type HC : HC | ExternalFlow.cs:245:21:245:21 | access to local variable h : HC | +| ExternalFlow.cs:245:21:245:21 | access to local variable h : HC | ExternalFlow.cs:245:21:245:39 | call to method ExtensionMethod : HC | +| ExternalFlow.cs:245:21:245:39 | call to method ExtensionMethod : HC | ExternalFlow.cs:246:18:246:18 | access to local variable o | nodes | ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | semmle.label | object creation of type Object : Object | | ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | semmle.label | call to method StepArgRes | @@ -148,10 +150,13 @@ nodes | ExternalFlow.cs:205:22:205:33 | object creation of type Object : Object | semmle.label | object creation of type Object : Object | | ExternalFlow.cs:206:18:206:40 | call to method MixedFlowArgs | semmle.label | call to method MixedFlowArgs | | ExternalFlow.cs:206:38:206:39 | access to local variable o2 : Object | semmle.label | access to local variable o2 : Object | -| ExternalFlow.cs:231:21:231:28 | object creation of type HC : HC | semmle.label | object creation of type HC : HC | -| ExternalFlow.cs:232:21:232:21 | access to local variable h : HC | semmle.label | access to local variable h : HC | -| ExternalFlow.cs:232:21:232:39 | call to method ExtensionMethod : HC | semmle.label | call to method ExtensionMethod : HC | -| ExternalFlow.cs:233:18:233:18 | access to local variable o | semmle.label | access to local variable o | +| ExternalFlow.cs:211:22:211:33 | object creation of type Object : Object | semmle.label | object creation of type Object : Object | +| ExternalFlow.cs:212:18:212:54 | call to method GeneratedFlowWithGeneratedNeutral | semmle.label | call to method GeneratedFlowWithGeneratedNeutral | +| ExternalFlow.cs:212:52:212:53 | access to local variable o1 : Object | semmle.label | access to local variable o1 : Object | +| ExternalFlow.cs:244:21:244:28 | object creation of type HC : HC | semmle.label | object creation of type HC : HC | +| ExternalFlow.cs:245:21:245:21 | access to local variable h : HC | semmle.label | access to local variable h : HC | +| ExternalFlow.cs:245:21:245:39 | call to method ExtensionMethod : HC | semmle.label | call to method ExtensionMethod : HC | +| ExternalFlow.cs:246:18:246:18 | access to local variable o | semmle.label | access to local variable o | subpaths #select | ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | $@ | ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | object creation of type Object : Object | @@ -175,4 +180,5 @@ subpaths | ExternalFlow.cs:112:18:112:25 | access to property MyProp | ExternalFlow.cs:111:24:111:35 | object creation of type Object : Object | ExternalFlow.cs:112:18:112:25 | access to property MyProp | $@ | ExternalFlow.cs:111:24:111:35 | object creation of type Object : Object | object creation of type Object : Object | | ExternalFlow.cs:120:18:120:21 | access to array element | ExternalFlow.cs:117:36:117:47 | object creation of type Object : Object | ExternalFlow.cs:120:18:120:21 | access to array element | $@ | ExternalFlow.cs:117:36:117:47 | object creation of type Object : Object | object creation of type Object : Object | | ExternalFlow.cs:206:18:206:40 | call to method MixedFlowArgs | ExternalFlow.cs:205:22:205:33 | object creation of type Object : Object | ExternalFlow.cs:206:18:206:40 | call to method MixedFlowArgs | $@ | ExternalFlow.cs:205:22:205:33 | object creation of type Object : Object | object creation of type Object : Object | -| ExternalFlow.cs:233:18:233:18 | access to local variable o | ExternalFlow.cs:231:21:231:28 | object creation of type HC : HC | ExternalFlow.cs:233:18:233:18 | access to local variable o | $@ | ExternalFlow.cs:231:21:231:28 | object creation of type HC : HC | object creation of type HC : HC | +| ExternalFlow.cs:212:18:212:54 | call to method GeneratedFlowWithGeneratedNeutral | ExternalFlow.cs:211:22:211:33 | object creation of type Object : Object | ExternalFlow.cs:212:18:212:54 | call to method GeneratedFlowWithGeneratedNeutral | $@ | ExternalFlow.cs:211:22:211:33 | object creation of type Object : Object | object creation of type Object : Object | +| ExternalFlow.cs:246:18:246:18 | access to local variable o | ExternalFlow.cs:244:21:244:28 | object creation of type HC : HC | ExternalFlow.cs:246:18:246:18 | access to local variable o | $@ | ExternalFlow.cs:244:21:244:28 | object creation of type HC : HC | object creation of type HC : HC | diff --git a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ext.yml b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ext.yml index ff578df868e..fe22fefa319 100644 --- a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ext.yml +++ b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ext.yml @@ -29,4 +29,13 @@ extensions: - ["My.Qltest", "G", false, "GeneratedFlowArgs", "(System.Object,System.Object)", "", "Argument[1]", "ReturnValue", "value", "df-generated"] - ["My.Qltest", "G", false, "MixedFlowArgs", "(System.Object,System.Object)", "", "Argument[0]", "ReturnValue", "value", "df-generated"] - ["My.Qltest", "G", false, "MixedFlowArgs", "(System.Object,System.Object)", "", "Argument[1]", "ReturnValue", "value", "manual"] + - ["My.Qltest", "G", false, "GeneratedFlowWithGeneratedNeutral", "(System.Object)", "", "Argument[0]", "ReturnValue", "value", "df-generated"] + - ["My.Qltest", "G", false, "GeneratedFlowWithManualNeutral", "(System.Object)", "", "Argument[0]", "ReturnValue", "value", "df-generated"] - ["My.Qltest", "HE", false, "ExtensionMethod", "(My.Qltest.HI)", "", "Argument[0]", "ReturnValue", "value", "manual"] + - addsTo: + pack: codeql/csharp-all + extensible: neutralModel + # "namespace", "type", "name", "signature", "kind", "provenance" + data: + - ["My.Qltest", "G", "GeneratedFlowWithGeneratedNeutral", "(System.Object)", "summary", "df-generated"] + - ["My.Qltest", "G", "GeneratedFlowWithManualNeutral", "(System.Object)", "summary", "manual"] diff --git a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ql b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ql index 5aca9148ff1..63fbcb935e4 100644 --- a/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ql +++ b/csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ql @@ -21,11 +21,18 @@ module TaintConfig implements DataFlow::ConfigSig { module Taint = TaintTracking::Global; /** - * Simulate that methods with summaries are not included in the source code. - * This is relevant for dataflow analysis using summaries tagged as generated. + * Emulate that methods with summaries do not have a body. + * This is relevant for dataflow analysis using summaries with a generated like + * provenance as generated summaries are only applied, if a + * callable does not have a body. */ -private class MyMethod extends Method { - override predicate fromSource() { none() } +private class MethodsWithGeneratedModels extends Method { + MethodsWithGeneratedModels() { + this.hasFullyQualifiedName("My.Qltest", "G", + ["MixedFlowArgs", "GeneratedFlowWithGeneratedNeutral", "GeneratedFlowWithManualNeutral"]) + } + + override predicate hasBody() { none() } } from Taint::PathNode source, Taint::PathNode sink diff --git a/csharp/ql/test/library-tests/dataflow/external-models/Steps.cs b/csharp/ql/test/library-tests/dataflow/external-models/Steps.cs index ca3546a20f9..259e7dd078a 100644 --- a/csharp/ql/test/library-tests/dataflow/external-models/Steps.cs +++ b/csharp/ql/test/library-tests/dataflow/external-models/Steps.cs @@ -42,6 +42,12 @@ namespace My.Qltest gen.StepGeneric2(false); new Sub().StepOverride("string"); + + object arg4 = new object(); + this.StepArgQualGenerated(arg4); + + object arg5 = new object(); + this.StepArgQualGeneratedIgnored(arg5); } object StepArgRes(object x) { return null; } @@ -50,6 +56,10 @@ namespace My.Qltest void StepArgQual(object x) { } + void StepArgQualGenerated(object x) { } + + void StepArgQualGeneratedIgnored(object x) { } + object StepQualRes() { return null; } void StepQualArg(object @out) { } @@ -87,4 +97,4 @@ namespace My.Qltest public override string StepOverride(string i) => throw null; } } -} \ No newline at end of file +} diff --git a/csharp/ql/test/library-tests/dataflow/external-models/steps.expected b/csharp/ql/test/library-tests/dataflow/external-models/steps.expected index f73872ed9be..7c228c7817e 100644 --- a/csharp/ql/test/library-tests/dataflow/external-models/steps.expected +++ b/csharp/ql/test/library-tests/dataflow/external-models/steps.expected @@ -11,11 +11,12 @@ summaryThroughStep | Steps.cs:41:29:41:29 | 0 | Steps.cs:41:13:41:30 | call to method StepGeneric | true | | Steps.cs:42:30:42:34 | false | Steps.cs:42:13:42:35 | call to method StepGeneric2 | true | | Steps.cs:44:36:44:43 | "string" | Steps.cs:44:13:44:44 | call to method StepOverride | true | +| Steps.cs:47:39:47:42 | access to local variable arg4 | Steps.cs:47:13:47:16 | [post] this access | false | summaryGetterStep -| Steps.cs:28:13:28:16 | this access | Steps.cs:28:13:28:34 | call to method StepFieldGetter | Steps.cs:57:13:57:17 | field Field | -| Steps.cs:32:13:32:16 | this access | Steps.cs:32:13:32:37 | call to method StepPropertyGetter | Steps.cs:63:13:63:20 | property Property | +| Steps.cs:28:13:28:16 | this access | Steps.cs:28:13:28:34 | call to method StepFieldGetter | Steps.cs:67:13:67:17 | field Field | +| Steps.cs:32:13:32:16 | this access | Steps.cs:32:13:32:37 | call to method StepPropertyGetter | Steps.cs:73:13:73:20 | property Property | | Steps.cs:36:13:36:16 | this access | Steps.cs:36:13:36:36 | call to method StepElementGetter | file://:0:0:0:0 | element | summarySetterStep -| Steps.cs:30:34:30:34 | 0 | Steps.cs:30:13:30:16 | [post] this access | Steps.cs:57:13:57:17 | field Field | -| Steps.cs:34:37:34:37 | 0 | Steps.cs:34:13:34:16 | [post] this access | Steps.cs:63:13:63:20 | property Property | +| Steps.cs:30:34:30:34 | 0 | Steps.cs:30:13:30:16 | [post] this access | Steps.cs:67:13:67:17 | field Field | +| Steps.cs:34:37:34:37 | 0 | Steps.cs:34:13:34:16 | [post] this access | Steps.cs:73:13:73:20 | property Property | | Steps.cs:38:36:38:36 | 0 | Steps.cs:38:13:38:16 | [post] this access | file://:0:0:0:0 | element | diff --git a/csharp/ql/test/library-tests/dataflow/external-models/steps.ext.yml b/csharp/ql/test/library-tests/dataflow/external-models/steps.ext.yml index c6cf52c4ae8..5f422e2e532 100644 --- a/csharp/ql/test/library-tests/dataflow/external-models/steps.ext.yml +++ b/csharp/ql/test/library-tests/dataflow/external-models/steps.ext.yml @@ -18,3 +18,13 @@ extensions: - ["My.Qltest", "C+Generic", false, "StepGeneric", "(T)", "", "Argument[0]", "ReturnValue", "value", "manual"] - ["My.Qltest", "C+Generic", false, "StepGeneric2", "(S)", "", "Argument[0]", "ReturnValue", "value", "manual"] - ["My.Qltest", "C+Base", true, "StepOverride", "(T)", "", "Argument[0]", "ReturnValue", "value", "manual"] + - ["My.Qltest", "C", false, "StepArgQualGenerated", "(System.Object)", "", "Argument[0]", "Argument[this]", "taint", "df-generated"] + - ["My.Qltest", "C", false, "StepArgQualGeneratedIgnored", "(System.Object)", "", "Argument[0]", "Argument[this]", "taint", "df-generated"] + - addsTo: + pack: codeql/csharp-all + extensible: neutralModel + # "namespace", "type", "name", "signature", "kind", "provenance" + data: + - ["My.Qltest", "C", "StepArgQualGenerated", "(System.Object)", "summary", "df-generated"] + - ["My.Qltest", "C", "StepArgQualGeneratedIgnored", "(System.Object)", "summary", "manual"] + diff --git a/csharp/ql/test/library-tests/dataflow/external-models/steps.ql b/csharp/ql/test/library-tests/dataflow/external-models/steps.ql index 313f1dc9b7a..59d5c02258f 100644 --- a/csharp/ql/test/library-tests/dataflow/external-models/steps.ql +++ b/csharp/ql/test/library-tests/dataflow/external-models/steps.ql @@ -6,6 +6,22 @@ import semmle.code.csharp.dataflow.FlowSummary import semmle.code.csharp.dataflow.internal.DataFlowDispatch as DataFlowDispatch import semmle.code.csharp.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl +/** + * Emulate that methods with summaries do not have a body. + * This is relevant for dataflow analysis using summaries with a generated like + * provenance as generated summaries are only applied, if a + * callable does not have a body. + */ +private class StepArgQualGenerated extends Method { + StepArgQualGenerated() { + exists(string name | + this.hasFullyQualifiedName("My.Qltest", "C", name) and name.matches("StepArgQualGenerated%") + ) + } + + override predicate hasBody() { none() } +} + query predicate summaryThroughStep( DataFlow::Node node1, DataFlow::Node node2, boolean preservesValue ) { diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll index d2ce3d0a7d6..1666413db76 100644 --- a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll @@ -528,6 +528,10 @@ private class SummarizedCallableAdapter extends SummarizedCallable { exists(Provenance provenance | summaryElement(this, input, output, kind, provenance) and provenance.isGenerated() + ) and + not exists(Provenance provenance | + neutralElement(this, "summary", provenance) and + provenance.isManual() ) } diff --git a/java/ql/src/change-notes/2024-01-11-manual-neutral-model-blocks-generated-models.md b/java/ql/src/change-notes/2024-01-11-manual-neutral-model-blocks-generated-models.md new file mode 100644 index 00000000000..bdc5c1b0f2d --- /dev/null +++ b/java/ql/src/change-notes/2024-01-11-manual-neutral-model-blocks-generated-models.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* A manual neutral summary model for a callable now blocks all generated summary models for that callable from having any effect. diff --git a/java/ql/test/library-tests/dataflow/external-models/C.java b/java/ql/test/library-tests/dataflow/external-models/C.java index b2f35172488..b1c7f2dc85c 100644 --- a/java/ql/test/library-tests/dataflow/external-models/C.java +++ b/java/ql/test/library-tests/dataflow/external-models/C.java @@ -32,6 +32,11 @@ public class C { // The summary for the first parameter is ignored, because it is generated and // because there is hand written summary for the second parameter. stepArgResGeneratedIgnored(arg1, arg2); + + stepArgQualGenerated(arg1); + // The summary for the first parameter is ignored, because it is generated and + // because there is hand written neutral summary model for this callable. + stepArgQualGeneratedIgnored(arg1); } Object stepArgRes(Object x) { return null; } @@ -47,4 +52,8 @@ public class C { Object stepArgResGenerated(Object x) { return null; } Object stepArgResGeneratedIgnored(Object x, Object y) { return null; } + + Object stepArgQualGenerated(Object x) { return null; } + + Object stepArgQualGeneratedIgnored(Object x) { return null; } } diff --git a/java/ql/test/library-tests/dataflow/external-models/steps.expected b/java/ql/test/library-tests/dataflow/external-models/steps.expected index 9f19ff65f9f..c9b2fd4d01e 100644 --- a/java/ql/test/library-tests/dataflow/external-models/steps.expected +++ b/java/ql/test/library-tests/dataflow/external-models/steps.expected @@ -10,3 +10,4 @@ invalidModelRow | C.java:24:5:24:23 | this <.method> | C.java:24:17:24:22 | argOut [post update] | | C.java:29:25:29:28 | arg1 | C.java:29:5:29:29 | stepArgResGenerated(...) | | C.java:34:38:34:41 | arg2 | C.java:34:5:34:42 | stepArgResGeneratedIgnored(...) | +| C.java:36:26:36:29 | arg1 | C.java:36:5:36:30 | this <.method> [post update] | diff --git a/java/ql/test/library-tests/dataflow/external-models/steps.ext.yml b/java/ql/test/library-tests/dataflow/external-models/steps.ext.yml index c6a1fb69d6d..831079dd40b 100644 --- a/java/ql/test/library-tests/dataflow/external-models/steps.ext.yml +++ b/java/ql/test/library-tests/dataflow/external-models/steps.ext.yml @@ -11,3 +11,11 @@ extensions: - ["my.qltest", "C", False, "stepArgResGenerated", "(Object)", "", "Argument[0]", "ReturnValue", "taint", "df-generated"] - ["my.qltest", "C", False, "stepArgResGeneratedIgnored", "(Object,Object)", "", "Argument[0]", "ReturnValue", "taint", "df-generated"] - ["my.qltest", "C", False, "stepArgResGeneratedIgnored", "(Object,Object)", "", "Argument[1]", "ReturnValue", "taint", "manual"] + - ["my.qltest", "C", False, "stepArgQualGenerated", "(Object)", "", "Argument[0]", "Argument[this]", "taint", "df-generated"] + - ["my.qltest", "C", False, "stepArgQualGeneratedIgnored", "(Object)", "", "Argument[0]", "Argument[this]", "taint", "df-generated"] + - addsTo: + pack: codeql/java-all + extensible: neutralModel + data: + - ["my.qltest", "C", "stepArgQualGenerated", "(Object)", "summary", "df-generated"] + - ["my.qltest", "C", "stepArgQualGeneratedIgnored", "(Object)", "summary", "manual"]