C++: Add tests to cpp/unsigned-difference-expression-compared-zero and remove a couple of classes of FPs.

Author: MathiasVP

cpp/ql/src/experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql

@@ -13,11 +13,28 @@
 
 import cpp
 import semmle.code.cpp.commons.Exclusions
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+import semmle.code.cpp.controlflow.Guards
+
+/** Holds if `sub` will never be nonnegative. */
+predicate nonNegative(SubExpr sub) {
+  not exprMightOverflowNegatively(sub.getFullyConverted())
+  or
+  // The subtraction is guarded by a check of the form `left >= right`.
+  exists(GuardCondition guard, Expr left, Expr right |
+    left = globalValueNumber(sub.getLeftOperand()).getAnExpr() and
+    right = globalValueNumber(sub.getRightOperand()).getAnExpr() and
+    guard.controls(sub.getBasicBlock(), true) and
+    guard.ensuresLt(left, right, 0, sub.getBasicBlock(), false)
+  )
+}
 
 from RelationalOperation ro, SubExpr sub
 where
   not isFromMacroDefinition(ro) and
   ro.getLesserOperand().getValue().toInt() = 0 and
   ro.getGreaterOperand() = sub and
-  sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned()
+  sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned() and
+  not nonNegative(sub)
 select ro, "Difference in condition is always greater than or equal to zero"

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero/UnsignedDifferenceExpressionComparedZero.expected

@@ -0,0 +1,10 @@
+| test.cpp:6:5:6:13 | ... > ... | Difference in condition is always greater than or equal to zero |
+| test.cpp:10:8:10:24 | ... > ... | Difference in condition is always greater than or equal to zero |
+| test.cpp:15:9:15:25 | ... > ... | Difference in condition is always greater than or equal to zero |
+| test.cpp:32:12:32:20 | ... > ... | Difference in condition is always greater than or equal to zero |
+| test.cpp:39:12:39:20 | ... > ... | Difference in condition is always greater than or equal to zero |
+| test.cpp:47:5:47:13 | ... > ... | Difference in condition is always greater than or equal to zero |
+| test.cpp:55:5:55:13 | ... > ... | Difference in condition is always greater than or equal to zero |
+| test.cpp:62:5:62:13 | ... > ... | Difference in condition is always greater than or equal to zero |
+| test.cpp:69:5:69:13 | ... > ... | Difference in condition is always greater than or equal to zero |
+| test.cpp:75:8:75:16 | ... > ... | Difference in condition is always greater than or equal to zero |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero/UnsignedDifferenceExpressionComparedZero.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero/test.cpp

@@ -0,0 +1,77 @@
+int getAnInt();
+
+bool cond();
+
+void test(unsigned x, unsigned y, bool unknown) {
+	if(x - y > 0) { } // BAD
+
+	unsigned total = getAnInt();
+	unsigned limit = getAnInt();
+	while(limit - total > 0) { // BAD
+		total += getAnInt();
+	}
+
+	if(total <= limit) {
+		while(limit - total > 0) { // GOOD [FALSE POSITIVE]
+			total += getAnInt();
+			if(total > limit) break;
+		}
+	}
+
+	if(x >= y) {
+		bool b = x - y > 0; // GOOD
+	}
+
+	if((int)(x - y) >= 0) { } // GOOD. Maybe an overflow happened, but the result is converted to the "likely intended" result before the comparison
+
+	if(unknown) {
+		y = x & 0xFF;
+	} else {
+		y = x;
+	}
+	bool b1 = x - y > 0; // GOOD [FALSE POSITIVE]
+
+	x = getAnInt();
+	y = getAnInt();
+	if(y > x) {
+		y = x - 1;
+	}
+	bool b2 = x - y > 0; // GOOD [FALSE POSITIVE]
+
+	int N = getAnInt();
+	y = x;
+	while(cond()) {
+		if(unknown) { y--; }
+	}
+	
+	if(x - y > 0) { } // GOOD [FALSE POSITIVE]
+
+	x = y;
+	while(cond()) {
+		if(unknown) break;
+		y--;
+	}
+
+	if(x - y > 0) { } // GOOD [FALSE POSITIVE]
+
+	y = 0;
+	for(int i = 0; i < x; ++i) {
+		if(unknown) { ++y; }
+	}
+
+	if(x - y > 0) { } // GOOD [FALSE POSITIVE]
+
+	x = y;
+	while(cond()) {
+		if(unknown) { x++; }
+	}
+
+	if(x - y > 0) { } // GOOD [FALSE POSITIVE]
+
+	int n = getAnInt();
+	if (n > x - y) { n = x - y; }
+	if (n > 0) {
+  	y += n; // NOTE: `n` is at most `x - y` at this point.
+  	if (x - y > 0) {} // GOOD [FALSE POSITIVE]
+	}
+}