CPP: Add query for CWE-190: Integer Overflow or Wraparound when using transform after operation

cpp/ql/src/experimental/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation.cpp

@@ -0,0 +1,5 @@
+...
+  vUnsignedLong = (unsigned long)(vUnsignedInt*vUnsignedInt); // BAD
+...
+  vUnsignedLong = ((unsigned long)vUnsignedInt*vUnsignedInt); // GOOD
+...

cpp/ql/src/experimental/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation.qhelp

@@ -0,0 +1,24 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Search for places where the result of the multiplication is subjected to explicit conversion, not the arguments. Therefore, during the multiplication period, you can lose meaningful data.</p>
+
+
+</overview>
+
+<example>
+<p>The following example demonstrates erroneous and fixed methods for working with type conversion.</p>
+<sample src="DangerousUseOfTransformationAfterOperation.cpp" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/INT30-C.+Ensure+that+unsigned+integer+operations+do+not+wrap">INT30-C. Ensure that unsigned integer operations do not wrap</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation.ql

@@ -0,0 +1,102 @@
+/**
+ * @name Dangerous use of transformation after operation.
+ * @description By using the transformation after the operation, you are doing a pointless and dangerous action.
+ * @kind problem
+ * @id cpp/dangerous-use-of-transformation-after-operation
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-190
+ */
+
+import cpp
+
+/** Returns the number of the expression in the function call arguments. */
+int argumentPosition(FunctionCall fc, Expr exp, int n) {
+  n in [0 .. fc.getNumberOfArguments() - 1] and
+  fc.getArgument(n) = exp and
+  result = n
+}
+
+/** Holds if a nonsensical type conversion situation is found. */
+predicate conversionDoneLate(MulExpr mexp) {
+  exists(Expr e1, Expr e2 |
+    mexp.hasOperands(e1, e2) and
+    not e1.isConstant() and
+    not e1.hasConversion() and
+    not e1.hasConversion() and
+    (
+      e2.isConstant() or
+      not e2.hasConversion()
+    ) and
+    mexp.getConversion().hasExplicitConversion() and
+    mexp.getConversion() instanceof ParenthesisExpr and
+    mexp.getConversion().getConversion() instanceof CStyleCast and
+    mexp.getConversion().getConversion().getType().getSize() > mexp.getType().getSize() and
+    mexp.getConversion().getConversion().getType().getSize() > e2.getType().getSize() and
+    mexp.getConversion().getConversion().getType().getSize() > e1.getType().getSize() and
+    exists(Expr e0 |
+      e0.(AssignExpr).getRValue() = mexp.getParent*() and
+      e0.(AssignExpr).getLValue().getType().getSize() =
+        mexp.getConversion().getConversion().getType().getSize()
+      or
+      mexp.getEnclosingElement().(ComparisonOperation).hasOperands(mexp, e0) and
+      e0.getType().getSize() = mexp.getConversion().getConversion().getType().getSize()
+      or
+      e0.(FunctionCall)
+          .getTarget()
+          .getParameter(argumentPosition(e0.(FunctionCall), mexp, _))
+          .getType()
+          .getSize() = mexp.getConversion().getConversion().getType().getSize()
+    )
+  )
+}
+
+/** Holds if the situation of a possible signed overflow used in pointer arithmetic is found. */
+predicate signSmallerWithEqualSizes(MulExpr mexp) {
+  exists(Expr e1, Expr e2 |
+    mexp.hasOperands(e1, e2) and
+    not e1.isConstant() and
+    not e1.hasConversion() and
+    not e1.hasConversion() and
+    (
+      e2.isConstant() or
+      not e2.hasConversion()
+    ) and
+    mexp.getConversion+().getUnderlyingType().getSize() = e1.getUnderlyingType().getSize() and
+    (
+      e2.isConstant() or
+      mexp.getConversion+().getUnderlyingType().getSize() = e2.getUnderlyingType().getSize()
+    ) and
+    mexp.getConversion+().getUnderlyingType().getSize() = e1.getUnderlyingType().getSize() and
+    exists(AssignExpr ae |
+      ae.getRValue() = mexp.getParent*() and
+      ae.getRValue().getUnderlyingType().(IntegralType).isUnsigned() and
+      ae.getLValue().getUnderlyingType().(IntegralType).isSigned() and
+      (
+        not exists(DivExpr de | mexp.getParent*() = de)
+        or
+        exists(DivExpr de, Expr ec |
+          e2.isConstant() and
+          de.hasOperands(mexp.getParent*(), ec) and
+          ec.isConstant() and
+          e2.getValue().toInt() > ec.getValue().toInt()
+        )
+      ) and
+      exists(PointerAddExpr pa |
+        ae.getASuccessor+() = pa and
+        pa.getAnOperand().(VariableAccess).getTarget() = ae.getLValue().(VariableAccess).getTarget()
+      )
+    )
+  )
+}
+
+from MulExpr mexp, string msg
+where
+  conversionDoneLate(mexp) and
+  msg = "This transformation is applied after multiplication."
+  or
+  signSmallerWithEqualSizes(mexp) and
+  msg = "Possible signed overflow followed by offset of the pointer out of bounds."
+select mexp, msg

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation/DangerousUseOfTransformationAfterOperation.expected

@@ -0,0 +1,4 @@
+| test.cpp:9:8:9:12 | ... * ... | Possible signed overflow followed by offset of the pointer out of bounds. |
+| test.cpp:13:24:13:28 | ... * ... | This transformation is applied after multiplication. |
+| test.cpp:16:28:16:32 | ... * ... | This transformation is applied after multiplication. |
+| test.cpp:19:22:19:26 | ... * ... | This transformation is applied after multiplication. |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation/DangerousUseOfTransformationAfterOperation.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/DangerousUseOfTransformationAfterOperation/test.cpp

@@ -0,0 +1,23 @@
+void testCall (unsigned long);
+void functionWork(char aA[10],unsigned int aUI) {
+
+  unsigned long aL;
+  char *aP;
+  int aI;
+
+  aI = (aUI*8)/10; // GOOD
+  aI = aUI*8; // BAD
+  aP = aA+aI;
+  aI = (int)aUI*8; // GOOD
+
+  aL = (unsigned long)(aI*aI); // BAD
+  aL = ((unsigned long)aI*aI); // GOOD
+
+  testCall((unsigned long)(aI*aI)); // BAD
+  testCall(((unsigned long)aI*aI)); // GOOD
+
+  if((unsigned long)(aI*aI) > aL) // BAD
+    return;
+  if(((unsigned long)aI*aI) > aL) // GOOD
+    return;
+}