C++: Fix an issue with padding.

Author: geoffw0

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

@@ -71,7 +71,7 @@ private int getSize(VariableAccess va) {
       result = t.getSize()
     )
     or
-    exists(Class c |
+    exists(Class c, int trueSize |
       // Otherwise, we find the "outermost" object and compute the size
       // as the difference between the size of the type of the "outermost
       // object" and the offset of the field relative to that type.
@@ -91,7 +91,9 @@ private int getSize(VariableAccess va) {
       // of `y` relative to the type `S2` (i.e., `4`). So the size of the
       // buffer is `12 - 4 = 8`.
       c = getRootType(va) and
-      result = c.getSize() - v.(Field).getOffsetInClass(c)
+      // 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
+      result = trueSize - v.(Field).getOffsetInClass(c)
     )
   )
 }

cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/VeryLikelyOverrunWrite.expected

@@ -16,16 +16,17 @@
 | tests.c:136:2:136:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
 | tests.c:186:3:186:9 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 2 bytes. |
 | tests.c:189:3:189:9 | call to sprintf | This 'call to sprintf' operation requires 3 bytes but the destination is only 2 bytes. |
-| unions.c:26:2:26:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 16 bytes. |
-| unions.c:27:2:27:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 16 bytes. |
+| unions.c:26:2:26:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 15 bytes. |
+| unions.c:27:2:27:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 15 bytes. |
 | var_size_struct.cpp:22:3:22:8 | call to strcpy | This 'call to strcpy' operation requires 10 bytes but the destination is only 9 bytes. |
 | varbuffer.c:15:5:15:10 | call to strcpy | This 'call to strcpy' operation requires 2 bytes but the destination is only 1 bytes. |
 | varbuffer.c:16:5:16:10 | call to strcpy | This 'call to strcpy' operation requires 10 bytes but the destination is only 1 bytes. |
 | varbuffer.c:23:5:23:10 | call to strcpy | This 'call to strcpy' operation requires 12 bytes but the destination is only 11 bytes. |
 | varbuffer.c:24:5:24:10 | call to strcpy | This 'call to strcpy' operation requires 17 bytes but the destination is only 11 bytes. |
-| varbuffer.c:40:5:40:10 | call to strcpy | This 'call to strcpy' operation requires 10 bytes but the destination is only 4 bytes. |
-| varbuffer.c:45:5:45:10 | call to strcpy | This 'call to strcpy' operation requires 10 bytes but the destination is only 4 bytes. |
-| varbuffer.c:46:5:46:10 | call to strcpy | This 'call to strcpy' operation requires 17 bytes but the destination is only 4 bytes. |
+| varbuffer.c:39:5:39:10 | call to strcpy | This 'call to strcpy' operation requires 3 bytes but the destination is only 2 bytes. |
+| varbuffer.c:40:5:40:10 | call to strcpy | This 'call to strcpy' operation requires 10 bytes but the destination is only 2 bytes. |
+| varbuffer.c:45:5:45:10 | call to strcpy | This 'call to strcpy' operation requires 10 bytes but the destination is only 2 bytes. |
+| varbuffer.c:46:5:46:10 | call to strcpy | This 'call to strcpy' operation requires 17 bytes but the destination is only 2 bytes. |
 | varbuffer.c:60:5:60:10 | call to strcpy | This 'call to strcpy' operation requires 2 bytes but the destination is only 1 bytes. |
 | varbuffer.c:61:5:61:10 | call to strcpy | This 'call to strcpy' operation requires 10 bytes but the destination is only 1 bytes. |
 | varbuffer.c:67:5:67:10 | call to strcpy | This 'call to strcpy' operation requires 17 bytes but the destination is only 11 bytes. |

cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/varbuffer.c

@@ -36,7 +36,7 @@ void testMyFixedStruct()
     ptr1->len = 1;
     strcpy(ptr1->buffer, ""); // GOOD
     strcpy(ptr1->buffer, "1"); // GOOD
-    strcpy(ptr1->buffer, "12"); // BAD: length 3, but destination only has length 2 [NOT DETECTED]
+    strcpy(ptr1->buffer, "12"); // BAD: length 3, but destination only has length 2
     strcpy(ptr1->buffer, "123456789"); // BAD: length 10, but destination only has length 2
     // ...