Skip to content

Commit e467cc0

Browse files
authored
Merge pull request #17037 from geoffw0/sizecheck
C++: Fix issue with cpp/suspicious-allocation-size
2 parents 1ed5af1 + 437c679 commit e467cc0

File tree

5 files changed

+43
-4
lines changed

5 files changed

+43
-4
lines changed

cpp/ql/src/Critical/SizeCheck2.ql

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import cpp
1717
import semmle.code.cpp.models.Models
18+
import semmle.code.cpp.commons.Buffer
1819

1920
predicate baseType(AllocationExpr alloc, Type base) {
2021
exists(PointerType pointer |
@@ -30,7 +31,8 @@ predicate baseType(AllocationExpr alloc, Type base) {
3031
}
3132

3233
predicate decideOnSize(Type t, int size) {
33-
// If the codebase has more than one type with the same name, it can have more than one size.
34+
// If the codebase has more than one type with the same name, it can have more than one size. For
35+
// most purposes in this query, we use the smallest.
3436
size = min(t.getSize())
3537
}
3638

@@ -45,7 +47,8 @@ where
4547
size = 0 or
4648
(allocated / size) * size = allocated
4749
) and
48-
not basesize > allocated // covered by SizeCheck.ql
50+
not basesize > allocated and // covered by SizeCheck.ql
51+
not memberMayBeVarSize(base.getUnspecifiedType(), _) // exclude variable size types
4952
select alloc,
5053
"Allocated memory (" + allocated.toString() + " bytes) is not a multiple of the size of '" +
5154
base.getName() + "' (" + basesize.toString() + " bytes)."
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `cpp/suspicious-allocation-size` ("Not enough memory allocated for array of pointer type") query no longer produces false positives on "variable size" `struct`s.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@
22
| test2.c:17:20:17:25 | call to malloc | Allocated memory (33 bytes) is not a multiple of the size of 'double' (8 bytes). |
33
| 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). |
44
| test2.c:33:20:33:25 | call to malloc | Allocated memory (20 bytes) is not a multiple of the size of 'double' (8 bytes). |
5+
| 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/test.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ void test_union() {
6060
}
6161

6262
// --- custom allocators ---
63-
63+
6464
void *MyMalloc1(size_t size) { return malloc(size); }
6565
void *MyMalloc2(size_t size);
6666

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

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ void good1(void) {
4444
}
4545

4646
// --- custom allocators ---
47-
47+
4848
void *MyMalloc1(size_t size) { return malloc(size); }
4949
void *MyMalloc2(size_t size);
5050

@@ -53,3 +53,34 @@ void customAllocatorTests()
5353
double *dptr1 = MyMalloc1(33); // BAD -- Not a multiple of sizeof(double) [NOT DETECTED]
5454
double *dptr2 = MyMalloc2(33); // BAD -- Not a multiple of sizeof(double) [NOT DETECTED]
5555
}
56+
57+
// --- variable length data structures ---
58+
59+
typedef unsigned char uint8_t;
60+
61+
typedef struct _MyVarStruct1 {
62+
size_t dataLen;
63+
uint8_t data[0];
64+
} MyVarStruct1;
65+
66+
typedef struct _MyVarStruct2 {
67+
size_t dataLen;
68+
uint8_t data[1];
69+
} MyVarStruct2;
70+
71+
typedef struct _MyVarStruct3 {
72+
size_t dataLen;
73+
uint8_t data[];
74+
} MyVarStruct3;
75+
76+
typedef struct _MyFixedStruct {
77+
size_t dataLen;
78+
uint8_t data[1024];
79+
} MyFixedStruct;
80+
81+
void varStructTests() {
82+
MyVarStruct1 *a = malloc(sizeof(MyVarStruct1) + 127); // GOOD
83+
MyVarStruct2 *b = malloc(sizeof(MyVarStruct2) + 127); // GOOD
84+
MyVarStruct3 *c = malloc(sizeof(MyVarStruct3) + 127); // GOOD
85+
MyFixedStruct *d = malloc(sizeof(MyFixedStruct) + 127); // BAD --- Not a multiple of sizeof(MyFixedStruct)
86+
}

0 commit comments

Comments
 (0)