Merge pull request github#12779 from MathiasVP/fix-missing-result-in-arith-tainted

C++: Fix FN in `cpp/tainted-arithmetic`

Author: MathiasVP

cpp/ql/src/Security/CWE/CWE-190/ArithmeticTainted.ql

@@ -35,7 +35,8 @@ class Configuration extends TaintTrackingConfiguration {
       op.getAnOperand() = e
     |
       op instanceof UnaryArithmeticOperation or
-      op instanceof BinaryArithmeticOperation
+      op instanceof BinaryArithmeticOperation or
+      op instanceof AssignArithmeticOperation
     )
   }
 

cpp/ql/src/change-notes/2023-04-06-arith-tainted-assign-operations.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The query `cpp/tainted-arithmetic` now also flags possible overflows in arithmetic assignment operations.

cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/ArithmeticTainted.expected

@@ -4,6 +4,18 @@ edges
 | test2.cpp:25:22:25:23 | & ... | test2.cpp:27:13:27:13 | v |
 | test2.cpp:25:22:25:23 | fscanf output argument | test2.cpp:27:13:27:13 | v |
 | test2.cpp:27:13:27:13 | v | test2.cpp:12:21:12:21 | v |
+| test2.cpp:36:9:36:14 | buffer | test2.cpp:39:9:39:11 | num |
+| test2.cpp:36:9:36:14 | buffer | test2.cpp:39:9:39:11 | num |
+| test2.cpp:36:9:36:14 | buffer | test2.cpp:39:9:39:11 | num |
+| test2.cpp:36:9:36:14 | buffer | test2.cpp:39:9:39:11 | num |
+| test2.cpp:36:9:36:14 | buffer | test2.cpp:40:3:40:5 | num |
+| test2.cpp:36:9:36:14 | buffer | test2.cpp:40:3:40:5 | num |
+| test2.cpp:36:9:36:14 | buffer | test2.cpp:40:3:40:5 | num |
+| test2.cpp:36:9:36:14 | buffer | test2.cpp:40:3:40:5 | num |
+| test2.cpp:36:9:36:14 | fgets output argument | test2.cpp:39:9:39:11 | num |
+| test2.cpp:36:9:36:14 | fgets output argument | test2.cpp:39:9:39:11 | num |
+| test2.cpp:36:9:36:14 | fgets output argument | test2.cpp:40:3:40:5 | num |
+| test2.cpp:36:9:36:14 | fgets output argument | test2.cpp:40:3:40:5 | num |
 | test5.cpp:5:5:5:17 | getTaintedInt indirection | test5.cpp:17:6:17:18 | call to getTaintedInt |
 | test5.cpp:5:5:5:17 | getTaintedInt indirection | test5.cpp:17:6:17:18 | call to getTaintedInt |
 | test5.cpp:5:5:5:17 | getTaintedInt indirection | test5.cpp:18:6:18:18 | call to getTaintedInt |
@@ -32,6 +44,13 @@ nodes
 | test2.cpp:25:22:25:23 | & ... | semmle.label | & ... |
 | test2.cpp:25:22:25:23 | fscanf output argument | semmle.label | fscanf output argument |
 | test2.cpp:27:13:27:13 | v | semmle.label | v |
+| test2.cpp:36:9:36:14 | buffer | semmle.label | buffer |
+| test2.cpp:36:9:36:14 | buffer | semmle.label | buffer |
+| test2.cpp:36:9:36:14 | fgets output argument | semmle.label | fgets output argument |
+| test2.cpp:39:9:39:11 | num | semmle.label | num |
+| test2.cpp:39:9:39:11 | num | semmle.label | num |
+| test2.cpp:40:3:40:5 | num | semmle.label | num |
+| test2.cpp:40:3:40:5 | num | semmle.label | num |
 | test5.cpp:5:5:5:17 | getTaintedInt indirection | semmle.label | getTaintedInt indirection |
 | test5.cpp:9:7:9:9 | buf | semmle.label | buf |
 | test5.cpp:9:7:9:9 | buf | semmle.label | buf |
@@ -56,6 +75,8 @@ nodes
 #select
 | test2.cpp:14:11:14:11 | v | test2.cpp:25:22:25:23 | & ... | test2.cpp:14:11:14:11 | v | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
 | test2.cpp:14:11:14:11 | v | test2.cpp:25:22:25:23 | & ... | test2.cpp:14:11:14:11 | v | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
+| test2.cpp:39:9:39:11 | num | test2.cpp:36:9:36:14 | buffer | test2.cpp:39:9:39:11 | num | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test2.cpp:36:9:36:14 | buffer | User-provided value |
+| test2.cpp:40:3:40:5 | num | test2.cpp:36:9:36:14 | buffer | test2.cpp:40:3:40:5 | num | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test2.cpp:36:9:36:14 | buffer | User-provided value |
 | test5.cpp:17:6:17:18 | call to getTaintedInt | test5.cpp:9:7:9:9 | buf | test5.cpp:17:6:17:18 | call to getTaintedInt | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test5.cpp:9:7:9:9 | buf | User-provided value |
 | test5.cpp:19:6:19:6 | y | test5.cpp:9:7:9:9 | buf | test5.cpp:19:6:19:6 | y | $@ flows to an operand of an arithmetic expression, potentially causing an overflow. | test5.cpp:9:7:9:9 | buf | User-provided value |
 | test5.cpp:19:6:19:6 | y | test5.cpp:9:7:9:9 | buf | test5.cpp:19:6:19:6 | y | $@ flows to an operand of an arithmetic expression, potentially causing an underflow. | test5.cpp:9:7:9:9 | buf | User-provided value |

cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/IntegerOverflowTainted.expected

@@ -2,6 +2,8 @@
 | test2.cpp:15:11:15:19 | ... * ... | $@ flows an expression which might overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
 | test2.cpp:16:11:16:21 | ... * ... | $@ flows an expression which might overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
 | test2.cpp:17:11:17:22 | ... * ... | $@ flows an expression which might overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
+| test2.cpp:39:9:39:18 | ... + ... | $@ flows an expression which might overflow. | test2.cpp:36:9:36:14 | buffer | User-provided value |
+| test2.cpp:40:3:40:13 | ... += ... | $@ flows an expression which might overflow. | test2.cpp:36:9:36:14 | buffer | User-provided value |
 | test3.c:12:31:12:34 | * ... | $@ flows an expression which might overflow negatively. | test3.c:11:15:11:18 | argv | User-provided value |
 | test3.c:13:16:13:19 | * ... | $@ flows an expression which might overflow negatively. | test3.c:11:15:11:18 | argv | User-provided value |
 | test4.cpp:13:17:13:20 | access to array | $@ flows an expression which might overflow negatively. | test4.cpp:9:13:9:16 | argv | User-provided value |

cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/test2.cpp

@@ -26,3 +26,16 @@ void test2_source()
 	ms.val = v;
 	test2_sink(v, ms, ms, &ms);
 }
+
+char *fgets(char *, int, FILE *);
+int atoi(const char *);
+
+void test3()
+{
+  char buffer[20];
+  fgets(buffer, 20, stdin);
+
+  int num = atoi(buffer);
+  num = num + 1000; // BAD
+  num += 1000; // BAD
+}