IntegerOverflow: Support divide and remainder

Author: lcartey

c/cert/src/rules/INT32-C/SignedIntegerOverflow.ql

@@ -21,7 +21,23 @@ import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 from InterestingOverflowingOperation op
 where
   not isExcluded(op, IntegerOverflowPackage::signedIntegerOverflowQuery()) and
-  op.getType().getUnderlyingType().(IntegralType).isSigned() and
+  (
+    // An operation that returns a signed integer type
+    op.getType().getUnderlyingType().(IntegralType).isSigned()
+    or
+    // The divide or rem expression on a signed integer
+    (op instanceof DivExpr or op instanceof RemExpr) and
+    op.(BinaryOperation).getLeftOperand().getType().getUnderlyingType().(IntegralType).isSigned()
+    or
+    // The assign divide or rem expression on a signed integer
+    (op instanceof AssignDivExpr or op instanceof AssignRemExpr) and
+    op.(AssignArithmeticOperation)
+        .getLValue()
+        .getType()
+        .getUnderlyingType()
+        .(IntegralType)
+        .isSigned()
+  ) and
   // Not checked before the operation
   not op.hasValidPreCheck() and
   // Not guarded by a check, where the check is not an invalid overflow check

c/cert/test/rules/INT32-C/SignedIntegerOverflow.expected

@@ -11,6 +11,10 @@
 | test.c:62:3:62:10 | ... -= ... | Operation -= of type signed int may overflow or underflow. |
 | test.c:69:3:69:8 | ... * ... | Operation * of type int may overflow or underflow. |
 | test.c:70:3:70:10 | ... *= ... | Operation *= of type signed int may overflow or underflow. |
-| test.c:153:3:153:10 | ... << ... | Operation << of type signed int may overflow or underflow. |
-| test.c:154:3:154:11 | ... <<= ... | Operation <<= of type signed int may overflow or underflow. |
-| test.c:173:3:173:5 | - ... | Operation - of type signed int may overflow or underflow. |
+| test.c:117:3:117:9 | ... / ... | Operation / of type int may overflow or underflow. |
+| test.c:118:3:118:10 | ... /= ... | Operation /= of type signed int may overflow or underflow. |
+| test.c:140:3:140:9 | ... % ... | Operation % of type int may overflow or underflow. |
+| test.c:141:3:141:10 | ... %= ... | Operation %= of type signed int may overflow or underflow. |
+| test.c:163:3:163:10 | ... << ... | Operation << of type signed int may overflow or underflow. |
+| test.c:164:3:164:11 | ... <<= ... | Operation <<= of type signed int may overflow or underflow. |
+| test.c:183:3:183:5 | - ... | Operation - of type signed int may overflow or underflow. |

c/cert/test/rules/INT32-C/test.c

@@ -114,6 +114,11 @@ void test_mul_precheck_2(signed int i1, signed int i2) {
 }
 
 void test_simple_div(signed int i1, signed int i2) {
+  i1 / i2;  // NON_COMPLIANT
+  i1 /= i2; // NON_COMPLIANT
+}
+
+void test_simple_div_no_zero(signed int i1, signed int i2) {
   if (i2 == 0) {
     // handle error
   } else {
@@ -132,6 +137,11 @@ void test_div_precheck(signed int i1, signed int i2) {
 }
 
 void test_simple_rem(signed int i1, signed int i2) {
+  i1 % i2;  // NON_COMPLIANT
+  i1 %= i2; // NON_COMPLIANT
+}
+
+void test_simple_rem_no_zero(signed int i1, signed int i2) {
   if (i2 == 0) {
     // handle error
   } else {

cpp/common/src/codingstandards/cpp/Overflow.qll

@@ -19,6 +19,23 @@ class InterestingOverflowingOperation extends Operation {
       exprMightOverflowNegatively(this)
       or
       exprMightOverflowPositively(this)
+      or
+      // Division and remainder are not handled by the library
+      exists(Expr leftOperand, Expr rightOperand |
+        (this instanceof DivExpr or this instanceof RemExpr) and
+        leftOperand = this.(BinaryOperation).getLeftOperand() and
+        rightOperand = this.(BinaryOperation).getRightOperand()
+        or
+        (this instanceof AssignDivExpr or this instanceof AssignRemExpr) and
+        leftOperand = this.(AssignArithmeticOperation).getLValue() and
+        rightOperand = this.(AssignArithmeticOperation).getRValue()
+      |
+        // The right hand side could be -1
+        upperBound(rightOperand) >= -1.0 and
+        lowerBound(rightOperand) <= -1.0 and
+        // The left hand side could be the smallest possible integer value
+        lowerBound(leftOperand) <= typeLowerBound(leftOperand.getType().getUnderlyingType())
+      )
     ) and
     // Multiplication is not covered by the standard range analysis library, so implement our own
     // mini analysis.