Skip to content

Commit 33133d8

Browse files
hvitvedhvitved
committed
C#: Adapt to changes in FlowSummaryImpl 
1 parent 574ad83 commit 33133d8

File tree

12 files changed

+60
-3874
lines changed

12 files changed

+60
-3874
lines changed

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

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -371,24 +371,9 @@ class NonDelegateDataFlowCall extends DataFlowCall, TNonDelegateCall {
371371
/** Gets the underlying call. */
372372
DispatchCall getDispatchCall() { result = dc }
373373

374-
pragma[nomagic]
375-
private predicate hasSourceTarget() { dc.getAStaticTarget().fromSource() }
376-
377374
pragma[nomagic]
378375
private FlowSummary::SummarizedCallable getASummarizedCallableTarget() {
379-
// Only use summarized callables with generated summaries in case
380-
// we are not able to dispatch to a source declaration.
381-
exists(boolean static |
382-
result = this.getATarget(static) and
383-
not (
384-
result.applyGeneratedModel() and
385-
this.hasSourceTarget()
386-
)
387-
|
388-
static = false
389-
or
390-
static = true and not result instanceof RuntimeCallable
391-
)
376+
result = this.getATarget(_)
392377
}
393378

394379
pragma[nomagic]

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,7 +848,7 @@ private predicate fieldOrPropertyStore(Expr e, ContentSet c, Expr src, Expr q, b
848848
FlowSummaryImpl::Private::SummarizedCallableImpl sc,
849849
FlowSummaryImpl::Private::SummaryComponentStack input, ContentSet readSet
850850
|
851-
sc.propagatesFlow(input, _, _, _) and
851+
sc.propagatesFlow(input, _, _, _, _, _) and
852852
input.contains(FlowSummaryImpl::Private::SummaryComponent::content(readSet)) and
853853
c.getAStoreContent() = readSet.getAReadContent()
854854
)

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

Lines changed: 26 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -349,20 +349,23 @@ private Declaration interpretExt(Declaration d, ExtPath ext) {
349349
/** Gets the source/sink/summary/neutral element corresponding to the supplied parameters. */
350350
pragma[nomagic]
351351
Declaration interpretElement(
352-
string namespace, string type, boolean subtypes, string name, string signature, string ext
352+
string namespace, string type, boolean subtypes, string name, string signature, string ext,
353+
boolean isExact
353354
) {
354355
elementSpec(namespace, type, subtypes, name, signature, ext) and
355356
exists(Declaration base, Declaration d |
356357
base = interpretBaseDeclaration(namespace, type, name, signature) and
357358
(
358-
d = base
359+
d = base and
360+
isExact = true
359361
or
360362
subtypes = true and
361363
(
362364
d.(UnboundCallable).overridesOrImplementsUnbound(base)
363365
or
364366
d = base.(UnboundValueOrRefType).getASubTypeUnbound+()
365-
)
367+
) and
368+
isExact = false
366369
)
367370
|
368371
result = interpretExt(d, ext)
@@ -500,71 +503,47 @@ string getSignature(UnboundCallable c) {
500503
}
501504

502505
private predicate interpretSummary(
503-
UnboundCallable c, string input, string output, string kind, string provenance, string model
506+
UnboundCallable c, string input, string output, string kind, string provenance, boolean isExact,
507+
string model
504508
) {
505509
exists(
506510
string namespace, string type, boolean subtypes, string name, string signature, string ext
507511
|
508512
summaryModel(namespace, type, subtypes, name, signature, ext, input, output, kind, provenance,
509513
model) and
510-
c = interpretElement(namespace, type, subtypes, name, signature, ext)
514+
c = interpretElement(namespace, type, subtypes, name, signature, ext, isExact)
511515
)
512516
}
513517

514-
predicate interpretNeutral(UnboundCallable c, string kind, string provenance) {
518+
predicate interpretNeutral(UnboundCallable c, string kind, string provenance, boolean isExact) {
515519
exists(string namespace, string type, string name, string signature |
516520
Extensions::neutralModel(namespace, type, name, signature, kind, provenance) and
517-
c = interpretElement(namespace, type, true, name, signature, "")
521+
c = interpretElement(namespace, type, true, name, signature, "", isExact)
518522
)
519523
}
520524

521525
// adapter class for converting Mad summaries to `SummarizedCallable`s
522526
private class SummarizedCallableAdapter extends SummarizedCallable {
523-
SummarizedCallableAdapter() {
524-
exists(Provenance provenance | interpretSummary(this, _, _, _, provenance, _) |
525-
not this.fromSource()
526-
or
527-
this.fromSource() and provenance.isManual()
528-
)
529-
}
530-
531-
private predicate relevantSummaryElementManual(
532-
string input, string output, string kind, string model
533-
) {
534-
exists(Provenance provenance |
535-
interpretSummary(this, input, output, kind, provenance, model) and
536-
provenance.isManual()
537-
)
538-
}
527+
string input_;
528+
string output_;
529+
string kind;
530+
Provenance p_;
531+
boolean isExact_;
532+
string model_;
539533

540-
private predicate relevantSummaryElementGenerated(
541-
string input, string output, string kind, string model
542-
) {
543-
exists(Provenance provenance |
544-
interpretSummary(this, input, output, kind, provenance, model) and
545-
provenance.isGenerated()
546-
) and
547-
not exists(Provenance provenance |
548-
interpretNeutral(this, "summary", provenance) and
549-
provenance.isManual()
550-
)
534+
SummarizedCallableAdapter() {
535+
interpretSummary(this, input_, output_, kind, p_, isExact_, model_)
551536
}
552537

553538
override predicate propagatesFlow(
554-
string input, string output, boolean preservesValue, string model
539+
string input, string output, boolean preservesValue, Provenance p, boolean isExact, string model
555540
) {
556-
exists(string kind |
557-
this.relevantSummaryElementManual(input, output, kind, model)
558-
or
559-
not this.relevantSummaryElementManual(_, _, _, _) and
560-
this.relevantSummaryElementGenerated(input, output, kind, model)
561-
|
562-
if kind = "value" then preservesValue = true else preservesValue = false
563-
)
564-
}
565-
566-
override predicate hasProvenance(Provenance provenance) {
567-
interpretSummary(this, _, _, _, provenance, _)
541+
input = input_ and
542+
output = output_ and
543+
(if kind = "value" then preservesValue = true else preservesValue = false) and
544+
p = p_ and
545+
isExact = isExact_ and
546+
model = model_
568547
}
569548
}
570549

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@ module Input implements InputSig<Location, DataFlowImplSpecific::CsharpDataFlow>
1818

1919
class SummarizedCallableBase = UnboundCallable;
2020

21+
predicate allowGeneratedSummary(SummarizedCallableBase c) { not c.fromSource() }
22+
2123
class SourceBase = Void;
2224

2325
class SinkBase = Void;
2426

2527
predicate neutralElement(SummarizedCallableBase c, string kind, string provenance, boolean isExact) {
26-
interpretNeutral(c, kind, provenance) and
28+
interpretNeutral(c, kind, provenance, _) and
2729
// isExact is not needed for C#.
2830
isExact = false
2931
}
@@ -216,7 +218,7 @@ module SourceSinkInterpretationInput implements
216218
string namespace, string type, boolean subtypes, string name, string signature, string ext
217219
|
218220
sourceModel(namespace, type, subtypes, name, signature, ext, output, kind, provenance, model) and
219-
e = interpretElement(namespace, type, subtypes, name, signature, ext)
221+
e = interpretElement(namespace, type, subtypes, name, signature, ext, _)
220222
)
221223
}
222224

@@ -227,7 +229,7 @@ module SourceSinkInterpretationInput implements
227229
string namespace, string type, boolean subtypes, string name, string signature, string ext
228230
|
229231
sinkModel(namespace, type, subtypes, name, signature, ext, input, kind, provenance, model) and
230-
e = interpretElement(namespace, type, subtypes, name, signature, ext)
232+
e = interpretElement(namespace, type, subtypes, name, signature, ext, _)
231233
)
232234
}
233235

@@ -437,13 +439,14 @@ private class SummarizedCallableWithCallback extends Public::SummarizedCallable
437439
SummarizedCallableWithCallback() { mayInvokeCallback(this, pos) }
438440

439441
override predicate propagatesFlow(
440-
string input, string output, boolean preservesValue, string model
442+
string input, string output, boolean preservesValue, Public::Provenance provenance,
443+
boolean isExact, string model
441444
) {
442445
input = "Argument[" + pos + "]" and
443446
output = "Argument[" + pos + "].Parameter[delegate-self]" and
444447
preservesValue = true and
448+
provenance = "hq-generated" and
449+
isExact = true and
445450
model = "heuristic-callback"
446451
}
447-
448-
override predicate hasProvenance(Public::Provenance provenance) { provenance = "hq-generated" }
449452
}

‎csharp/ql/lib/semmle/code/csharp/frameworks/EntityFramework.qll‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -92,22 +92,17 @@ module EntityFramework {
9292
abstract class EFSummarizedCallable extends SummarizedCallableImpl {
9393
bindingset[this]
9494
EFSummarizedCallable() { any() }
95-
96-
override predicate hasProvenance(Provenance provenance) { provenance = "manual" }
9795
}
9896

9997
// see `SummarizedCallableImpl` qldoc
10098
private class EFSummarizedCallableAdapter extends SummarizedCallable instanceof EFSummarizedCallable
10199
{
102100
override predicate propagatesFlow(
103-
string input, string output, boolean preservesValue, string model
101+
string input, string output, boolean preservesValue, Provenance provenance, boolean isExact,
102+
string model
104103
) {
105104
none()
106105
}
107-
108-
override predicate hasProvenance(Provenance provenance) {
109-
EFSummarizedCallable.super.hasProvenance(provenance)
110-
}
111106
}
112107

113108
/** The class ``Microsoft.EntityFrameworkCore.DbQuery`1`` or ``System.Data.Entity.DbQuery`1``. */
@@ -177,11 +172,13 @@ module EntityFramework {
177172

178173
override predicate propagatesFlow(
179174
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue,
180-
string model
175+
Provenance p, boolean isExact, string model
181176
) {
182177
input = SummaryComponentStack::argument(0) and
183178
output = SummaryComponentStack::return() and
184179
preservesValue = false and
180+
p = "manual" and
181+
isExact = true and
185182
model = "RawSqlStringConstructorSummarizedCallable"
186183
}
187184
}
@@ -193,11 +190,13 @@ module EntityFramework {
193190

194191
override predicate propagatesFlow(
195192
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue,
196-
string model
193+
Provenance p, boolean isExact, string model
197194
) {
198195
input = SummaryComponentStack::argument(0) and
199196
output = SummaryComponentStack::return() and
200197
preservesValue = false and
198+
p = "manual" and
199+
isExact = true and
201200
model = "RawSqlStringConversionSummarizedCallable"
202201
}
203202
}
@@ -459,18 +458,20 @@ module EntityFramework {
459458
}
460459

461460
private class DbContextClassSetPropertySynthetic extends EFSummarizedCallable {
462-
private DbContextClassSetProperty p;
461+
private DbContextClassSetProperty prop;
463462

464-
DbContextClassSetPropertySynthetic() { this = p.getGetter() }
463+
DbContextClassSetPropertySynthetic() { this = prop.getGetter() }
465464

466465
override predicate propagatesFlow(
467466
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue,
468-
string model
467+
Provenance p, boolean isExact, string model
469468
) {
470469
exists(string name, DbContextClass c |
471470
preservesValue = true and
472-
name = c.getSyntheticName(output, _, p) and
471+
name = c.getSyntheticName(output, _, prop) and
473472
input = SummaryComponentStack::syntheticGlobal(name) and
473+
p = "manual" and
474+
isExact = true and
474475
model = "DbContextClassSetPropertySynthetic"
475476
)
476477
}
@@ -483,13 +484,15 @@ module EntityFramework {
483484

484485
override predicate propagatesFlow(
485486
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue,
486-
string model
487+
Provenance p, boolean isExact, string model
487488
) {
488489
exists(string name, Property mapped |
489490
preservesValue = true and
490491
c.input(input, mapped) and
491492
name = c.getSyntheticNameProj(mapped) and
492493
output = SummaryComponentStack::syntheticGlobal(name) and
494+
p = "manual" and
495+
isExact = true and
493496
model = "DbContextSaveChanges"
494497
)
495498
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ module SummaryModelGeneratorInput implements SummaryModelGeneratorInputSig {
230230
}
231231

232232
private predicate hasManualSummaryModel(Callable api) {
233-
api = any(FlowSummaryImpl::Public::SummarizedCallable sc | sc.applyManualModel()) or
233+
api = any(FlowSummaryImpl::Public::SummarizedCallable sc | sc.hasManualModel()) or
234234
api = any(FlowSummaryImpl::Public::NeutralSummaryCallable sc | sc.hasManualModel())
235235
}
236236

‎csharp/ql/test/library-tests/dataflow/global/DataFlowPath.expected‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ edges
2929
| Capture.cs:29:26:29:32 | access to parameter tainted : String | Capture.cs:29:17:29:22 | access to local variable sink29 : String | provenance | |
3030
| Capture.cs:33:30:33:39 | access to local variable captureIn3 : Func<String,String> [captured tainted] : String | Capture.cs:29:26:29:32 | access to parameter tainted : String | provenance | MaD:3 |
3131
| Capture.cs:33:30:33:39 | access to local variable captureIn3 : Func<String,String> [captured tainted] : String | Capture.cs:29:26:29:32 | access to parameter tainted : String | provenance | MaD:4 |
32-
| Capture.cs:33:30:33:39 | access to local variable captureIn3 : Func<String,String> [captured tainted] : String | Capture.cs:29:26:29:32 | access to parameter tainted : String | provenance | heuristic-callback |
3332
| Capture.cs:50:50:50:55 | sink39 : String | Capture.cs:52:23:59:13 | (...) => ... : (...) => ... [captured sink39] : String | provenance | |
3433
| Capture.cs:52:23:59:13 | (...) => ... : (...) => ... [captured sink39] : String | Capture.cs:350:34:350:34 | a : (...) => ... [captured sink39] : String | provenance | |
3534
| Capture.cs:55:27:58:17 | (...) => ... : (...) => ... [captured sink39] : String | Capture.cs:350:34:350:34 | a : (...) => ... [captured sink39] : String | provenance | |
@@ -40,7 +39,6 @@ edges
4039
| Capture.cs:81:13:81:13 | [post] access to local function M : M [captured sink31] : String | Capture.cs:83:9:83:19 | [post] access to local function CaptureOut2 : CaptureOut2 [captured sink31] : String | provenance | |
4140
| Capture.cs:83:9:83:19 | [post] access to local function CaptureOut2 : CaptureOut2 [captured sink31] : String | Capture.cs:84:15:84:20 | access to local variable sink31 | provenance | |
4241
| Capture.cs:89:22:89:35 | "taint source" : String | Capture.cs:92:30:92:40 | [post] access to local variable captureOut3 : (...) => ... [captured sink32] : String | provenance | |
43-
| Capture.cs:89:22:89:35 | "taint source" : String | Capture.cs:92:30:92:40 | [post] access to local variable captureOut3 : (...) => ... [captured sink32] : String | provenance | heuristic-callback |
4442
| Capture.cs:92:30:92:40 | [post] access to local variable captureOut3 : (...) => ... [captured sink32] : String | Capture.cs:93:15:93:20 | access to local variable sink32 | provenance | |
4543
| Capture.cs:114:23:117:13 | [post] (...) => ... : (...) => ... [captured sink40] : String | Capture.cs:123:9:123:33 | [post] access to local function CaptureOutMultipleLambdas : CaptureOutMultipleLambdas [captured sink40] : String | provenance | |
4644
| Capture.cs:116:26:116:39 | "taint source" : String | Capture.cs:352:9:352:9 | [post] access to parameter a : (...) => ... [captured sink40] : String | provenance | |
@@ -62,10 +60,8 @@ edges
6260
| Capture.cs:155:30:155:44 | [post] access to local variable captureThrough3 : (...) => ... [captured sink35] : String | Capture.cs:156:15:156:20 | access to local variable sink35 | provenance | |
6361
| Capture.cs:155:30:155:44 | access to local variable captureThrough3 : Func<String,String> [captured tainted] : String | Capture.cs:152:22:152:28 | access to parameter tainted : String | provenance | MaD:3 |
6462
| Capture.cs:155:30:155:44 | access to local variable captureThrough3 : Func<String,String> [captured tainted] : String | Capture.cs:152:22:152:28 | access to parameter tainted : String | provenance | MaD:4 |
65-
| Capture.cs:155:30:155:44 | access to local variable captureThrough3 : Func<String,String> [captured tainted] : String | Capture.cs:152:22:152:28 | access to parameter tainted : String | provenance | heuristic-callback |
6663
| Capture.cs:155:30:155:44 | access to local variable captureThrough3 : Func<String,String> [captured tainted] : String | Capture.cs:155:30:155:44 | [post] access to local variable captureThrough3 : (...) => ... [captured sink35] : String | provenance | MaD:3 |
6764
| Capture.cs:155:30:155:44 | access to local variable captureThrough3 : Func<String,String> [captured tainted] : String | Capture.cs:155:30:155:44 | [post] access to local variable captureThrough3 : (...) => ... [captured sink35] : String | provenance | MaD:4 |
68-
| Capture.cs:155:30:155:44 | access to local variable captureThrough3 : Func<String,String> [captured tainted] : String | Capture.cs:155:30:155:44 | [post] access to local variable captureThrough3 : (...) => ... [captured sink35] : String | provenance | heuristic-callback |
6965
| Capture.cs:162:13:162:18 | access to local variable sink36 : String | Capture.cs:163:15:163:20 | access to local variable sink36 | provenance | |
7066
| Capture.cs:162:22:162:36 | access to local function CaptureThrough4 : CaptureThrough4 [captured tainted] : String | Capture.cs:160:20:160:26 | access to parameter tainted : String | provenance | |
7167
| Capture.cs:162:22:162:36 | access to local function CaptureThrough4 : CaptureThrough4 [captured tainted] : String | Capture.cs:162:22:162:38 | call to local function CaptureThrough4 : String | provenance | |
@@ -94,10 +90,8 @@ edges
9490
| Capture.cs:228:17:228:30 | "taint source" : String | Capture.cs:229:20:233:9 | (...) => ... : (...) => ... [captured x] : String | provenance | |
9591
| Capture.cs:228:17:228:30 | "taint source" : String | Capture.cs:234:15:234:15 | access to local variable x | provenance | |
9692
| Capture.cs:229:20:233:9 | (...) => ... : (...) => ... [captured x] : String | Capture.cs:231:19:231:19 | access to local variable x | provenance | MaD:1 |
97-
| Capture.cs:229:20:233:9 | (...) => ... : (...) => ... [captured x] : String | Capture.cs:231:19:231:19 | access to local variable x | provenance | heuristic-callback |
9893
| Capture.cs:229:20:233:9 | [post] (...) => ... : (...) => ... [captured x] : String | Capture.cs:234:15:234:15 | access to local variable x | provenance | |
9994
| Capture.cs:232:17:232:30 | "taint source" : String | Capture.cs:229:20:233:9 | [post] (...) => ... : (...) => ... [captured x] : String | provenance | |
100-
| Capture.cs:232:17:232:30 | "taint source" : String | Capture.cs:229:20:233:9 | [post] (...) => ... : (...) => ... [captured x] : String | provenance | heuristic-callback |
10195
| Capture.cs:242:9:242:9 | [post] access to local variable c : Capture [field Field] : String | Capture.cs:249:9:249:9 | access to local variable a : Action [captured c, field Field] : String | provenance | |
10296
| Capture.cs:242:9:242:9 | [post] access to local variable c : Capture [field Field] : String | Capture.cs:251:15:251:15 | access to local variable c : Capture [field Field] : String | provenance | |
10397
| Capture.cs:242:19:242:32 | "taint source" : String | Capture.cs:242:9:242:9 | [post] access to local variable c : Capture [field Field] : String | provenance | |

0 commit comments

Comments
 (0)