Skip to content

Commit b638d4d

Browse files
MathiasVPMathiasVP
authored
Merge pull request #15900 from MathiasVP/glib-alloc-and-dealloc
C++: Add models for `GLib` allocation and deallocation
2 parents bd121b9 + 465c3c1 commit b638d4d

File tree

8 files changed

+45
-4
lines changed

8 files changed

+45
-4
lines changed

‎cpp/ql/lib/semmle/code/cpp/models/implementations/Allocation.qll

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ private class MallocAllocationFunction extends AllocationFunction {
3636
"CRYPTO_malloc", // CRYPTO_malloc(size_t num, const char *file, int line)
3737
"CRYPTO_zalloc", // CRYPTO_zalloc(size_t num, const char *file, int line)
3838
"CRYPTO_secure_malloc", // CRYPTO_secure_malloc(size_t num, const char *file, int line)
39-
"CRYPTO_secure_zalloc" // CRYPTO_secure_zalloc(size_t num, const char *file, int line)
39+
"CRYPTO_secure_zalloc", // CRYPTO_secure_zalloc(size_t num, const char *file, int line)
40+
"g_malloc", // g_malloc (n_bytes);
41+
"g_try_malloc" // g_try_malloc(n_bytes);
4042
]) and
4143
sizeArg = 0
4244
or
@@ -139,7 +141,9 @@ private class ReallocAllocationFunction extends AllocationFunction, TaintFunctio
139141
// --- Windows COM allocation
140142
"CoTaskMemRealloc", // CoTaskMemRealloc(ptr, size)
141143
// --- OpenSSL memory allocation
142-
"CRYPTO_realloc" // CRYPTO_realloc(void *addr, size_t num, const char *file, int line)
144+
"CRYPTO_realloc", // CRYPTO_realloc(void *addr, size_t num, const char *file, int line)
145+
"g_realloc", // g_realloc(mem, n_bytes);
146+
"g_try_realloc" // g_try_realloc(mem, n_bytes);
143147
]) and
144148
sizeArg = 1 and
145149
reallocArg = 0

‎cpp/ql/lib/semmle/code/cpp/models/implementations/Deallocation.qll

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@ private class StandardDeallocationFunction extends DeallocationFunction {
2020
freedArg = 0
2121
or
2222
this.hasGlobalName([
23-
// --- OpenSSL memory allocation
24-
"CRYPTO_free", "CRYPTO_secure_free"
23+
// --- OpenSSL memory deallocation
24+
"CRYPTO_free", "CRYPTO_secure_free",
25+
// --- glib memory deallocation
26+
"g_free"
2527
]) and
2628
freedArg = 0
2729
or

‎cpp/ql/src/change-notes/2024-03-13-glib-alloc-and-dealloc.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added models for `GLib` allocation and deallocation functions.

‎cpp/ql/test/query-tests/Critical/MemoryFreed/DoubleFree.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ edges
1111
| test_free.cpp:128:10:128:11 | pointer to free output argument | test_free.cpp:129:10:129:11 | * ... | provenance | |
1212
| test_free.cpp:152:27:152:27 | pointer to free output argument | test_free.cpp:154:10:154:10 | a | provenance | |
1313
| test_free.cpp:207:10:207:10 | pointer to free output argument | test_free.cpp:209:10:209:10 | a | provenance | |
14+
| test_free.cpp:301:12:301:14 | pointer to g_free output argument | test_free.cpp:302:12:302:14 | buf | provenance | |
1415
nodes
1516
| test_free.cpp:11:10:11:10 | pointer to free output argument | semmle.label | pointer to free output argument |
1617
| test_free.cpp:14:10:14:10 | a | semmle.label | a |
@@ -36,6 +37,8 @@ nodes
3637
| test_free.cpp:154:10:154:10 | a | semmle.label | a |
3738
| test_free.cpp:207:10:207:10 | pointer to free output argument | semmle.label | pointer to free output argument |
3839
| test_free.cpp:209:10:209:10 | a | semmle.label | a |
40+
| test_free.cpp:301:12:301:14 | pointer to g_free output argument | semmle.label | pointer to g_free output argument |
41+
| test_free.cpp:302:12:302:14 | buf | semmle.label | buf |
3942
subpaths
4043
#select
4144
| test_free.cpp:14:10:14:10 | a | test_free.cpp:11:10:11:10 | pointer to free output argument | test_free.cpp:14:10:14:10 | a | Memory pointed to by 'a' may already have been freed by $@. | test_free.cpp:11:5:11:8 | call to free | call to free |
@@ -50,3 +53,4 @@ subpaths
5053
| test_free.cpp:129:10:129:11 | * ... | test_free.cpp:128:10:128:11 | pointer to free output argument | test_free.cpp:129:10:129:11 | * ... | Memory pointed to by '* ...' may already have been freed by $@. | test_free.cpp:128:5:128:8 | call to free | call to free |
5154
| test_free.cpp:154:10:154:10 | a | test_free.cpp:152:27:152:27 | pointer to free output argument | test_free.cpp:154:10:154:10 | a | Memory pointed to by 'a' may already have been freed by $@. | test_free.cpp:152:22:152:25 | call to free | call to free |
5255
| test_free.cpp:209:10:209:10 | a | test_free.cpp:207:10:207:10 | pointer to free output argument | test_free.cpp:209:10:209:10 | a | Memory pointed to by 'a' may already have been freed by $@. | test_free.cpp:207:5:207:8 | call to free | call to free |
56+
| test_free.cpp:302:12:302:14 | buf | test_free.cpp:301:12:301:14 | pointer to g_free output argument | test_free.cpp:302:12:302:14 | buf | Memory pointed to by 'buf' may already have been freed by $@. | test_free.cpp:301:5:301:10 | call to g_free | call to g_free |

‎cpp/ql/test/query-tests/Critical/MemoryFreed/MemoryFreed.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@
102102
| test_free.cpp:282:10:282:12 | buf |
103103
| test_free.cpp:288:8:288:10 | buf |
104104
| test_free.cpp:293:8:293:10 | buf |
105+
| test_free.cpp:301:12:301:14 | buf |
106+
| test_free.cpp:302:12:302:14 | buf |
105107
| virtual.cpp:18:10:18:10 | a |
106108
| virtual.cpp:19:10:19:10 | c |
107109
| virtual.cpp:38:10:38:10 | b |

‎cpp/ql/test/query-tests/Critical/MemoryFreed/test_free.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,4 +293,11 @@ void test_free_struct4(char* buf, MyStruct s) {
293293
free(buf);
294294
s.buf = buf;
295295
char c = s.buf[0]; // BAD
296+
}
297+
298+
void g_free (void*);
299+
300+
void test_g_free(char* buf) {
301+
g_free(buf);
302+
g_free(buf); // BAD
296303
}

‎cpp/ql/test/query-tests/Security/CWE/CWE-193/InvalidPointerDeref.expected

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ edges
101101
| test.cpp:857:16:857:29 | ... + ... | test.cpp:857:16:857:29 | ... + ... | provenance | |
102102
| test.cpp:857:16:857:29 | ... + ... | test.cpp:860:5:860:11 | ... = ... | provenance | |
103103
| test.cpp:857:16:857:29 | ... + ... | test.cpp:860:5:860:11 | ... = ... | provenance | |
104+
| test.cpp:868:15:868:35 | call to g_malloc | test.cpp:869:15:869:22 | ... + ... | provenance | |
105+
| test.cpp:869:15:869:22 | ... + ... | test.cpp:869:15:869:22 | ... + ... | provenance | |
106+
| test.cpp:869:15:869:22 | ... + ... | test.cpp:870:14:870:15 | * ... | provenance | |
107+
| test.cpp:869:15:869:22 | ... + ... | test.cpp:870:14:870:15 | * ... | provenance | |
104108
nodes
105109
| test.cpp:4:15:4:33 | call to malloc | semmle.label | call to malloc |
106110
| test.cpp:5:15:5:22 | ... + ... | semmle.label | ... + ... |
@@ -198,6 +202,10 @@ nodes
198202
| test.cpp:857:16:857:29 | ... + ... | semmle.label | ... + ... |
199203
| test.cpp:857:16:857:29 | ... + ... | semmle.label | ... + ... |
200204
| test.cpp:860:5:860:11 | ... = ... | semmle.label | ... = ... |
205+
| test.cpp:868:15:868:35 | call to g_malloc | semmle.label | call to g_malloc |
206+
| test.cpp:869:15:869:22 | ... + ... | semmle.label | ... + ... |
207+
| test.cpp:869:15:869:22 | ... + ... | semmle.label | ... + ... |
208+
| test.cpp:870:14:870:15 | * ... | semmle.label | * ... |
201209
subpaths
202210
#select
203211
| test.cpp:6:14:6:15 | * ... | test.cpp:4:15:4:33 | call to malloc | test.cpp:6:14:6:15 | * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:4:15:4:33 | call to malloc | call to malloc | test.cpp:5:19:5:22 | size | size |
@@ -231,3 +239,4 @@ subpaths
231239
| test.cpp:842:3:842:20 | ... = ... | test.cpp:841:18:841:35 | call to malloc | test.cpp:842:3:842:20 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:841:18:841:35 | call to malloc | call to malloc | test.cpp:842:11:842:15 | index | index |
232240
| test.cpp:849:5:849:22 | ... = ... | test.cpp:848:20:848:37 | call to malloc | test.cpp:849:5:849:22 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:848:20:848:37 | call to malloc | call to malloc | test.cpp:849:13:849:17 | index | index |
233241
| test.cpp:860:5:860:11 | ... = ... | test.cpp:856:12:856:35 | call to malloc | test.cpp:860:5:860:11 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:856:12:856:35 | call to malloc | call to malloc | test.cpp:857:21:857:28 | ... + ... | ... + ... |
242+
| test.cpp:870:14:870:15 | * ... | test.cpp:868:15:868:35 | call to g_malloc | test.cpp:870:14:870:15 | * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:868:15:868:35 | call to g_malloc | call to g_malloc | test.cpp:869:19:869:22 | size | size |

‎cpp/ql/test/query-tests/Security/CWE/CWE-193/test.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,4 +859,13 @@ void test_regression(size_t size) {
859859
if(p <= chend) {
860860
*p = 42; // $ deref=L857->L860 // BAD
861861
}
862+
}
863+
864+
865+
void* g_malloc(size_t size);
866+
867+
void test17(int size) {
868+
char* p = (char*)g_malloc(size);
869+
char* q = p + size; // $ alloc=L868
870+
char a = *q; // $ deref=L869->L870 // BAD
862871
}

0 commit comments

Comments
 (0)