C++: Fix false positives.

Author: geoffw0

cpp/ql/src/Critical/SizeCheck2.ql

@@ -30,10 +30,20 @@ predicate baseType(AllocationExpr alloc, Type base) {
 }
 
 predicate decideOnSize(Type t, int size) {
-  // If the codebase has more than one type with the same name, it can have more than one size.
+  // If the codebase has more than one type with the same name, it can have more than one size. For
+  // most purposes in this query, we use the smallest.
   size = min(t.getSize())
 }
 
+predicate mayHaveVarSize(Type t) {
+  // a member (normally at the end of the type) that looks like it may be intended have variable size.
+  exists(MemberVariable mv, ArrayType at |
+    mv.getDeclaringType() = t and
+    mv.getUnspecifiedType() = at and
+    not at.getArraySize() > 1
+  )
+}
+
 from AllocationExpr alloc, Type base, int basesize, int allocated
 where
   baseType(alloc, base) and
@@ -45,7 +55,8 @@ where
     size = 0 or
     (allocated / size) * size = allocated
   ) and
-  not basesize > allocated // covered by SizeCheck.ql
+  not basesize > allocated and // covered by SizeCheck.ql
+  not mayHaveVarSize(base.getUnspecifiedType()) // exclude variable size types
 select alloc,
   "Allocated memory (" + allocated.toString() + " bytes) is not a multiple of the size of '" +
     base.getName() + "' (" + basesize.toString() + " bytes)."

cpp/ql/test/query-tests/Critical/SizeCheck/SizeCheck2.expected

@@ -2,7 +2,4 @@
 | test2.c:17:20:17:25 | call to malloc | Allocated memory (33 bytes) is not a multiple of the size of 'double' (8 bytes). |
 | test2.c:32:23:32:28 | call to malloc | Allocated memory (28 bytes) is not a multiple of the size of 'long long' (8 bytes). |
 | test2.c:33:20:33:25 | call to malloc | Allocated memory (20 bytes) is not a multiple of the size of 'double' (8 bytes). |
-| test2.c:82:23:82:28 | call to malloc | Allocated memory (135 bytes) is not a multiple of the size of 'MyVarStruct1' (8 bytes). |
-| test2.c:83:23:83:28 | call to malloc | Allocated memory (143 bytes) is not a multiple of the size of 'MyVarStruct2' (16 bytes). |
-| test2.c:84:23:84:28 | call to malloc | Allocated memory (135 bytes) is not a multiple of the size of 'MyVarStruct3' (8 bytes). |
 | test2.c:85:24:85:29 | call to malloc | Allocated memory (1159 bytes) is not a multiple of the size of 'MyFixedStruct' (1032 bytes). |

cpp/ql/test/query-tests/Critical/SizeCheck/test2.c

@@ -79,8 +79,8 @@ typedef struct _MyFixedStruct {
 } MyFixedStruct;
 
 void varStructTests() {
-    MyVarStruct1 *a = malloc(sizeof(MyVarStruct1) + 127); // GOOD [FALSE POSITIVE]
-    MyVarStruct2 *b = malloc(sizeof(MyVarStruct2) + 127); // GOOD [FALSE POSITIVE]
-    MyVarStruct3 *c = malloc(sizeof(MyVarStruct3) + 127); // GOOD [FALSE POSITIVE]
+    MyVarStruct1 *a = malloc(sizeof(MyVarStruct1) + 127); // GOOD
+    MyVarStruct2 *b = malloc(sizeof(MyVarStruct2) + 127); // GOOD
+    MyVarStruct3 *c = malloc(sizeof(MyVarStruct3) + 127); // GOOD
     MyFixedStruct *d = malloc(sizeof(MyFixedStruct) + 127); // BAD --- Not a multiple of sizeof(MyFixedStruct)
 }