Skip to content

Commit 0ec5aa6

Browse files
michaelnebelmichaelnebel
authored
Merge pull request github#8675 from michaelnebel/csharp/capturemodelimprovement
C#: CaptureModel improvements
2 parents b6309c9 + 6180970 commit 0ec5aa6

File tree

4 files changed

+76
-42
lines changed

4 files changed

+76
-42
lines changed

‎csharp/ql/src/utils/model-generator/internal/CaptureModelsSpecific.qll

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ private import semmle.code.csharp.commons.Util as Util
77
private import semmle.code.csharp.commons.Collections as Collections
88
private import semmle.code.csharp.dataflow.internal.DataFlowDispatch
99
private import semmle.code.csharp.frameworks.System as System
10+
private import semmle.code.csharp.frameworks.system.linq.Expressions
1011
import semmle.code.csharp.dataflow.ExternalFlow as ExternalFlow
1112
import semmle.code.csharp.dataflow.internal.DataFlowImplCommon as DataFlowImplCommon
1213
import semmle.code.csharp.dataflow.internal.DataFlowPrivate as DataFlowPrivate
@@ -18,21 +19,11 @@ module TaintTracking = CS::TaintTracking;
1819
class Type = CS::Type;
1920

2021
/**
21-
* Holds if `api` is an override or an interface implementation that
22-
* is irrelevant to the data flow analysis.
22+
* Holds if any of the parameters of `api` are `System.Func<>`.
2323
*/
24-
private predicate isIrrelevantOverrideOrImplementation(CS::Callable api) {
25-
exists(CS::Callable exclude, CS::Method m |
26-
(
27-
api = m.getAnOverrider*().getUnboundDeclaration()
28-
or
29-
api = m.getAnUltimateImplementor().getUnboundDeclaration()
30-
) and
31-
exclude = m.getUnboundDeclaration()
32-
|
33-
exists(System::SystemObjectClass c | exclude = [c.getGetHashCodeMethod(), c.getEqualsMethod()])
34-
or
35-
exists(System::SystemIEquatableTInterface i | exclude = i.getEqualsMethod())
24+
private predicate isHigherOrder(CS::Callable api) {
25+
exists(Type t | t = api.getAParameter().getType().getUnboundDeclaration() |
26+
t instanceof SystemLinqExpressions::DelegateExtType
3627
)
3728
}
3829

@@ -44,7 +35,7 @@ private predicate isRelevantForModels(CS::Callable api) {
4435
api.getDeclaringType().getNamespace().getQualifiedName() != "" and
4536
not api instanceof CS::ConversionOperator and
4637
not api instanceof Util::MainMethod and
47-
not isIrrelevantOverrideOrImplementation(api)
38+
not isHigherOrder(api)
4839
}
4940

5041
/**
@@ -65,8 +56,13 @@ predicate asPartialModel = DataFlowPrivate::Csv::asPartialModel/1;
6556
/**
6657
* Holds for type `t` for fields that are relevant as an intermediate
6758
* read or write step in the data flow analysis.
59+
* That is, flow through any data-flow node that does not have a relevant type
60+
* will be excluded.
6861
*/
69-
predicate isRelevantType(CS::Type t) { not t instanceof CS::Enum }
62+
predicate isRelevantType(CS::Type t) {
63+
not t instanceof CS::SimpleType and
64+
not t instanceof CS::Enum
65+
}
7066

7167
/**
7268
* Gets the CSV string representation of the qualifier.

‎csharp/ql/test/utils/model-generator/CaptureSummaryModels.expected

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
| NoSummaries;PublicClassFlow;false;PublicReturn;(System.Int32);;Argument[0];ReturnValue;generated:taint |
2-
| Summaries;BaseClassFlow;true;ReturnParam;(System.Int32);;Argument[0];ReturnValue;generated:taint |
1+
| NoSummaries;PublicClassFlow;false;PublicReturn;(System.Object);;Argument[0];ReturnValue;generated:taint |
2+
| Summaries;BaseClassFlow;true;ReturnParam;(System.Object);;Argument[0];ReturnValue;generated:taint |
33
| Summaries;BasicFlow;false;ReturnField;();;Argument[Qualifier];ReturnValue;generated:taint |
44
| Summaries;BasicFlow;false;ReturnParam0;(System.String,System.Object);;Argument[0];ReturnValue;generated:taint |
55
| Summaries;BasicFlow;false;ReturnParam1;(System.String,System.Object);;Argument[1];ReturnValue;generated:taint |
@@ -11,14 +11,14 @@
1111
| Summaries;CollectionFlow;false;AddFieldToList;(System.Collections.Generic.List<System.String>);;Argument[Qualifier];Argument[0].Element;generated:taint |
1212
| Summaries;CollectionFlow;false;AddToList;(System.Collections.Generic.List<System.Object>,System.Object);;Argument[1];Argument[0].Element;generated:taint |
1313
| Summaries;CollectionFlow;false;AssignFieldToArray;(System.Object[]);;Argument[Qualifier];Argument[0].Element;generated:taint |
14-
| Summaries;CollectionFlow;false;AssignToArray;(System.Int32,System.Int32[]);;Argument[0];Argument[1].Element;generated:taint |
15-
| Summaries;CollectionFlow;false;ReturnArrayElement;(System.Int32[]);;Argument[0].Element;ReturnValue;generated:taint |
14+
| Summaries;CollectionFlow;false;AssignToArray;(System.Object,System.Object[]);;Argument[0];Argument[1].Element;generated:taint |
15+
| Summaries;CollectionFlow;false;ReturnArrayElement;(System.Object[]);;Argument[0].Element;ReturnValue;generated:taint |
1616
| Summaries;CollectionFlow;false;ReturnFieldInAList;();;Argument[Qualifier];ReturnValue;generated:taint |
1717
| Summaries;CollectionFlow;false;ReturnListElement;(System.Collections.Generic.List<System.Object>);;Argument[0].Element;ReturnValue;generated:taint |
18-
| Summaries;DerivedClass1Flow;false;ReturnParam1;(System.Int32,System.Int32);;Argument[1];ReturnValue;generated:taint |
19-
| Summaries;DerivedClass2Flow;false;ReturnParam0;(System.Int32,System.Int32);;Argument[0];ReturnValue;generated:taint |
20-
| Summaries;DerivedClass2Flow;false;ReturnParam;(System.Int32);;Argument[0];ReturnValue;generated:taint |
21-
| Summaries;EqualsGetHashCodeNoFlow;false;Equals;(System.Int32);;Argument[0];ReturnValue;generated:taint |
18+
| Summaries;DerivedClass1Flow;false;ReturnParam1;(System.String,System.String);;Argument[1];ReturnValue;generated:taint |
19+
| Summaries;DerivedClass2Flow;false;ReturnParam0;(System.String,System.Int32);;Argument[0];ReturnValue;generated:taint |
20+
| Summaries;DerivedClass2Flow;false;ReturnParam;(System.Object);;Argument[0];ReturnValue;generated:taint |
21+
| Summaries;EqualsGetHashCodeNoFlow;false;Equals;(System.String);;Argument[0];ReturnValue;generated:taint |
2222
| Summaries;GenericFlow<>;false;AddFieldToGenericList;(System.Collections.Generic.List<T>);;Argument[Qualifier];Argument[0].Element;generated:taint |
2323
| Summaries;GenericFlow<>;false;AddToGenericList<>;(System.Collections.Generic.List<S>,S);;Argument[1];Argument[0].Element;generated:taint |
2424
| Summaries;GenericFlow<>;false;ReturnFieldInGenericList;();;Argument[Qualifier];ReturnValue;generated:taint |

‎csharp/ql/test/utils/model-generator/NoSummaries.cs

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,27 +6,27 @@ namespace NoSummaries;
66
// Just to prove that, if a method like this is correctly exposed, a flow summary will be captured.
77
public class PublicClassFlow
88
{
9-
public int PublicReturn(int input)
9+
public object PublicReturn(object input)
1010
{
1111
return input;
1212
}
1313
}
1414

1515
public sealed class PublicClassNoFlow
1616
{
17-
private int PrivateReturn(int input)
17+
private object PrivateReturn(object input)
1818
{
1919
return input;
2020
}
2121

22-
internal int InternalReturn(int input)
22+
internal object InternalReturn(object input)
2323
{
2424
return input;
2525
}
2626

2727
private class PrivateClassNoFlow
2828
{
29-
public int ReturnParam(int input)
29+
public object ReturnParam(object input)
3030
{
3131
return input;
3232
}
@@ -36,18 +36,18 @@ private class PrivateClassNestedPublicClassNoFlow
3636
{
3737
public class NestedPublicClassFlow
3838
{
39-
public int ReturnParam(int input)
39+
public object ReturnParam(object input)
4040
{
4141
return input;
4242
}
4343
}
4444
}
4545
}
4646

47-
public class EquatableBound : IEquatable<int>
47+
public class EquatableBound : IEquatable<object>
4848
{
4949
public readonly bool tainted;
50-
public bool Equals(int other)
50+
public bool Equals(object other)
5151
{
5252
return tainted;
5353
}
@@ -60,4 +60,42 @@ public bool Equals(T? other)
6060
{
6161
return tainted;
6262
}
63+
}
64+
65+
// No methods in this class will have generated flow summaries as
66+
// simple types are used.
67+
public class SimpleTypes
68+
{
69+
public bool M1(bool b)
70+
{
71+
return b;
72+
}
73+
74+
public Boolean M2(Boolean b)
75+
{
76+
return b;
77+
}
78+
79+
public int M3(int i)
80+
{
81+
return i;
82+
}
83+
84+
public Int32 M4(Int32 i)
85+
{
86+
return i;
87+
}
88+
}
89+
90+
public class HigherOrderParameters
91+
{
92+
public string M1(string s, Func<string, string> map)
93+
{
94+
return s;
95+
}
96+
97+
public object M2(Func<object, object> map, object o)
98+
{
99+
return map(o);
100+
}
63101
}

‎csharp/ql/test/utils/model-generator/Summaries.cs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,12 @@ public class CollectionFlow
4848
{
4949
private string tainted;
5050

51-
public int ReturnArrayElement(int[] input)
51+
public object ReturnArrayElement(object[] input)
5252
{
5353
return input[0];
5454
}
5555

56-
public void AssignToArray(int data, int[] target)
56+
public void AssignToArray(object data, object[] target)
5757
{
5858
target[0] = data;
5959
}
@@ -146,28 +146,28 @@ public void AddToGenericList<S>(List<S> input, S data)
146146

147147
public abstract class BaseClassFlow
148148
{
149-
public virtual int ReturnParam(int input)
149+
public virtual object ReturnParam(object input)
150150
{
151151
return input;
152152
}
153153
}
154154

155155
public class DerivedClass1Flow : BaseClassFlow
156156
{
157-
public int ReturnParam1(int input0, int input1)
157+
public string ReturnParam1(string input0, string input1)
158158
{
159159
return input1;
160160
}
161161
}
162162

163163
public class DerivedClass2Flow : BaseClassFlow
164164
{
165-
public override int ReturnParam(int input)
165+
public override object ReturnParam(object input)
166166
{
167167
return input;
168168
}
169169

170-
public int ReturnParam0(int input0, int input1)
170+
public string ReturnParam0(string input0, int input1)
171171
{
172172
return input0;
173173
}
@@ -195,13 +195,13 @@ public OperatorFlow(object o)
195195
}
196196

197197
// No flow summary as this is an implicit conversion operator.
198-
public static implicit operator OperatorFlow(int i)
198+
public static implicit operator OperatorFlow(string s)
199199
{
200-
return new OperatorFlow(i);
200+
return new OperatorFlow(s);
201201
}
202202

203203
// No flow summary as this is an explicit conversion operator.
204-
public static explicit operator OperatorFlow(byte b)
204+
public static explicit operator OperatorFlow(string[] b)
205205
{
206206
return new OperatorFlow(b);
207207
}
@@ -220,9 +220,9 @@ public override bool Equals(object obj)
220220
}
221221

222222
// Flow summary as this is not an override of the object Equals method.
223-
public int Equals(int i)
223+
public string Equals(string s)
224224
{
225-
return i;
225+
return s;
226226
}
227227

228228
// No flow summary as this is an override of the GetHashCode method.

0 commit comments

Comments
 (0)