Skip to content

Commit 6945289

Browse files
owen-mcowen-mc
authored
Merge pull request github#15246 from owen-mc/java/manual-neutral-overrides-generated
C#/Java: Manual neutral summaries should block generated summaries
2 parents ed4843f + 5e9ddd8 commit 6945289

File tree

15 files changed

+131
-23
lines changed

15 files changed

+131
-23
lines changed

‎csharp/ql/lib/change-notes/2024-01-11-manual-neutral-model-blocks-generated-models.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* A manual neutral summary model for a callable now blocks all generated summary models for that callable from having any effect.

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,13 @@ private predicate interpretSummary(
529529
)
530530
}
531531

532+
private predicate interpretNeutral(UnboundCallable c, string kind, string provenance) {
533+
exists(string namespace, string type, string name, string signature |
534+
neutralModel(namespace, type, name, signature, kind, provenance) and
535+
c = interpretElement(namespace, type, false, name, signature, "")
536+
)
537+
}
538+
532539
// adapter class for converting Mad summaries to `SummarizedCallable`s
533540
private class SummarizedCallableAdapter extends SummarizedCallable {
534541
SummarizedCallableAdapter() { interpretSummary(this, _, _, _, _) }
@@ -544,6 +551,10 @@ private class SummarizedCallableAdapter extends SummarizedCallable {
544551
exists(Provenance provenance |
545552
interpretSummary(this, input, output, kind, provenance) and
546553
provenance.isGenerated()
554+
) and
555+
not exists(Provenance provenance |
556+
interpretNeutral(this, "summary", provenance) and
557+
provenance.isManual()
547558
)
548559
}
549560

@@ -568,12 +579,7 @@ private class NeutralCallableAdapter extends NeutralCallable {
568579
string kind;
569580
string provenance_;
570581

571-
NeutralCallableAdapter() {
572-
exists(string namespace, string type, string name, string signature |
573-
neutralModel(namespace, type, name, signature, kind, provenance_) and
574-
this = interpretElement(namespace, type, false, name, signature, "")
575-
)
576-
}
582+
NeutralCallableAdapter() { interpretNeutral(this, kind, provenance_) }
577583

578584
override string getKind() { result = kind }
579585

‎csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,25 @@ void M3()
206206
Sink(MixedFlowArgs(null, o2));
207207
}
208208

209+
void M4()
210+
{
211+
var o1 = new object();
212+
Sink(GeneratedFlowWithGeneratedNeutral(o1));
213+
214+
var o2 = new object();
215+
Sink(GeneratedFlowWithManualNeutral(o2)); // no flow because the modelled method has a manual neutral summary model
216+
}
217+
209218
object GeneratedFlow(object o) => throw null;
210219

211220
object GeneratedFlowArgs(object o1, object o2) => throw null;
212221

213222
object MixedFlowArgs(object o1, object o2) => throw null;
214223

224+
object GeneratedFlowWithGeneratedNeutral(object o) => throw null;
225+
226+
object GeneratedFlowWithManualNeutral(object o) => throw null;
227+
215228
static void Sink(object o) { }
216229
}
217230

‎csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,11 @@ edges
6363
| ExternalFlow.cs:120:18:120:18 | access to local variable b : null [element] : Object | ExternalFlow.cs:120:18:120:21 | access to array element |
6464
| ExternalFlow.cs:205:22:205:33 | object creation of type Object : Object | ExternalFlow.cs:206:38:206:39 | access to local variable o2 : Object |
6565
| ExternalFlow.cs:206:38:206:39 | access to local variable o2 : Object | ExternalFlow.cs:206:18:206:40 | call to method MixedFlowArgs |
66-
| ExternalFlow.cs:231:21:231:28 | object creation of type HC : HC | ExternalFlow.cs:232:21:232:21 | access to local variable h : HC |
67-
| ExternalFlow.cs:232:21:232:21 | access to local variable h : HC | ExternalFlow.cs:232:21:232:39 | call to method ExtensionMethod : HC |
68-
| ExternalFlow.cs:232:21:232:39 | call to method ExtensionMethod : HC | ExternalFlow.cs:233:18:233:18 | access to local variable o |
66+
| ExternalFlow.cs:211:22:211:33 | object creation of type Object : Object | ExternalFlow.cs:212:52:212:53 | access to local variable o1 : Object |
67+
| ExternalFlow.cs:212:52:212:53 | access to local variable o1 : Object | ExternalFlow.cs:212:18:212:54 | call to method GeneratedFlowWithGeneratedNeutral |
68+
| ExternalFlow.cs:244:21:244:28 | object creation of type HC : HC | ExternalFlow.cs:245:21:245:21 | access to local variable h : HC |
69+
| ExternalFlow.cs:245:21:245:21 | access to local variable h : HC | ExternalFlow.cs:245:21:245:39 | call to method ExtensionMethod : HC |
70+
| ExternalFlow.cs:245:21:245:39 | call to method ExtensionMethod : HC | ExternalFlow.cs:246:18:246:18 | access to local variable o |
6971
nodes
7072
| ExternalFlow.cs:9:27:9:38 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
7173
| ExternalFlow.cs:10:18:10:33 | call to method StepArgRes | semmle.label | call to method StepArgRes |
@@ -148,10 +150,13 @@ nodes
148150
| ExternalFlow.cs:205:22:205:33 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
149151
| ExternalFlow.cs:206:18:206:40 | call to method MixedFlowArgs | semmle.label | call to method MixedFlowArgs |
150152
| ExternalFlow.cs:206:38:206:39 | access to local variable o2 : Object | semmle.label | access to local variable o2 : Object |
151-
| ExternalFlow.cs:231:21:231:28 | object creation of type HC : HC | semmle.label | object creation of type HC : HC |
152-
| ExternalFlow.cs:232:21:232:21 | access to local variable h : HC | semmle.label | access to local variable h : HC |
153-
| ExternalFlow.cs:232:21:232:39 | call to method ExtensionMethod : HC | semmle.label | call to method ExtensionMethod : HC |
154-
| ExternalFlow.cs:233:18:233:18 | access to local variable o | semmle.label | access to local variable o |
153+
| ExternalFlow.cs:211:22:211:33 | object creation of type Object : Object | semmle.label | object creation of type Object : Object |
154+
| ExternalFlow.cs:212:18:212:54 | call to method GeneratedFlowWithGeneratedNeutral | semmle.label | call to method GeneratedFlowWithGeneratedNeutral |
155+
| ExternalFlow.cs:212:52:212:53 | access to local variable o1 : Object | semmle.label | access to local variable o1 : Object |
156+
| ExternalFlow.cs:244:21:244:28 | object creation of type HC : HC | semmle.label | object creation of type HC : HC |
157+
| ExternalFlow.cs:245:21:245:21 | access to local variable h : HC | semmle.label | access to local variable h : HC |
158+
| ExternalFlow.cs:245:21:245:39 | call to method ExtensionMethod : HC | semmle.label | call to method ExtensionMethod : HC |
159+
| ExternalFlow.cs:246:18:246:18 | access to local variable o | semmle.label | access to local variable o |
155160
subpaths
156161
#select
157162
| 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
175180
| 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 |
176181
| 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 |
177182
| 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 |
178-
| 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 |
183+
| 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 |
184+
| 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 |

‎csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ext.yml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,13 @@ extensions:
2929
- ["My.Qltest", "G", false, "GeneratedFlowArgs", "(System.Object,System.Object)", "", "Argument[1]", "ReturnValue", "value", "df-generated"]
3030
- ["My.Qltest", "G", false, "MixedFlowArgs", "(System.Object,System.Object)", "", "Argument[0]", "ReturnValue", "value", "df-generated"]
3131
- ["My.Qltest", "G", false, "MixedFlowArgs", "(System.Object,System.Object)", "", "Argument[1]", "ReturnValue", "value", "manual"]
32+
- ["My.Qltest", "G", false, "GeneratedFlowWithGeneratedNeutral", "(System.Object)", "", "Argument[0]", "ReturnValue", "value", "df-generated"]
33+
- ["My.Qltest", "G", false, "GeneratedFlowWithManualNeutral", "(System.Object)", "", "Argument[0]", "ReturnValue", "value", "df-generated"]
3234
- ["My.Qltest", "HE", false, "ExtensionMethod", "(My.Qltest.HI)", "", "Argument[0]", "ReturnValue", "value", "manual"]
35+
- addsTo:
36+
pack: codeql/csharp-all
37+
extensible: neutralModel
38+
# "namespace", "type", "name", "signature", "kind", "provenance"
39+
data:
40+
- ["My.Qltest", "G", "GeneratedFlowWithGeneratedNeutral", "(System.Object)", "summary", "df-generated"]
41+
- ["My.Qltest", "G", "GeneratedFlowWithManualNeutral", "(System.Object)", "summary", "manual"]

‎csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ql

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,18 @@ module TaintConfig implements DataFlow::ConfigSig {
2121
module Taint = TaintTracking::Global<TaintConfig>;
2222

2323
/**
24-
* Simulate that methods with summaries are not included in the source code.
25-
* This is relevant for dataflow analysis using summaries tagged as generated.
24+
* Emulate that methods with summaries do not have a body.
25+
* This is relevant for dataflow analysis using summaries with a generated like
26+
* provenance as generated summaries are only applied, if a
27+
* callable does not have a body.
2628
*/
27-
private class MyMethod extends Method {
28-
override predicate fromSource() { none() }
29+
private class MethodsWithGeneratedModels extends Method {
30+
MethodsWithGeneratedModels() {
31+
this.hasFullyQualifiedName("My.Qltest", "G",
32+
["MixedFlowArgs", "GeneratedFlowWithGeneratedNeutral", "GeneratedFlowWithManualNeutral"])
33+
}
34+
35+
override predicate hasBody() { none() }
2936
}
3037

3138
from Taint::PathNode source, Taint::PathNode sink

‎csharp/ql/test/library-tests/dataflow/external-models/Steps.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ void Foo()
4242
gen.StepGeneric2(false);
4343

4444
new Sub().StepOverride("string");
45+
46+
object arg4 = new object();
47+
this.StepArgQualGenerated(arg4);
48+
49+
object arg5 = new object();
50+
this.StepArgQualGeneratedIgnored(arg5);
4551
}
4652

4753
object StepArgRes(object x) { return null; }
@@ -50,6 +56,10 @@ void StepArgArg(object @in, object @out) { }
5056

5157
void StepArgQual(object x) { }
5258

59+
void StepArgQualGenerated(object x) { }
60+
61+
void StepArgQualGeneratedIgnored(object x) { }
62+
5363
object StepQualRes() { return null; }
5464

5565
void StepQualArg(object @out) { }
@@ -87,4 +97,4 @@ class Sub : Base<string>
8797
public override string StepOverride(string i) => throw null;
8898
}
8999
}
90-
}
100+
}

‎csharp/ql/test/library-tests/dataflow/external-models/steps.expected

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@ summaryThroughStep
1111
| Steps.cs:41:29:41:29 | 0 | Steps.cs:41:13:41:30 | call to method StepGeneric | true |
1212
| Steps.cs:42:30:42:34 | false | Steps.cs:42:13:42:35 | call to method StepGeneric2<Boolean> | true |
1313
| Steps.cs:44:36:44:43 | "string" | Steps.cs:44:13:44:44 | call to method StepOverride | true |
14+
| Steps.cs:47:39:47:42 | access to local variable arg4 | Steps.cs:47:13:47:16 | [post] this access | false |
1415
summaryGetterStep
15-
| 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 |
16-
| 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 |
16+
| 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 |
17+
| 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 |
1718
| Steps.cs:36:13:36:16 | this access | Steps.cs:36:13:36:36 | call to method StepElementGetter | file://:0:0:0:0 | element |
1819
summarySetterStep
19-
| Steps.cs:30:34:30:34 | 0 | Steps.cs:30:13:30:16 | [post] this access | Steps.cs:57:13:57:17 | field Field |
20-
| Steps.cs:34:37:34:37 | 0 | Steps.cs:34:13:34:16 | [post] this access | Steps.cs:63:13:63:20 | property Property |
20+
| Steps.cs:30:34:30:34 | 0 | Steps.cs:30:13:30:16 | [post] this access | Steps.cs:67:13:67:17 | field Field |
21+
| Steps.cs:34:37:34:37 | 0 | Steps.cs:34:13:34:16 | [post] this access | Steps.cs:73:13:73:20 | property Property |
2122
| Steps.cs:38:36:38:36 | 0 | Steps.cs:38:13:38:16 | [post] this access | file://:0:0:0:0 | element |

‎csharp/ql/test/library-tests/dataflow/external-models/steps.ext.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,13 @@ extensions:
1818
- ["My.Qltest", "C+Generic<T,U>", false, "StepGeneric", "(T)", "", "Argument[0]", "ReturnValue", "value", "manual"]
1919
- ["My.Qltest", "C+Generic<T,U>", false, "StepGeneric2<S>", "(S)", "", "Argument[0]", "ReturnValue", "value", "manual"]
2020
- ["My.Qltest", "C+Base<T>", true, "StepOverride", "(T)", "", "Argument[0]", "ReturnValue", "value", "manual"]
21+
- ["My.Qltest", "C", false, "StepArgQualGenerated", "(System.Object)", "", "Argument[0]", "Argument[this]", "taint", "df-generated"]
22+
- ["My.Qltest", "C", false, "StepArgQualGeneratedIgnored", "(System.Object)", "", "Argument[0]", "Argument[this]", "taint", "df-generated"]
23+
- addsTo:
24+
pack: codeql/csharp-all
25+
extensible: neutralModel
26+
# "namespace", "type", "name", "signature", "kind", "provenance"
27+
data:
28+
- ["My.Qltest", "C", "StepArgQualGenerated", "(System.Object)", "summary", "df-generated"]
29+
- ["My.Qltest", "C", "StepArgQualGeneratedIgnored", "(System.Object)", "summary", "manual"]
30+

‎csharp/ql/test/library-tests/dataflow/external-models/steps.ql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,22 @@ import semmle.code.csharp.dataflow.FlowSummary
66
import semmle.code.csharp.dataflow.internal.DataFlowDispatch as DataFlowDispatch
77
import semmle.code.csharp.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
88

9+
/**
10+
* Emulate that methods with summaries do not have a body.
11+
* This is relevant for dataflow analysis using summaries with a generated like
12+
* provenance as generated summaries are only applied, if a
13+
* callable does not have a body.
14+
*/
15+
private class StepArgQualGenerated extends Method {
16+
StepArgQualGenerated() {
17+
exists(string name |
18+
this.hasFullyQualifiedName("My.Qltest", "C", name) and name.matches("StepArgQualGenerated%")
19+
)
20+
}
21+
22+
override predicate hasBody() { none() }
23+
}
24+
925
query predicate summaryThroughStep(
1026
DataFlow::Node node1, DataFlow::Node node2, boolean preservesValue
1127
) {

0 commit comments

Comments
 (0)