CPP: Add a query to find incorrectly used switch

cpp/ql/src/experimental/Security/CWE/CWE-561/FindIncorrectlyUsedSwitch.c

@@ -0,0 +1,34 @@
+...
+  int i1;
+  char c1;
+...
+  if((c1<50)&&(c>10))
+  switch(c1){
+    case 300: // BAD: the code will not be executed
+...  
+  if((i1<5)&&(i1>0))
+  switch(i1){ // BAD
+    case 21: // BAD: the code will not be executed
+...
+  switch(c1){
+...
+  dafault: // BAD: maybe it will be right `default`
+...
+  }
+
+...
+  switch(c1){ 
+      i1=c1*2; // BAD: the code will not be executed
+    case 12:
+...
+  switch(c1){ // GOOD
+    case 12:
+      break;
+    case 10:
+      break;
+    case 9:
+      break;
+    default:
+      break;
+  }
+...

cpp/ql/src/experimental/Security/CWE/CWE-561/FindIncorrectlyUsedSwitch.qhelp

@@ -0,0 +1,24 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>A mismatch between conditionals and <code>switch</code> cases can lead to control-flow violations (CWE-691) where the developer either does not handle all combinations of conditions or unintentionally created dead code (CWE-561).</p>
+
+
+</overview>
+
+<example>
+<p>The following example demonstrates fallacious and fixed ways of using a <code>switch</code> statement.</p>
+<sample src="FindIncorrectlyUsedSwitch.c" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/MSC12-C.+Detect+and+remove+code+that+has+no+effect+or+is+never+executed">MSC12-C. Detect and remove code that has no effect or is never executed</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-561/FindIncorrectlyUsedSwitch.ql

@@ -0,0 +1,153 @@
+/**
+ * @name Incorrect switch statement
+ * @description --Finding places the dangerous use of a switch.
+ *              --For example, when the range of values for a condition does not cover all of the selection values..
+ * @kind problem
+ * @id cpp/operator-find-incorrectly-used-switch
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-561
+ *       external/cwe/cwe-691
+ *       external/cwe/cwe-478
+ */
+
+import cpp
+import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+
+/** Holds if the range contains no boundary values. */
+predicate isRealRange(Expr exp) {
+  upperBound(exp).toString() != "18446744073709551616" and
+  upperBound(exp).toString() != "9223372036854775807" and
+  upperBound(exp).toString() != "4294967295" and
+  upperBound(exp).toString() != "Infinity" and
+  upperBound(exp).toString() != "NaN" and
+  lowerBound(exp).toString() != "-9223372036854775808" and
+  lowerBound(exp).toString() != "-4294967296" and
+  lowerBound(exp).toString() != "-Infinity" and
+  lowerBound(exp).toString() != "NaN" and
+  upperBound(exp) != 2147483647 and
+  upperBound(exp) != 268435455 and
+  upperBound(exp) != 33554431 and
+  upperBound(exp) != 8388607 and
+  upperBound(exp) != 65535 and
+  upperBound(exp) != 32767 and
+  upperBound(exp) != 255 and
+  upperBound(exp) != 127 and
+  upperBound(exp) != 63 and
+  upperBound(exp) != 31 and
+  upperBound(exp) != 15 and
+  upperBound(exp) != 7 and
+  lowerBound(exp) != -2147483648 and
+  lowerBound(exp) != -268435456 and
+  lowerBound(exp) != -33554432 and
+  lowerBound(exp) != -8388608 and
+  lowerBound(exp) != -65536 and
+  lowerBound(exp) != -32768 and
+  lowerBound(exp) != -128
+}
+
+/** Holds if the range of values for the condition is less than the choices. */
+predicate isNotAllSelected(SwitchStmt swtmp) {
+  not swtmp.getExpr().isConstant() and
+  exists(int i |
+    i != 0 and
+    (
+      i = lowerBound(swtmp.getASwitchCase().getExpr()) and
+      upperBound(swtmp.getExpr()) < i
+      or
+      (
+        i = upperBound(swtmp.getASwitchCase().getExpr()) or
+        i = upperBound(swtmp.getASwitchCase().getEndExpr())
+      ) and
+      lowerBound(swtmp.getExpr()) > i
+    )
+  )
+}
+
+/** Holds if the range of values for the condition is greater than the selection. */
+predicate isConditionBig(SwitchStmt swtmp) {
+  not swtmp.hasDefaultCase() and
+  not exists(int iu, int il |
+    (
+      iu = upperBound(swtmp.getASwitchCase().getExpr()) or
+      iu = upperBound(swtmp.getASwitchCase().getEndExpr())
+    ) and
+    upperBound(swtmp.getExpr()) = iu and
+    (
+      il = lowerBound(swtmp.getASwitchCase().getExpr()) or
+      il = lowerBound(swtmp.getASwitchCase().getEndExpr())
+    ) and
+    lowerBound(swtmp.getExpr()) = il
+  )
+}
+
+/** Holds if there are labels inside the block with names similar to `default` or `case`. */
+predicate isWrongLableName(SwitchStmt swtmp) {
+  not swtmp.hasDefaultCase() and
+  exists(LabelStmt lb |
+    (
+      (
+        lb.getName().charAt(0) = "d" or
+        lb.getName().charAt(0) = "c"
+      ) and
+      (
+        lb.getName().charAt(1) = "e" or
+        lb.getName().charAt(1) = "a"
+      ) and
+      (
+        lb.getName().charAt(2) = "f" or
+        lb.getName().charAt(2) = "s"
+      )
+    ) and
+    lb.getEnclosingStmt().getParentStmt*() = swtmp.getStmt() and
+    not exists(GotoStmt gs | gs.getName() = lb.getName())
+  )
+}
+
+/** Holds if the block contains code before the first `case`. */
+predicate isCodeBeforeCase(SwitchStmt swtmp) {
+  exists(Expr exp |
+    exp.getEnclosingStmt().getParentStmt*() = swtmp.getStmt() and
+    not exists(Loop lp |
+      exp.getEnclosingStmt().getParentStmt*() = lp and
+      lp.getEnclosingStmt().getParentStmt*() = swtmp.getStmt()
+    ) and
+    not exists(Stmt sttmp, SwitchCase sctmp |
+      sttmp = swtmp.getASwitchCase().getAStmt() and
+      sctmp = swtmp.getASwitchCase() and
+      (
+        exp.getEnclosingStmt().getParentStmt*() = sttmp or
+        exp.getEnclosingStmt() = sctmp
+      )
+    )
+  )
+}
+
+from SwitchStmt sw, string msg
+where
+  isRealRange(sw.getExpr()) and
+  lowerBound(sw.getExpr()) != upperBound(sw.getExpr()) and
+  lowerBound(sw.getExpr()) != 0 and
+  not exists(Expr cexp |
+    cexp = sw.getASwitchCase().getExpr() and not isRealRange(cexp)
+    or
+    cexp = sw.getASwitchCase().getEndExpr() and not isRealRange(cexp)
+  ) and
+  not exists(Expr exptmp |
+    exptmp = sw.getExpr().getAChild*() and
+    not exptmp.isConstant() and
+    not isRealRange(exptmp)
+  ) and
+  (sw.getASwitchCase().terminatesInBreakStmt() or sw.getASwitchCase().terminatesInReturnStmt()) and
+  (
+    isNotAllSelected(sw) and msg = "The range of condition values is less than the selection."
+    or
+    isConditionBig(sw) and msg = "The range of condition values is wider than the choices."
+  )
+  or
+  isWrongLableName(sw) and msg = "Possibly erroneous label name."
+  or
+  isCodeBeforeCase(sw) and msg = "Code before case will not be executed."
+select sw, msg

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-561/semmle/tests/FindIncorrectlyUsedSwitch.expected

@@ -0,0 +1,4 @@
+| test.c:20:3:28:3 | switch (...) ...  | Possibly erroneous label name. |
+| test.c:30:3:38:3 | switch (...) ...  | Code before case will not be executed. |
+| test.c:41:3:50:3 | switch (...) ...  | The range of condition values is less than the selection. |
+| test.c:53:3:58:3 | switch (...) ...  | The range of condition values is wider than the choices. |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-561/semmle/tests/FindIncorrectlyUsedSwitch.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-561/FindIncorrectlyUsedSwitch.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-561/semmle/tests/test.c

@@ -0,0 +1,60 @@
+void testFunction(char c1,int i1)
+{
+
+  switch(c1){ // GOOD
+    case 12:
+      break;
+    case 10:
+      break;
+    case 9:
+      break;
+  }
+
+  switch(i1){ // GOOD
+    for(i1=0;i1<20;i1++){
+      case 12:
+      case 10:
+      case 9:
+    }
+  }
+  switch(c1){ // BAD
+    case 12:
+      break;
+    case 10:
+      break;
+    case 9:
+      break;
+  dafault:
+  }
+
+  switch(c1){ // BAD
+      c1=c1*2;
+    case 12:
+      break;
+    case 10:
+      break;
+    case 9:
+      break;
+  }
+
+  if((c1<6)&&(c1>0))
+  switch(c1){ // BAD
+    case 8:
+      break;
+    case 5:
+      break;
+    case 3:
+      break;
+    case 1:
+      break;
+  }
+
+  if((c1<6)&&(c1>0))
+  switch(c1){ // BAD
+    case 3:
+      break;
+    case 1:
+      break;
+  }
+
+}