Skip to content

Commit fc8d8bb

Browse files
authored
Merge pull request github#17742 from michaelnebel/csharp/higherordermodels
C#: Models for higher order methods.
2 parents 3488b9f + 8041f00 commit fc8d8bb

File tree

6 files changed

+203
-10
lines changed

6 files changed

+203
-10
lines changed

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ private import semmle.code.csharp.Unification
1313
private import semmle.code.csharp.controlflow.Guards
1414
private import semmle.code.csharp.dispatch.Dispatch
1515
private import semmle.code.csharp.frameworks.EntityFramework
16+
private import semmle.code.csharp.frameworks.system.linq.Expressions
1617
private import semmle.code.csharp.frameworks.NHibernate
1718
private import semmle.code.csharp.frameworks.Razor
1819
private import semmle.code.csharp.frameworks.system.Collections
@@ -1146,7 +1147,11 @@ private module Cached {
11461147
TPrimaryConstructorParameterContent(Parameter p) {
11471148
p.getCallable() instanceof PrimaryConstructor
11481149
} or
1149-
TCapturedVariableContent(VariableCapture::CapturedVariable v)
1150+
TCapturedVariableContent(VariableCapture::CapturedVariable v) or
1151+
TDelegateCallArgumentContent(int i) {
1152+
i = [0 .. max(any(DelegateLikeCall dc).getNumberOfArguments()) - 1]
1153+
} or
1154+
TDelegateCallReturnContent()
11501155

11511156
cached
11521157
newtype TContentSet =
@@ -1162,7 +1167,9 @@ private module Cached {
11621167
TPrimaryConstructorParameterApproxContent(string firstChar) {
11631168
firstChar = approximatePrimaryConstructorParameterContent(_)
11641169
} or
1165-
TCapturedVariableContentApprox(VariableCapture::CapturedVariable v)
1170+
TCapturedVariableContentApprox(VariableCapture::CapturedVariable v) or
1171+
TDelegateCallArgumentApproxContent() or
1172+
TDelegateCallReturnApproxContent()
11661173

11671174
pragma[nomagic]
11681175
private predicate commonSubTypeGeneral(DataFlowTypeOrUnifiable t1, RelevantGvnType t2) {
@@ -2273,6 +2280,21 @@ private predicate recordProperty(RecordType t, ContentSet c, string name) {
22732280
)
22742281
}
22752282

2283+
/**
2284+
* Holds if data can flow from `node1` to `node2` via an assignment to
2285+
* the content set `c` of a delegate call.
2286+
*
2287+
* If there is a delegate call f(x), then we store "x" on "f"
2288+
* using a delegate argument content set.
2289+
*/
2290+
private predicate storeStepDelegateCall(ExplicitArgumentNode node1, ContentSet c, Node node2) {
2291+
exists(ExplicitDelegateLikeDataFlowCall call, int i |
2292+
node1.argumentOf(call, TPositionalArgumentPosition(i)) and
2293+
lambdaCall(call, _, node2.(PostUpdateNode).getPreUpdateNode()) and
2294+
c.isDelegateCallArgument(i)
2295+
)
2296+
}
2297+
22762298
/**
22772299
* Holds if data can flow from `node1` to `node2` via an assignment to
22782300
* content `c`.
@@ -2305,6 +2327,8 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
23052327
or
23062328
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), c,
23072329
node2.(FlowSummaryNode).getSummaryNode())
2330+
or
2331+
storeStepDelegateCall(node1, c, node2)
23082332
}
23092333

23102334
private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration {
@@ -2425,6 +2449,21 @@ private predicate readContentStep(Node node1, Content c, Node node2) {
24252449
VariableCapture::readStep(node1, c, node2)
24262450
}
24272451

2452+
/**
2453+
* Holds if data can flow from `node1` to `node2` via an assignment to
2454+
* the content set `c` of a delegate call.
2455+
*
2456+
* If there is a delegate call f(x), then we read the return of the delegate
2457+
* call.
2458+
*/
2459+
private predicate readStepDelegateCall(Node node1, ContentSet c, OutNode node2) {
2460+
exists(ExplicitDelegateLikeDataFlowCall call |
2461+
lambdaCall(call, _, node1) and
2462+
node2.getCall(TNormalReturnKind()) = call and
2463+
c.isDelegateCallReturn()
2464+
)
2465+
}
2466+
24282467
/**
24292468
* Holds if data can flow from `node1` to `node2` via a read of content `c`.
24302469
*/
@@ -2443,6 +2482,8 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
24432482
or
24442483
FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
24452484
node2.(FlowSummaryNode).getSummaryNode())
2485+
or
2486+
readStepDelegateCall(node1, c, node2)
24462487
}
24472488

24482489
private predicate clearsCont(Node n, Content c) {
@@ -3037,6 +3078,12 @@ class ContentApprox extends TContentApprox {
30373078
exists(VariableCapture::CapturedVariable v |
30383079
this = TCapturedVariableContentApprox(v) and result = "captured " + v
30393080
)
3081+
or
3082+
this = TDelegateCallArgumentApproxContent() and
3083+
result = "approximated delegate call argument"
3084+
or
3085+
this = TDelegateCallReturnApproxContent() and
3086+
result = "approximated delegate call return"
30403087
}
30413088
}
30423089

@@ -3073,6 +3120,12 @@ ContentApprox getContentApprox(Content c) {
30733120
TPrimaryConstructorParameterApproxContent(approximatePrimaryConstructorParameterContent(c))
30743121
or
30753122
result = TCapturedVariableContentApprox(VariableCapture::getCapturedVariableContent(c))
3123+
or
3124+
c instanceof DelegateCallArgumentContent and
3125+
result = TDelegateCallArgumentApproxContent()
3126+
or
3127+
c instanceof DelegateCallReturnContent and
3128+
result = TDelegateCallReturnApproxContent()
30763129
}
30773130

30783131
/**

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ private import DataFlowDispatch
33
private import DataFlowPrivate
44
private import semmle.code.csharp.controlflow.Guards
55
private import semmle.code.csharp.Unification
6+
private import semmle.code.csharp.frameworks.system.linq.Expressions
67

78
/**
89
* An element, viewed as a node in a data flow graph. Either an expression
@@ -238,6 +239,30 @@ class PropertyContent extends Content, TPropertyContent {
238239
override Location getLocation() { result = p.getLocation() }
239240
}
240241

242+
/**
243+
* A reference to the index of an argument of a delegate call.
244+
*/
245+
class DelegateCallArgumentContent extends Content, TDelegateCallArgumentContent {
246+
private int i;
247+
248+
DelegateCallArgumentContent() { this = TDelegateCallArgumentContent(i) }
249+
250+
override string toString() { result = "delegate argument at position " + i }
251+
252+
override Location getLocation() { result instanceof EmptyLocation }
253+
}
254+
255+
/**
256+
* A reference to the return of a delegate call.
257+
*/
258+
class DelegateCallReturnContent extends Content, TDelegateCallReturnContent {
259+
DelegateCallReturnContent() { this = TDelegateCallReturnContent() }
260+
261+
override string toString() { result = "delegate return" }
262+
263+
override Location getLocation() { result instanceof EmptyLocation }
264+
}
265+
241266
/**
242267
* A reference to a synthetic field corresponding to a
243268
* primary constructor parameter.
@@ -299,6 +324,16 @@ class ContentSet extends TContentSet {
299324
*/
300325
predicate isProperty(Property p) { this = TPropertyContentSet(p) }
301326

327+
/**
328+
* Holds if this content set represents the `i`th argument of a delegate call.
329+
*/
330+
predicate isDelegateCallArgument(int i) { this.isSingleton(TDelegateCallArgumentContent(i)) }
331+
332+
/**
333+
* Holds if this content set represents the return of a delegate call.
334+
*/
335+
predicate isDelegateCallReturn() { this.isSingleton(TDelegateCallReturnContent()) }
336+
302337
/** Holds if this content set represents the field `f`. */
303338
predicate isField(Field f) { this.isSingleton(TFieldContent(f)) }
304339

csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,9 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
101101
api = any(FlowSummaryImpl::Public::NeutralSinkCallable sc | sc.hasManualModel())
102102
}
103103

104-
predicate isUninterestingForDataFlowModels(Callable api) { isHigherOrder(api) }
104+
predicate isUninterestingForDataFlowModels(Callable api) { none() }
105+
106+
predicate isUninterestingForHeuristicDataFlowModels(Callable api) { isHigherOrder(api) }
105107

106108
class SourceOrSinkTargetApi extends Callable {
107109
SourceOrSinkTargetApi() { relevant(this) }
@@ -174,8 +176,15 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
174176
* Gets the underlying type of the content `c`.
175177
*/
176178
private CS::Type getUnderlyingContType(DataFlow::Content c) {
177-
result = c.(DataFlow::FieldContent).getField().getType() or
179+
result = c.(DataFlow::FieldContent).getField().getType()
180+
or
178181
result = c.(DataFlow::SyntheticFieldContent).getField().getType()
182+
or
183+
// Use System.Object as the type of delegate arguments and returns as the content doesn't
184+
// contain any type information.
185+
c instanceof DataFlow::DelegateCallArgumentContent and result instanceof ObjectType
186+
or
187+
c instanceof DataFlow::DelegateCallReturnContent and result instanceof ObjectType
179188
}
180189

181190
Type getUnderlyingContentType(DataFlow::ContentSet c) {
@@ -309,6 +318,10 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
309318
c.isField(_) or c.isSyntheticField(_) or c.isProperty(_)
310319
}
311320

321+
predicate isCallback(DataFlow::ContentSet c) {
322+
c.isDelegateCallArgument(_) or c.isDelegateCallReturn()
323+
}
324+
312325
string getSyntheticName(DataFlow::ContentSet c) {
313326
exists(CS::Field f |
314327
not f.isEffectivelyPublic() and
@@ -342,6 +355,10 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
342355
or
343356
c.isElement() and
344357
result = "Element"
358+
or
359+
exists(int i | c.isDelegateCallArgument(i) and result = "Parameter[" + i + "]")
360+
or
361+
c.isDelegateCallReturn() and result = "ReturnValue"
345362
}
346363

347364
predicate partialModel = ExternalFlow::partialModel/6;

csharp/ql/test/utils/modelgenerator/dataflow/Summaries.cs

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,15 @@ public string ReturnField()
6262
{
6363
return tainted;
6464
}
65+
66+
public Func<object, object> MyFunction;
67+
// summary=Models;BasicFlow;false;ApplyMyFunction;(System.Object);;Argument[0];Argument[this];taint;df-generated
68+
// summary=Models;BasicFlow;false;ApplyMyFunction;(System.Object);;Argument[this];ReturnValue;taint;df-generated
69+
// No content based flow as MaD doesn't support callback logic in fields and properties.
70+
public object ApplyMyFunction(object o)
71+
{
72+
return MyFunction(o);
73+
}
6574
}
6675

6776
public class CollectionFlow
@@ -497,18 +506,55 @@ public Type M6(Type t)
497506
}
498507
}
499508

500-
// No models as higher order methods are excluded
501-
// from model generation.
509+
// Methods in this class are "neutral" with respect to the heuristic model generation, but
510+
// the content based model generation is able to produce flow summaries for them.
502511
public class HigherOrderParameters
503512
{
513+
// neutral=Models;HigherOrderParameters;M1;(System.String,System.Func<System.String,System.String>);summary;df-generated
514+
// contentbased-summary=Models;HigherOrderParameters;false;M1;(System.String,System.Func<System.String,System.String>);;Argument[0];ReturnValue;value;dfc-generated
504515
public string M1(string s, Func<string, string> map)
505516
{
506517
return s;
507518
}
508519

509-
public object M2(Func<object, object> map, object o)
520+
// neutral=Models;HigherOrderParameters;Apply;(System.Func<System.Object,System.Object>,System.Object);summary;df-generated
521+
// contentbased-summary=Models;HigherOrderParameters;false;Apply;(System.Func<System.Object,System.Object>,System.Object);;Argument[1];Argument[0].Parameter[0];value;dfc-generated
522+
// contentbased-summary=Models;HigherOrderParameters;false;Apply;(System.Func<System.Object,System.Object>,System.Object);;Argument[0].ReturnValue;ReturnValue;value;dfc-generated
523+
public object Apply(Func<object, object> f, object o)
524+
{
525+
return f(o);
526+
}
527+
528+
// neutral=Models;HigherOrderParameters;Apply2;(System.Object,System.Func<System.Object,System.Object,System.Object>);summary;df-generated
529+
// contentbased-summary=Models;HigherOrderParameters;false;Apply2;(System.Object,System.Func<System.Object,System.Object,System.Object>);;Argument[0];Argument[1].Parameter[1];value;dfc-generated
530+
// contentbased-summary=Models;HigherOrderParameters;false;Apply2;(System.Object,System.Func<System.Object,System.Object,System.Object>);;Argument[1].ReturnValue;ReturnValue;value;dfc-generated
531+
public object Apply2(object o, Func<object, object, object> f)
510532
{
511-
return map(o);
533+
var x = f(null, o);
534+
return x;
535+
}
536+
537+
// neutral=Models;HigherOrderParameters;Apply;(System.Action<System.Object>,System.Object);summary;df-generated
538+
// contentbased-summary=Models;HigherOrderParameters;false;Apply;(System.Action<System.Object>,System.Object);;Argument[1];Argument[0].Parameter[0];value;dfc-generated
539+
public void Apply(Action<object> a, object o)
540+
{
541+
a(o);
542+
}
543+
}
544+
545+
public static class HigherOrderExtensionMethods
546+
{
547+
// neutral=Models;HigherOrderExtensionMethods;Select<TSource,TResult>;(System.Collections.Generic.IEnumerable<TSource>,System.Func<TSource,TResult>);summary;df-generated
548+
// contentbased-summary=Models;HigherOrderExtensionMethods;false;Select<TSource,TResult>;(System.Collections.Generic.IEnumerable<TSource>,System.Func<TSource,TResult>);;Argument[0].Element;Argument[1].Parameter[0];value;dfc-generated
549+
// contentbased-summary=Models;HigherOrderExtensionMethods;false;Select<TSource,TResult>;(System.Collections.Generic.IEnumerable<TSource>,System.Func<TSource,TResult>);;Argument[1].ReturnValue;ReturnValue.Element;value;dfc-generated
550+
public static IEnumerable<TResult> Select<TSource, TResult>(
551+
this IEnumerable<TSource> source,
552+
Func<TSource, TResult> selector)
553+
{
554+
foreach (var item in source)
555+
{
556+
yield return selector(item);
557+
}
512558
}
513559
}
514560

java/ql/src/utils/modelgenerator/internal/CaptureModels.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, JavaDataF
8888
api.getDeclaringType() instanceof J::Interface and not exists(api.getBody())
8989
}
9090

91+
predicate isUninterestingForHeuristicDataFlowModels(Callable api) { none() }
92+
9193
class SourceOrSinkTargetApi extends Callable {
9294
SourceOrSinkTargetApi() { relevant(this) }
9395
}
@@ -252,6 +254,8 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, JavaDataF
252254
c instanceof DataFlowUtil::SyntheticFieldContent
253255
}
254256

257+
predicate isCallback(DataFlow::ContentSet c) { none() }
258+
255259
string getSyntheticName(DataFlow::ContentSet c) {
256260
exists(Field f |
257261
not f.isPublic() and

0 commit comments

Comments
 (0)