Skip to content

Commit 91806da

Browse files
hvitvedhvitved
committed
C#: Address review comments
1 parent 5d1a592 commit 91806da

File tree

6 files changed

+33
-13
lines changed

6 files changed

+33
-13
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ private import semmle.code.csharp.frameworks.system.Collections
99
private import semmle.code.csharp.frameworks.system.collections.Generic
1010

1111
/**
12-
* Gets a source declaration of callable `c` that has a body.
12+
* Gets a source declaration of callable `c` that has a body or has
13+
* a flow summary.
14+
*
1315
* If the callable has both CIL and source code, return only the source
1416
* code version.
1517
*/
@@ -141,7 +143,7 @@ private module DispatchImpl {
141143
* call is a delegate call, or if the qualifier accesses a parameter of
142144
* the enclosing callable `c` (including the implicit `this` parameter).
143145
*/
144-
predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) {
146+
predicate mayBenefitFromCallContext(DataFlowCall call, Callable c) {
145147
c = call.getEnclosingCallable() and
146148
(
147149
exists(CallContext cc | exists(viableDelegateCallable(call, cc)) |

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,21 +1715,21 @@ module Summaries {
17151715
exists(
17161716
SourceDeclarationCallable sdc, CallableFlowSource source, AccessPath sourceAp,
17171717
CallableFlowSink sink, AccessPath sinkAp, boolean preservesValue,
1718-
SummaryInternalNodeState predState, AccessPath ap
1718+
SummaryInternalNodeState predState, AccessPath predAp
17191719
|
1720-
predState = TSummaryInternalNodeBeforeStoreState(ap) and
1720+
predState = TSummaryInternalNodeBeforeStoreState(predAp) and
17211721
pred = TSummaryInternalNode(sdc, source, sourceAp, sink, sinkAp, preservesValue, predState) and
1722-
c = ap.getHead()
1722+
c = predAp.getHead()
17231723
|
17241724
// More stores needed
17251725
exists(SummaryInternalNodeState succState |
17261726
succState =
1727-
TSummaryInternalNodeBeforeStoreState(any(AccessPath succAp | succAp.getTail() = ap)) and
1727+
TSummaryInternalNodeBeforeStoreState(any(AccessPath succAp | succAp.getTail() = predAp)) and
17281728
succ = TSummaryInternalNode(sdc, source, sourceAp, sink, sinkAp, preservesValue, succState)
17291729
)
17301730
or
17311731
// Last store
1732-
ap = sinkAp and
1732+
predAp = sinkAp and
17331733
succ = getSinkNode(sdc, sink)
17341734
)
17351735
}
@@ -1742,20 +1742,20 @@ module Summaries {
17421742
exists(
17431743
SourceDeclarationCallable sdc, CallableFlowSource source, AccessPath sourceAp,
17441744
CallableFlowSink sink, AccessPath sinkAp, boolean preservesValue,
1745-
SummaryInternalNodeState succState, AccessPath ap
1745+
SummaryInternalNodeState succState, AccessPath succAp
17461746
|
1747-
succState = TSummaryInternalNodeAfterReadState(ap) and
1747+
succState = TSummaryInternalNodeAfterReadState(succAp) and
17481748
succ = TSummaryInternalNode(sdc, source, sourceAp, sink, sinkAp, preservesValue, succState) and
1749-
c = ap.getHead()
1749+
c = succAp.getHead()
17501750
|
17511751
// First read
1752-
ap = sourceAp and
1752+
succAp = sourceAp and
17531753
pred = getSourceNode(sdc, source)
17541754
or
17551755
// Subsequent reads
17561756
exists(SummaryInternalNodeState predState, AccessPath predAp |
17571757
predState = TSummaryInternalNodeAfterReadState(predAp) and
1758-
predAp.getTail() = ap and
1758+
predAp.getTail() = succAp and
17591759
pred = TSummaryInternalNode(sdc, source, sourceAp, sink, sinkAp, preservesValue, predState)
17601760
)
17611761
)

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,9 @@
189189
| GlobalDataFlow.cs:455:9:455:18 | call to method Clear | return | GlobalDataFlow.cs:455:9:455:18 | call to method Clear |
190190
| GlobalDataFlow.cs:456:23:456:35 | call to method ToString | qualifier | GlobalDataFlow.cs:456:23:456:24 | [post] access to local variable sb |
191191
| GlobalDataFlow.cs:456:23:456:35 | call to method ToString | return | GlobalDataFlow.cs:456:23:456:35 | call to method ToString |
192-
| GlobalDataFlow.cs:468:44:468:47 | delegate call | return | GlobalDataFlow.cs:468:44:468:47 | delegate call |
192+
| GlobalDataFlow.cs:462:22:462:65 | call to method Join | return | GlobalDataFlow.cs:462:22:462:65 | call to method Join |
193+
| GlobalDataFlow.cs:465:23:465:65 | call to method Join | return | GlobalDataFlow.cs:465:23:465:65 | call to method Join |
194+
| GlobalDataFlow.cs:477:44:477:47 | delegate call | return | GlobalDataFlow.cs:477:44:477:47 | delegate call |
193195
| Splitting.cs:8:17:8:31 | [b (line 3): false] call to method Return | return | Splitting.cs:8:17:8:31 | [b (line 3): false] call to method Return |
194196
| Splitting.cs:8:17:8:31 | [b (line 3): true] call to method Return | return | Splitting.cs:8:17:8:31 | [b (line 3): true] call to method Return |
195197
| Splitting.cs:20:22:20:30 | call to method Return | return | Splitting.cs:20:22:20:30 | call to method Return |

‎csharp/ql/test/library-tests/dataflow/global/GlobalDataFlow.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,15 @@ void TestStringBuilderFlow()
456456
var nonSink = sb.ToString();
457457
Check(nonSink);
458458
}
459+
460+
void TestStringFlow()
461+
{
462+
var sink44 = string.Join(",", "whatever", "taint source");
463+
Check(sink44);
464+
465+
var nonSink = string.Join(",", "whatever", "not tainted");
466+
Check(nonSink);
467+
}
459468
}
460469

461470
static class IEnumerableExtensions

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
| GlobalDataFlow.cs:401:15:401:20 | access to local variable sink11 |
5757
| GlobalDataFlow.cs:424:41:424:46 | access to local variable sink20 |
5858
| GlobalDataFlow.cs:453:15:453:20 | access to local variable sink43 |
59+
| GlobalDataFlow.cs:463:15:463:20 | access to local variable sink44 |
5960
| Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x |
6061
| Splitting.cs:9:15:9:15 | [b (line 3): true] access to local variable x |
6162
| Splitting.cs:11:19:11:19 | access to local variable x |

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,8 @@ edges
239239
| GlobalDataFlow.cs:451:35:451:48 | "taint source" : String | GlobalDataFlow.cs:451:31:451:32 | [post] access to local variable sb [[]] : String |
240240
| GlobalDataFlow.cs:452:22:452:23 | access to local variable sb [[]] : String | GlobalDataFlow.cs:452:22:452:34 | call to method ToString : String |
241241
| GlobalDataFlow.cs:452:22:452:34 | call to method ToString : String | GlobalDataFlow.cs:453:15:453:20 | access to local variable sink43 |
242+
| GlobalDataFlow.cs:462:22:462:65 | call to method Join : String | GlobalDataFlow.cs:463:15:463:20 | access to local variable sink44 |
243+
| GlobalDataFlow.cs:462:51:462:64 | "taint source" : String | GlobalDataFlow.cs:462:22:462:65 | call to method Join : String |
242244
| Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:8:24:8:30 | [b (line 3): false] access to parameter tainted : String |
243245
| Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:8:24:8:30 | [b (line 3): true] access to parameter tainted : String |
244246
| Splitting.cs:8:17:8:31 | [b (line 3): false] call to method Return : String | Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x |
@@ -465,6 +467,9 @@ nodes
465467
| GlobalDataFlow.cs:452:22:452:23 | access to local variable sb [[]] : String | semmle.label | access to local variable sb [[]] : String |
466468
| GlobalDataFlow.cs:452:22:452:34 | call to method ToString : String | semmle.label | call to method ToString : String |
467469
| GlobalDataFlow.cs:453:15:453:20 | access to local variable sink43 | semmle.label | access to local variable sink43 |
470+
| GlobalDataFlow.cs:462:22:462:65 | call to method Join : String | semmle.label | call to method Join : String |
471+
| GlobalDataFlow.cs:462:51:462:64 | "taint source" : String | semmle.label | "taint source" : String |
472+
| GlobalDataFlow.cs:463:15:463:20 | access to local variable sink44 | semmle.label | access to local variable sink44 |
468473
| Splitting.cs:3:28:3:34 | tainted : String | semmle.label | tainted : String |
469474
| Splitting.cs:8:17:8:31 | [b (line 3): false] call to method Return : String | semmle.label | [b (line 3): false] call to method Return : String |
470475
| Splitting.cs:8:17:8:31 | [b (line 3): true] call to method Return : String | semmle.label | [b (line 3): true] call to method Return : String |
@@ -551,6 +556,7 @@ nodes
551556
| GlobalDataFlow.cs:401:15:401:20 | access to local variable sink11 | GlobalDataFlow.cs:398:39:398:45 | tainted : String | GlobalDataFlow.cs:401:15:401:20 | access to local variable sink11 | access to local variable sink11 |
552557
| GlobalDataFlow.cs:424:41:424:46 | access to local variable sink20 | GlobalDataFlow.cs:18:27:18:40 | "taint source" : String | GlobalDataFlow.cs:424:41:424:46 | access to local variable sink20 | access to local variable sink20 |
553558
| GlobalDataFlow.cs:453:15:453:20 | access to local variable sink43 | GlobalDataFlow.cs:451:35:451:48 | "taint source" : String | GlobalDataFlow.cs:453:15:453:20 | access to local variable sink43 | access to local variable sink43 |
559+
| GlobalDataFlow.cs:463:15:463:20 | access to local variable sink44 | GlobalDataFlow.cs:462:51:462:64 | "taint source" : String | GlobalDataFlow.cs:463:15:463:20 | access to local variable sink44 | access to local variable sink44 |
554560
| Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x | Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:9:15:9:15 | [b (line 3): false] access to local variable x | [b (line 3): false] access to local variable x |
555561
| Splitting.cs:9:15:9:15 | [b (line 3): true] access to local variable x | Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:9:15:9:15 | [b (line 3): true] access to local variable x | [b (line 3): true] access to local variable x |
556562
| Splitting.cs:11:19:11:19 | access to local variable x | Splitting.cs:3:28:3:34 | tainted : String | Splitting.cs:11:19:11:19 | access to local variable x | access to local variable x |

0 commit comments

Comments
 (0)