C++: Use 'max' instead of 'unique.'

cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll

@@ -80,7 +80,7 @@ private predicate localFlowToExprStep(DataFlow::Node n1, DataFlow::Node n2) {
 
 /** Holds if `n2 + delta` may be equal to `n1`. */
 private predicate localFlowStepToExpr(Expr e1, Expr e2) {
-  getBufferSize0(e1) and
+  getBufferSizeCand0(e1) and
   exists(DataFlow::Node n1, DataFlow::Node mid, DataFlow::Node n2 |
     n1.asExpr() = e1 and
     localFlowToExprStep*(n1, mid) and
@@ -95,7 +95,7 @@ private predicate localFlowStepToExpr(Expr e1, Expr e2) {
  * expression.
  */
 private predicate step(Expr e1, Expr e2, int delta) {
-  getBufferSize0(e1) and
+  getBufferSizeCand0(e1) and
   exists(Variable bufferVar, Class parentClass, VariableAccess parentPtr, int bufferSize |
     e1 = parentPtr
   |
@@ -117,27 +117,37 @@ private predicate step(Expr e1, Expr e2, int delta) {
 }
 
 pragma[nomagic]
-private predicate getBufferSize0(Expr e) {
+private predicate getBufferSizeCand0(Expr e) {
   exists(isSource(e, _))
   or
   exists(Expr e0 |
-    getBufferSize0(e0) and
+    getBufferSizeCand0(e0) and
     step(e0, e, _)
   )
 }
 
+/**
+ * Get the size in bytes of the buffer pointed to by an expression (if this can be determined).
+ *
+ * NOTE: There can be multiple `(result, why)` for a given `bufferExpr`.
+ */
+private int getBufferSizeCand(Expr bufferExpr, Element why) {
+  getBufferSizeCand0(bufferExpr) and
+  (
+    result = isSource(bufferExpr, why)
+    or
+    exists(Expr e0, int delta, int size |
+      size = getBufferSizeCand(e0, why) and
+      step(e0, bufferExpr, delta) and
+      result = size + delta
+    )
+  )
+}
+
 /**
  * Get the size in bytes of the buffer pointed to by an expression (if this can be determined).
  */
 int getBufferSize(Expr bufferExpr, Element why) {
-  getBufferSize0(bufferExpr) and
-  (
-    result = isSource(bufferExpr, why)
-    or
-    exists(Expr e0, int delta, int size |
-      size = getBufferSize(e0, why) and
-      delta = unique(int cand | step(e0, bufferExpr, cand) | cand) and
-      result = size + delta
-    )
-  )
+  result = max( | | getBufferSizeCand(bufferExpr, _)) and
+  result = getBufferSizeCand(bufferExpr, why)
 }

cpp/ql/test/query-tests/Security/CWE/CWE-119/SAMATE/VeryLikelyOverrunWrite.expected

@@ -1 +0,0 @@
-| tests.cpp:668:9:668:14 | call to strcpy | This 'call to strcpy' operation requires 11 bytes but the destination is only 10 bytes. |

cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowBuffer.expected

@@ -48,7 +48,6 @@
 | tests.cpp:495:2:495:7 | access to array | This array indexing operation accesses a negative index -1 on the $@. | tests.cpp:474:21:474:26 | call to malloc | array |
 | tests.cpp:519:3:519:8 | call to memset | This 'memset' operation accesses 20 bytes but the $@ is only 10 bytes. | tests.cpp:502:15:502:20 | call to malloc | destination buffer |
 | tests.cpp:519:3:519:8 | call to memset | This 'memset' operation accesses 20 bytes but the $@ is only 10 bytes. | tests.cpp:510:16:510:21 | call to malloc | destination buffer |
-| tests.cpp:520:3:520:8 | call to memset | This 'memset' operation accesses 20 bytes but the $@ is only 10 bytes. | tests.cpp:503:15:503:20 | call to malloc | destination buffer |
 | tests.cpp:541:6:541:10 | call to fread | This 'fread' operation may access 101 bytes but the $@ is only 100 bytes. | tests.cpp:532:7:532:16 | charBuffer | destination buffer |
 | tests.cpp:546:6:546:10 | call to fread | This 'fread' operation may access 400 bytes but the $@ is only 100 bytes. | tests.cpp:532:7:532:16 | charBuffer | destination buffer |
 | tests.cpp:569:6:569:15 | access to array | This array indexing operation accesses a negative index -1 on the $@. | tests.cpp:565:7:565:12 | buffer | array |
@@ -57,9 +56,6 @@
 | tests.cpp:586:6:586:12 | access to array | This array indexing operation accesses a negative index -1 on the $@. | tests.cpp:565:7:565:12 | buffer | array |
 | tests_restrict.c:12:2:12:7 | call to memcpy | This 'memcpy' operation accesses 2 bytes but the $@ is only 1 byte. | tests_restrict.c:7:6:7:13 | smallbuf | source buffer |
 | unions.cpp:26:2:26:7 | call to memset | This 'memset' operation accesses 200 bytes but the $@ is only 100 bytes. | unions.cpp:21:10:21:11 | mu | destination buffer |
-| unions.cpp:27:2:27:7 | call to memset | This 'memset' operation accesses 100 bytes but the $@ is only 10 bytes. | unions.cpp:15:7:15:11 | small | destination buffer |
-| unions.cpp:29:2:29:7 | call to memset | This 'memset' operation accesses 100 bytes but the $@ is only 10 bytes. | unions.cpp:15:7:15:11 | small | destination buffer |
-| unions.cpp:30:2:30:7 | call to memset | This 'memset' operation accesses 200 bytes but the $@ is only 10 bytes. | unions.cpp:15:7:15:11 | small | destination buffer |
 | unions.cpp:30:2:30:7 | call to memset | This 'memset' operation accesses 200 bytes but the $@ is only 100 bytes. | unions.cpp:15:7:15:11 | small | destination buffer |
 | unions.cpp:34:2:34:7 | call to memset | This 'memset' operation accesses 200 bytes but the $@ is only 100 bytes. | unions.cpp:16:7:16:11 | large | destination buffer |
 | var_size_struct.cpp:71:3:71:8 | call to memset | This 'memset' operation accesses 1025 bytes but the $@ is only 1024 bytes. | var_size_struct.cpp:67:35:67:40 | call to malloc | destination buffer |

cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/tests.cpp

@@ -517,7 +517,7 @@ void test19(bool b)
 	if (b)
 	{
 		memset(p1, 0, 20); // BAD
-		memset(p2, 0, 20); // GOOD [FALSE POSITIVE]
+		memset(p2, 0, 20); // GOOD
 		memset(p3, 0, 20); // GOOD
 	}
 }