Update IfStatementAdditionOverflow.ql

Author: nmouha

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

@@ -1,10 +1,13 @@
 /**
- * @name Integer addition may overflow inside if statement
- * @description Detects "if (a+b>c) a=c-b", which is incorrect if a+b overflows.
- *              Should be replaced by "if (a>c-b) a=c-b", which correctly
- *              implements a = min(a,c-b)". This integer overflow is the root
- *              cause of the buffer overflow in the SHA-3 reference implementation
- *              (CVE-2022-37454).
+ * @name Integer addition may overflow inside condition
+ * @description Detects "c-b" when the condition "a+b>c" has been imposed,
+ *              which is not the same as the condition "a>b-c" if "a+b"
+ *              overflows. Rewriting improves readability and optimizability
+ *              (CSE elimination). Also detects "b+a>c" (swapped terms in
+ *              addition), "c<a+b" (swapped operands), and  ">=", "<",
+ *              "<=" instead of ">" (all operators). This integer overflow
+ *              is the root cause of the buffer overflow in the SHA-3
+ *              reference implementation (CVE-2022-37454).
  * @kind problem
  * @problem.severity warning
  * @id cpp/if-statement-addition-overflow
@@ -15,14 +18,19 @@
  */
 
 import cpp
+import semmle.code.cpp.controlflow.Guards
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
+import semmle.code.cpp.commons.Exclusions
 
-from IfStmt ifstmt, GTExpr gtexpr, ExprStmt exprstmt, AssignExpr assignexpr, AddExpr addexpr, SubExpr subexpr
-where ifstmt.getCondition() = gtexpr and
-  gtexpr.getLeftOperand() = addexpr and
-  ifstmt.getThen() = exprstmt and
-  exprstmt.getExpr() = assignexpr and
-  assignexpr.getRValue() = subexpr and
-  addexpr.getLeftOperand().toString() = assignexpr.getLValue().toString() and
-  addexpr.getRightOperand().toString() = subexpr.getRightOperand().toString() and
-  gtexpr.getRightOperand().toString() = subexpr.getLeftOperand().toString()
-select ifstmt, "Integer addition may overflow inside if statement."
+from GuardCondition guard, BasicBlock block, RelationalOperation relop, AddExpr addexpr, SubExpr subexpr
+where guard.controls(block, _) and
+  guard.getAChild*() = relop and
+  pragma[only_bind_into](block) = subexpr.getBasicBlock() and
+  relop.getAnOperand() = addexpr and
+  addexpr.getUnspecifiedType() instanceof IntegralType and
+  not isFromMacroDefinition(relop) and
+  exprMightOverflowPositively(addexpr) and
+  globalValueNumber(addexpr.getAnOperand()) = globalValueNumber(subexpr.getRightOperand()) and
+  globalValueNumber(relop.getAnOperand()) = globalValueNumber(subexpr.getLeftOperand())
+select guard, "Integer addition may overflow inside condition."