Merge pull request github#5325 from ihsinme/ihsinme-patch-245

CPP: Add query for CWE-783 Operator Precedence Logic Error When Use Bool Type

Author: rdmarsh2

cpp/ql/src/experimental/Security/CWE/CWE-783/OperatorPrecedenceLogicErrorWhenUseBoolType.c

@@ -0,0 +1,11 @@
+if(len=funcReadData()==0) return 1; // BAD: variable `len` will not equal the value returned by function `funcReadData()`
+...
+if((len=funcReadData())==0) return 1; // GOOD: variable `len` equal the value returned by function `funcReadData()`
+...
+bool a=true;
+a++;// BAD: variable `a` does not change its meaning
+bool b;
+b=-a;// BAD: variable `b` equal `true`
+...
+a=false;// GOOD: variable `a` equal `false`
+b=!a;// GOOD: variable `b` equal `false`

cpp/ql/src/experimental/Security/CWE/CWE-783/OperatorPrecedenceLogicErrorWhenUseBoolType.qhelp

@@ -0,0 +1,28 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Finding places of confusing use of boolean type. For example, a unary minus does not work before a boolean type and an increment always gives true.</p>
+
+
+</overview>
+<recommendation>
+
+<p>we recommend making the code simpler.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates erroneous and fixed methods for using a boolean data type.</p>
+<sample src="OperatorPrecedenceLogicErrorWhenUseBoolType.c" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/EXP00-C.+Use+parentheses+for+precedence+of+operation">EXP00-C. Use parentheses for precedence of operation</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-783/OperatorPrecedenceLogicErrorWhenUseBoolType.ql

@@ -0,0 +1,54 @@
+/**
+ * @name Operator Precedence Logic Error When Use Bool Type
+ * @description --Finding places of confusing use of boolean type.
+ *              --For example, a unary minus does not work before a boolean type and an increment always gives true.
+ * @kind problem
+ * @id cpp/operator-precedence-logic-error-when-use-bool-type
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-783
+ *       external/cwe/cwe-480
+ */
+
+import cpp
+import semmle.code.cpp.valuenumbering.HashCons
+
+/** Holds if `exp` increments a boolean value. */
+predicate incrementBoolType(IncrementOperation exp) {
+  exp.getOperand().getType() instanceof BoolType
+}
+
+/** Holds if `exp` applies the unary minus operator to a boolean type. */
+predicate revertSignBoolType(UnaryMinusExpr exp) {
+  exp.getAnOperand().getType() instanceof BoolType and
+  exp.getFullyConverted().getType() instanceof BoolType
+}
+
+/** Holds, if this is an expression, uses comparison and assignment outside of execution precedence. */
+predicate assignBoolType(Expr exp) {
+  exists(ComparisonOperation co |
+    exp.(AssignExpr).getRValue() = co and
+    exp.isCondition() and
+    not co.isParenthesised() and
+    not exp.(AssignExpr).getLValue().getType() instanceof BoolType and
+    not exists(Expr exbl |
+      hashCons(exbl.(AssignExpr).getLValue()) = hashCons(exp.(AssignExpr).getLValue()) and
+      not exbl.isCondition() and
+      exbl.(AssignExpr).getRValue().getType() instanceof BoolType and
+      exbl.(AssignExpr).getLValue().getType() = exp.(AssignExpr).getLValue().getType()
+    ) and
+    co.getLeftOperand() instanceof FunctionCall and
+    not co.getRightOperand().getType() instanceof BoolType and
+    not co.getRightOperand().getValue() = "0" and
+    not co.getRightOperand().getValue() = "1"
+  )
+}
+
+from Expr exp
+where
+  incrementBoolType(exp) or
+  revertSignBoolType(exp) or
+  assignBoolType(exp)
+select exp, "this expression needs attention"

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-788/semmle/tests/OperatorPrecedenceLogicErrorWhenUseBoolType.expected

@@ -0,0 +1,5 @@
+| test.cpp:10:8:10:10 | - ... | this expression needs attention |
+| test.cpp:12:3:12:6 | ... ++ | this expression needs attention |
+| test.cpp:13:3:13:6 | ++ ... | this expression needs attention |
+| test.cpp:14:6:14:21 | ... = ... | this expression needs attention |
+| test.cpp:16:6:16:21 | ... = ... | this expression needs attention |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-788/semmle/tests/OperatorPrecedenceLogicErrorWhenUseBoolType.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-783/OperatorPrecedenceLogicErrorWhenUseBoolType.ql

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

@@ -0,0 +1,26 @@
+int tmpFunc()
+{
+  return 12;
+}
+void testFunction()
+{
+  int i1,i2,i3;
+  bool b1,b2,b3;
+  char c1,c2,c3;
+  b1 = -b2; //BAD
+  b1 = !b2; //GOOD
+  b1++; //BAD
+  ++b1; //BAD
+  if(i1=tmpFunc()!=i2) //BAD
+    return;
+  if(i1=tmpFunc()!=11) //BAD
+    return;
+  if((i1=tmpFunc())!=i2) //GOOD
+    return;
+  if((i1=tmpFunc())!=11) //GOOD
+    return;
+  if(i1=tmpFunc()!=1) //GOOD
+    return;
+  if(i1=tmpFunc()==b1) //GOOD
+    return;
+}