hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-21 06:55:31 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #15246 from owen-mc/java/manual-neutral-overrides-generated

Browse Source 
C#/Java: Manual neutral summaries should block generated summaries
This commit is contained in:
Owen Mansel-Chan
2024-01-12 10:05:18 +00:00
committed by GitHub
parent ed4843f397 5e9ddd8c63
commit 6945289afc
15 changed files with 131 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
csharp/ql/lib/change-notes/2024-01-11-manual-neutral-model-blocks-generated-models.md Normal file
View File

@@ -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.

18
csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll
View File

@@ -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 }

13
csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.cs
View File

@@ -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) { }
}

22
csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.expected
View File

@@ -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 |

9
csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ext.yml
View File

@@ -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"]

15
csharp/ql/test/library-tests/dataflow/external-models/ExternalFlow.ql
View File

@@ -21,11 +21,18 @@ module TaintConfig implements DataFlow::ConfigSig {
module Taint = TaintTracking::Global<TaintConfig>;
/**
* 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

12
csharp/ql/test/library-tests/dataflow/external-models/Steps.cs
View File

@@ -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;
}
}
}
}

9
csharp/ql/test/library-tests/dataflow/external-models/steps.expected
View File

@@ -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<Boolean> | 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 |

10
csharp/ql/test/library-tests/dataflow/external-models/steps.ext.yml
View File

@@ -18,3 +18,13 @@ extensions:
- ["My.Qltest", "C+Generic<T,U>", false, "StepGeneric", "(T)", "", "Argument[0]", "ReturnValue", "value", "manual"]
- ["My.Qltest", "C+Generic<T,U>", false, "StepGeneric2<S>", "(S)", "", "Argument[0]", "ReturnValue", "value", "manual"]
- ["My.Qltest", "C+Base<T>", 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"]

16
csharp/ql/test/library-tests/dataflow/external-models/steps.ql
View File

@@ -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
) {

4
java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll
View File

@@ -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()
)
}

4
java/ql/src/change-notes/2024-01-11-manual-neutral-model-blocks-generated-models.md Normal file
View File

@@ -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.

9
java/ql/test/library-tests/dataflow/external-models/C.java
View File

@@ -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; }
}

1
java/ql/test/library-tests/dataflow/external-models/steps.expected
View File

@@ -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] |

8
java/ql/test/library-tests/dataflow/external-models/steps.ext.yml
View File

@@ -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"]