Merge pull request #416 from raulgarciamsft/users/raulga/c6317

cpp: Incorrect not operator usage

cpp/ql/src/Likely Bugs/Likely Typos/IncorrectNotOperatorUsage.cpp

@@ -0,0 +1,20 @@
+#define FLAGS   0x4004
+
+void f_warning(int i)
+{
+    // The usage of the logical not operator in this case is unlikely to be correct
+    // as the output is being used as an operator for a bit-wise and operation
+    if (i & !FLAGS) 
+    {
+        // code
+    }
+}
+
+
+void f_fixed(int i)
+{
+    if (i & ~FLAGS) // Changing the logical not operator for the bit-wise not operator would fix this logic
+    {
+        // code
+    }
+}

cpp/ql/src/Likely Bugs/Likely Typos/IncorrectNotOperatorUsage.qhelp

@@ -0,0 +1,26 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>This rule finds logical-not operator usage as an operator for in a bit-wise operation.</p>
+
+<p>Due to the nature of logical operation result value, only the lowest bit could possibly be set, and it is unlikely to be intent in bitwise opeartions. Violations are often indicative of a typo, using a logical-not (<code>!</code>) opeartor instead of the bit-wise not (<code>~</code>) operator. </p>
+<p>This rule is restricted to analyze bit-wise and (<code>&amp;</code>) and bit-wise or (<code>|</code>) operation in order to provide better precision.</p>
+<p>This rule ignores instances where a double negation (<code>!!</code>) is explicitly used as the opeartor of the bitwise operation, as this is a commonly used as a mechanism to normalize an integer value to either 1 or 0.</p>
+<p>NOTE: It is not recommended to use this rule in kernel code or older C code as it will likely find several false positive instances.</p>
+
+</overview>
+<recommendation>
+<p>Carefully inspect the flagged expressions. Consider the intent in the code logic, and decide whether it is necessary to change the not operator.</p>
+</recommendation>
+
+<example><sample src="IncorrectNotOperatorUsage.cpp" /></example>
+
+<references>
+  <li>
+    <a href="https://docs.microsoft.com/en-us/visualstudio/code-quality/c6317?view=vs-2017">warning C6317: incorrect operator: logical-not (!) is not interchangeable with ones-complement (~)</a>
+  </li>
+</references>
+</qhelp>

cpp/ql/src/Likely Bugs/Likely Typos/IncorrectNotOperatorUsage.ql

@@ -0,0 +1,35 @@
+/**
+ * @name Incorrect Not Operator Usage
+ * @description Usage of a logical-not (!) operator as an operand for a bit-wise operation.
+ *              This commonly indicates the usage of an incorrect operator instead of the bit-wise not (~) operator,
+ *              also known as ones' complement operator.
+ * @kind problem
+ * @id cpp/incorrect-not-operator-usage
+ * @problem.severity warning
+ * @precision medium
+ * @tags security
+ *       external/cwe/cwe-480
+ *       external/microsoft/c6317
+ */
+
+import cpp
+
+/**
+ * It's common in some projects to use "a double negation" to normalize the boolean
+ * result to either 1 or 0.
+ * This predciate is intended to filter explicit usage of a double negation as it typically 
+ * indicates the explicit purpose to normalize the result for bit-wise or arithmetic purposes. 
+ */
+predicate doubleNegationNormalization( NotExpr notexpr ){
+  notexpr.getAnOperand() instanceof NotExpr
+}
+
+from BinaryBitwiseOperation binbitwop
+where exists( NotExpr notexpr | 
+  binbitwop.getAnOperand() = notexpr
+  and not doubleNegationNormalization(notexpr)
+  and ( binbitwop instanceof BitwiseAndExpr
+    or binbitwop instanceof BitwiseOrExpr )
+)
+select binbitwop, "Usage of a logical not (!) expression as a bitwise operator."
+