Merge pull request #4725 from hvitved/csharp/cfg/constant-condition-block

C#: Always create basic blocks for nodes with a conditional predecessor

Author: hvitved

csharp/ql/src/semmle/code/csharp/controlflow/BasicBlocks.qll

@@ -298,6 +298,19 @@ private module Internal {
     cfn.isJoin()
     or
     cfn.getAPredecessor().isBranch()
+    or
+    /*
+     * In cases such as
+     * ```csharp
+     * if (b)
+     *     M()
+     * ```
+     * where the `false` edge out of `b` is not present (because we can prove it
+     * impossible), we still split up the basic block in two, in order to generate
+     * a `ConditionBlock` which can be used by the guards library.
+     */
+
+    exists(cfn.getAPredecessorByType(any(ControlFlow::SuccessorTypes::ConditionalSuccessor s)))
   }
 
   /**

csharp/ql/src/semmle/code/csharp/controlflow/internal/PreBasicBlocks.qll

@@ -24,6 +24,8 @@ private predicate startsBB(ControlFlowElement cfe) {
   or
   strictcount(ControlFlowElement pred, Completion c | succ(pred, cfe, c)) > 1
   or
+  succ(_, cfe, any(ConditionalCompletion c))
+  or
   exists(ControlFlowElement pred, int i |
     succ(pred, cfe, _) and
     i = count(ControlFlowElement succ, Completion c | succ(pred, succ, c))
@@ -95,14 +97,11 @@ private Completion getConditionalCompletion(ConditionalCompletion cc) {
 
 class ConditionBlock extends PreBasicBlock {
   ConditionBlock() {
-    strictcount(Completion c |
-      c = getConditionalCompletion(_) and
-      (
-        succ(this.getLastElement(), _, c)
-        or
-        succExit(this.getLastElement(), _, c)
-      )
-    ) > 1
+    exists(Completion c | c = getConditionalCompletion(_) |
+      succ(this.getLastElement(), _, c)
+      or
+      succExit(this.getLastElement(), _, c)
+    )
   }
 
   private predicate immediatelyControls(PreBasicBlock succ, ConditionalCompletion cc) {

csharp/ql/src/semmle/code/csharp/security/dataflow/TaintedPath.qll

@@ -110,6 +110,8 @@ module TaintedPath {
       or
       // Checking against `null` has no bearing on path traversal.
       this.controlsNode(_, _, any(AbstractValues::NullValue nv))
+      or
+      this.(LogicalOperation).getAnOperand() instanceof WeakGuard
     }
   }
 

csharp/ql/test/library-tests/controlflow/graph/BasicBlock.expected

@@ -5,20 +5,28 @@
 | Assert.cs:53:27:53:27 | access to local variable s | Assert.cs:52:24:52:32 | ... == ... | Assert.cs:52:24:52:24 | access to local variable s | false |
 | Assert.cs:59:36:59:36 | access to parameter b | Assert.cs:58:20:58:20 | access to parameter b | Assert.cs:58:20:58:20 | access to parameter b | false |
 | Assert.cs:60:27:60:27 | access to local variable s | Assert.cs:59:23:59:31 | ... != ... | Assert.cs:59:23:59:23 | access to local variable s | true |
+| Assert.cs:60:27:60:27 | access to local variable s | Assert.cs:59:23:59:36 | ... && ... | Assert.cs:59:23:59:23 | access to local variable s | true |
 | Assert.cs:66:37:66:37 | access to parameter b | Assert.cs:65:20:65:20 | access to parameter b | Assert.cs:65:20:65:20 | access to parameter b | false |
 | Assert.cs:67:27:67:27 | access to local variable s | Assert.cs:66:24:66:32 | ... == ... | Assert.cs:66:24:66:24 | access to local variable s | false |
+| Assert.cs:67:27:67:27 | access to local variable s | Assert.cs:66:24:66:37 | ... \|\| ... | Assert.cs:66:24:66:24 | access to local variable s | false |
 | Assert.cs:73:36:73:36 | access to parameter b | Assert.cs:72:20:72:20 | access to parameter b | Assert.cs:72:20:72:20 | access to parameter b | true |
 | Assert.cs:74:27:74:27 | access to local variable s | Assert.cs:73:23:73:31 | ... == ... | Assert.cs:73:23:73:23 | access to local variable s | true |
+| Assert.cs:74:27:74:27 | access to local variable s | Assert.cs:73:23:73:36 | ... && ... | Assert.cs:73:23:73:23 | access to local variable s | true |
 | Assert.cs:80:37:80:37 | access to parameter b | Assert.cs:79:20:79:20 | access to parameter b | Assert.cs:79:20:79:20 | access to parameter b | true |
 | Assert.cs:81:27:81:27 | access to local variable s | Assert.cs:80:24:80:32 | ... != ... | Assert.cs:80:24:80:24 | access to local variable s | false |
+| Assert.cs:81:27:81:27 | access to local variable s | Assert.cs:80:24:80:37 | ... \|\| ... | Assert.cs:80:24:80:24 | access to local variable s | false |
 | Assert.cs:94:16:94:17 | access to parameter b1 | Assert.cs:93:25:93:26 | access to parameter b1 | Assert.cs:93:25:93:26 | access to parameter b1 | true |
 | Assert.cs:94:23:94:24 | access to parameter b2 | Assert.cs:93:29:93:30 | access to parameter b2 | Assert.cs:93:29:93:30 | access to parameter b2 | false |
 | Collections.cs:52:17:52:20 | access to parameter args | Collections.cs:50:13:50:27 | ... == ... | Collections.cs:50:13:50:16 | access to parameter args | false |
 | Collections.cs:53:9:53:12 | access to parameter args | Collections.cs:50:13:50:27 | ... == ... | Collections.cs:50:13:50:16 | access to parameter args | false |
 | Collections.cs:54:13:54:16 | access to parameter args | Collections.cs:50:13:50:27 | ... == ... | Collections.cs:50:13:50:16 | access to parameter args | false |
 | Collections.cs:67:13:67:13 | access to local variable x | Collections.cs:65:13:65:24 | ... == ... | Collections.cs:65:13:65:13 | access to local variable x | true |
 | Collections.cs:68:13:68:13 | access to local variable x | Collections.cs:65:13:65:24 | ... == ... | Collections.cs:65:13:65:13 | access to local variable x | true |
+| Guards.cs:12:13:12:13 | access to parameter s | Guards.cs:10:13:10:25 | !... | Guards.cs:10:16:10:16 | access to parameter s | false |
+| Guards.cs:12:13:12:13 | access to parameter s | Guards.cs:10:14:10:25 | !... | Guards.cs:10:16:10:16 | access to parameter s | true |
 | Guards.cs:12:13:12:13 | access to parameter s | Guards.cs:10:16:10:24 | ... == ... | Guards.cs:10:16:10:16 | access to parameter s | false |
+| Guards.cs:14:31:14:31 | access to parameter s | Guards.cs:10:13:10:25 | !... | Guards.cs:10:16:10:16 | access to parameter s | false |
+| Guards.cs:14:31:14:31 | access to parameter s | Guards.cs:10:14:10:25 | !... | Guards.cs:10:16:10:16 | access to parameter s | true |
 | Guards.cs:14:31:14:31 | access to parameter s | Guards.cs:10:16:10:24 | ... == ... | Guards.cs:10:16:10:16 | access to parameter s | false |
 | Guards.cs:14:31:14:31 | access to parameter s | Guards.cs:12:13:12:24 | ... > ... | Guards.cs:12:13:12:13 | access to parameter s | true |
 | Guards.cs:26:31:26:31 | access to parameter s | Guards.cs:24:13:24:21 | ... != ... | Guards.cs:24:13:24:13 | access to parameter s | true |
@@ -29,10 +37,24 @@
 | Guards.cs:33:35:33:35 | access to parameter y | Guards.cs:32:40:32:51 | !... | Guards.cs:32:42:32:42 | access to parameter y | true |
 | Guards.cs:33:35:33:35 | access to parameter y | Guards.cs:32:42:32:50 | ... == ... | Guards.cs:32:42:32:42 | access to parameter y | false |
 | Guards.cs:36:32:36:32 | access to parameter x | Guards.cs:35:13:35:21 | ... == ... | Guards.cs:35:13:35:13 | access to parameter x | false |
+| Guards.cs:36:32:36:32 | access to parameter x | Guards.cs:35:13:35:34 | ... \|\| ... | Guards.cs:35:13:35:13 | access to parameter x | false |
+| Guards.cs:36:36:36:36 | access to parameter y | Guards.cs:35:13:35:34 | ... \|\| ... | Guards.cs:35:26:35:26 | access to parameter y | false |
 | Guards.cs:36:36:36:36 | access to parameter y | Guards.cs:35:26:35:34 | ... == ... | Guards.cs:35:26:35:26 | access to parameter y | false |
+| Guards.cs:39:31:39:31 | access to parameter x | Guards.cs:38:13:38:37 | !... | Guards.cs:38:15:38:15 | access to parameter x | true |
 | Guards.cs:39:31:39:31 | access to parameter x | Guards.cs:38:15:38:23 | ... == ... | Guards.cs:38:15:38:15 | access to parameter x | false |
+| Guards.cs:39:31:39:31 | access to parameter x | Guards.cs:38:15:38:36 | ... \|\| ... | Guards.cs:38:15:38:15 | access to parameter x | false |
+| Guards.cs:39:35:39:35 | access to parameter y | Guards.cs:38:13:38:37 | !... | Guards.cs:38:28:38:28 | access to parameter y | true |
+| Guards.cs:39:35:39:35 | access to parameter y | Guards.cs:38:15:38:36 | ... \|\| ... | Guards.cs:38:28:38:28 | access to parameter y | false |
 | Guards.cs:39:35:39:35 | access to parameter y | Guards.cs:38:28:38:36 | ... == ... | Guards.cs:38:28:38:28 | access to parameter y | false |
+| Guards.cs:42:32:42:32 | access to parameter x | Guards.cs:41:13:41:39 | !... | Guards.cs:41:17:41:17 | access to parameter x | false |
+| Guards.cs:42:32:42:32 | access to parameter x | Guards.cs:41:14:41:39 | !... | Guards.cs:41:17:41:17 | access to parameter x | true |
+| Guards.cs:42:32:42:32 | access to parameter x | Guards.cs:41:15:41:39 | !... | Guards.cs:41:17:41:17 | access to parameter x | false |
 | Guards.cs:42:32:42:32 | access to parameter x | Guards.cs:41:17:41:25 | ... != ... | Guards.cs:41:17:41:17 | access to parameter x | true |
+| Guards.cs:42:32:42:32 | access to parameter x | Guards.cs:41:17:41:38 | ... && ... | Guards.cs:41:17:41:17 | access to parameter x | true |
+| Guards.cs:42:36:42:36 | access to parameter y | Guards.cs:41:13:41:39 | !... | Guards.cs:41:30:41:30 | access to parameter y | false |
+| Guards.cs:42:36:42:36 | access to parameter y | Guards.cs:41:14:41:39 | !... | Guards.cs:41:30:41:30 | access to parameter y | true |
+| Guards.cs:42:36:42:36 | access to parameter y | Guards.cs:41:15:41:39 | !... | Guards.cs:41:30:41:30 | access to parameter y | false |
+| Guards.cs:42:36:42:36 | access to parameter y | Guards.cs:41:17:41:38 | ... && ... | Guards.cs:41:30:41:30 | access to parameter y | true |
 | Guards.cs:42:36:42:36 | access to parameter y | Guards.cs:41:30:41:38 | ... != ... | Guards.cs:41:30:41:30 | access to parameter y | true |
 | Guards.cs:48:31:48:40 | access to field Field | Guards.cs:47:13:47:25 | ... != ... | Guards.cs:47:13:47:17 | access to field Field | true |
 | Guards.cs:55:27:55:27 | access to parameter g | Guards.cs:53:13:53:27 | ... == ... | Guards.cs:53:13:53:13 | access to parameter g | false |
@@ -63,18 +85,24 @@
 | Guards.cs:138:20:138:20 | access to parameter s | Guards.cs:137:13:137:25 | ... is ... | Guards.cs:137:13:137:13 | access to parameter s | true |
 | Guards.cs:139:16:139:16 | access to parameter s | Guards.cs:137:13:137:25 | ... is ... | Guards.cs:137:13:137:13 | access to parameter s | false |
 | Guards.cs:146:16:146:16 | access to parameter o | Guards.cs:144:13:144:25 | ... is ... | Guards.cs:144:13:144:13 | access to parameter o | false |
+| Guards.cs:169:31:169:31 | access to parameter x | Guards.cs:168:13:168:41 | !... | Guards.cs:168:40:168:40 | access to parameter x | true |
 | Guards.cs:169:31:169:31 | access to parameter x | Guards.cs:168:14:168:41 | call to method IsNullOrWhiteSpace | Guards.cs:168:40:168:40 | access to parameter x | false |
+| Guards.cs:190:31:190:31 | access to parameter s | Guards.cs:189:13:189:25 | !... | Guards.cs:189:24:189:24 | access to parameter s | true |
 | Guards.cs:190:31:190:31 | access to parameter s | Guards.cs:189:14:189:25 | call to method NullTest1 | Guards.cs:189:24:189:24 | access to parameter s | false |
+| Guards.cs:192:31:192:31 | access to parameter s | Guards.cs:191:13:191:25 | !... | Guards.cs:191:24:191:24 | access to parameter s | true |
 | Guards.cs:192:31:192:31 | access to parameter s | Guards.cs:191:14:191:25 | call to method NullTest2 | Guards.cs:191:24:191:24 | access to parameter s | false |
+| Guards.cs:194:31:194:31 | access to parameter s | Guards.cs:193:13:193:25 | !... | Guards.cs:193:24:193:24 | access to parameter s | true |
 | Guards.cs:194:31:194:31 | access to parameter s | Guards.cs:193:14:193:25 | call to method NullTest3 | Guards.cs:193:24:193:24 | access to parameter s | false |
 | Guards.cs:196:31:196:31 | access to parameter s | Guards.cs:195:13:195:27 | call to method NotNullTest4 | Guards.cs:195:26:195:26 | access to parameter s | true |
+| Guards.cs:198:31:198:31 | access to parameter s | Guards.cs:197:13:197:29 | !... | Guards.cs:197:28:197:28 | access to parameter s | true |
 | Guards.cs:198:31:198:31 | access to parameter s | Guards.cs:197:14:197:29 | call to method NullTestWrong | Guards.cs:197:28:197:28 | access to parameter s | false |
 | Guards.cs:205:13:205:13 | access to parameter o | Guards.cs:203:13:203:21 | ... != ... | Guards.cs:203:13:203:13 | access to parameter o | true |
 | Guards.cs:208:17:208:17 | access to parameter o | Guards.cs:203:13:203:21 | ... != ... | Guards.cs:203:13:203:13 | access to parameter o | true |
 | Guards.cs:269:13:269:14 | access to parameter o1 | Guards.cs:268:13:268:41 | call to operator == | Guards.cs:268:13:268:14 | access to parameter o1 | true |
 | Guards.cs:271:13:271:14 | access to parameter o1 | Guards.cs:270:13:270:42 | call to operator == | Guards.cs:270:13:270:14 | access to parameter o1 | true |
 | Guards.cs:342:27:342:27 | access to parameter b | Guards.cs:341:20:341:20 | access to parameter b | Guards.cs:341:20:341:20 | access to parameter b | false |
 | Guards.cs:343:31:343:31 | access to local variable s | Guards.cs:342:13:342:21 | ... != ... | Guards.cs:342:13:342:13 | access to local variable s | true |
+| Guards.cs:343:31:343:31 | access to local variable s | Guards.cs:342:13:342:27 | ... && ... | Guards.cs:342:13:342:13 | access to local variable s | true |
 | Splitting.cs:13:17:13:17 | access to parameter o | Splitting.cs:12:17:12:25 | ... != ... | Splitting.cs:12:17:12:17 | access to parameter o | true |
 | Splitting.cs:23:24:23:24 | access to parameter o | Splitting.cs:22:17:22:25 | ... != ... | Splitting.cs:22:17:22:17 | access to parameter o | true |
 | Splitting.cs:25:13:25:13 | access to parameter o | Splitting.cs:22:17:22:25 | ... != ... | Splitting.cs:22:17:22:17 | access to parameter o | false |