C++: Fix the bug.

Author: geoffw0

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

@@ -92,7 +92,12 @@ private int getSize(VariableAccess va) {
       // buffer is `12 - 4 = 8`.
       c = getRootType(va) and
       // we calculate the size based on the last field, to avoid including any padding after it
-      trueSize = max(Field f | f = c.getAField() | f.getOffsetInClass(c) + f.getUnspecifiedType().getSize()) and
+      trueSize =
+        max(Field f |
+          f.getDeclaringType*() = c
+        |
+          f.getOffsetInClass(c) + f.getUnspecifiedType().getSize()
+        ) and
       result = trueSize - v.(Field).getOffsetInClass(c)
     )
   )

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

@@ -1,4 +1,4 @@
-| tests.cpp:1056:2:1056:8 | call to strncpy | This 'call to strncpy' operation is limited to 63 bytes but the destination is only -64 bytes. |
-| tests.cpp:1057:2:1057:8 | call to strncpy | This 'call to strncpy' operation is limited to 131 bytes but the destination is only -64 bytes. |
+| tests.cpp:1055:2:1055:8 | call to strncpy | This 'call to strncpy' operation is limited to 131 bytes but the destination is only 128 bytes. |
+| tests.cpp:1057:2:1057:8 | call to strncpy | This 'call to strncpy' operation is limited to 131 bytes but the destination is only 64 bytes. |
 | var_size_struct.cpp:73:3:73:9 | call to strncpy | This 'call to strncpy' operation is limited to 1025 bytes but the destination is only 1024 bytes. |
 | var_size_struct.cpp:103:3:103:9 | call to strncpy | This 'call to strncpy' operation is limited to 129 bytes but the destination is only 128 bytes. |

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

@@ -88,10 +88,10 @@
 | tests.cpp:1001:2:1001:9 | access to array | This array indexing operation accesses a negative index -1 on the $@. | tests.cpp:981:6:981:8 | arr | array |
 | tests.cpp:1009:2:1009:9 | access to array | This array indexing operation accesses a negative index -1 on the $@. | tests.cpp:981:6:981:8 | arr | array |
 | tests.cpp:1031:2:1031:7 | call to memset | This 'memset' operation accesses 130 bytes but the $@ is only 120 bytes. | tests.cpp:1020:12:1020:15 | arr1 | destination buffer |
-| tests.cpp:1051:2:1051:7 | call to memset | This 'memset' operation accesses 64 bytes but the $@ is only -64 bytes. | tests.cpp:1041:8:1041:14 | buffer2 | destination buffer |
-| tests.cpp:1052:2:1052:7 | call to memset | This 'memset' operation accesses 132 bytes but the $@ is only -64 bytes. | tests.cpp:1041:8:1041:14 | buffer2 | destination buffer |
-| tests.cpp:1056:2:1056:8 | call to strncpy | This 'strncpy' operation may access 63 bytes but the $@ is only -64 bytes. | tests.cpp:1041:8:1041:14 | buffer2 | destination buffer |
-| tests.cpp:1057:2:1057:8 | call to strncpy | This 'strncpy' operation may access 131 bytes but the $@ is only -64 bytes. | tests.cpp:1041:8:1041:14 | buffer2 | destination buffer |
+| tests.cpp:1050:2:1050:7 | call to memset | This 'memset' operation accesses 132 bytes but the $@ is only 128 bytes. | tests.cpp:1037:8:1037:14 | buffer1 | destination buffer |
+| tests.cpp:1052:2:1052:7 | call to memset | This 'memset' operation accesses 132 bytes but the $@ is only 64 bytes. | tests.cpp:1041:8:1041:14 | buffer2 | destination buffer |
+| tests.cpp:1055:2:1055:8 | call to strncpy | This 'strncpy' operation may access 131 bytes but the $@ is only 128 bytes. | tests.cpp:1037:8:1037:14 | buffer1 | destination buffer |
+| tests.cpp:1057:2:1057:8 | call to strncpy | This 'strncpy' operation may access 131 bytes but the $@ is only 64 bytes. | tests.cpp:1041:8:1041:14 | buffer2 | destination buffer |
 | 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: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 |

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

@@ -5,6 +5,6 @@
 | tests.cpp:245:42:245:42 | 6 | Potential buffer-overflow: 'global_array_5' has size 5 not 6. |
 | tests.cpp:351:2:351:14 | access to array | Potential buffer-overflow: 'charArray' has size 10 but 'charArray[10]' may be accessed here. |
 | tests.cpp:352:17:352:29 | access to array | Potential buffer-overflow: 'charArray' has size 10 but 'charArray[10]' may be accessed here. |
-| tests.cpp:1056:26:1056:47 | ... - ... | Potential buffer-overflow: 'buffer2' has size -64 not 63. |
-| tests.cpp:1057:26:1057:39 | ... - ... | Potential buffer-overflow: 'buffer2' has size -64 not 131. |
+| tests.cpp:1055:26:1055:39 | ... - ... | Potential buffer-overflow: 'buffer1' has size 128 not 131. |
+| tests.cpp:1057:26:1057:39 | ... - ... | Potential buffer-overflow: 'buffer2' has size 64 not 131. |
 | var_size_struct.cpp:103:39:103:41 | 129 | Potential buffer-overflow: 'str' has size 128 not 129. |

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

@@ -1047,13 +1047,13 @@ void test30() {
 	UnionStruct us;
 
 	memset(us.buffer1, 0, sizeof(us.buffer1)); // GOOD
-	memset(us.buffer1, 0, sizeof(us)); // BAD [NOT DETECTED]
-	memset(us.buffer2, 0, sizeof(us.buffer2)); // GOOD [FALSE POSITIVE]
+	memset(us.buffer1, 0, sizeof(us)); // BAD
+	memset(us.buffer2, 0, sizeof(us.buffer2)); // GOOD
 	memset(us.buffer2, 0, sizeof(us)); // BAD
 
 	strncpy(us.buffer1, "", sizeof(us.buffer1) - 1); // GOOD
-	strncpy(us.buffer1, "", sizeof(us) - 1); // BAD [NOT DETECTED]
-	strncpy(us.buffer2, "", sizeof(us.buffer2) - 1); // GOOD [FALSE POSITIVE]
+	strncpy(us.buffer1, "", sizeof(us) - 1); // BAD
+	strncpy(us.buffer2, "", sizeof(us.buffer2) - 1); // GOOD
 	strncpy(us.buffer2, "", sizeof(us) - 1); // BAD
 }