Skip to content

Commit 44752d5

Browse files
committed
C++: Model strdupa and strndupa string functions returning memory allocated with alloca
1 parent 21d03cd commit 44752d5

File tree

4 files changed

+43
-6
lines changed

4 files changed

+43
-6
lines changed

cpp/ql/lib/semmle/code/cpp/models/implementations/Strdup.qll

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ private class StrdupFunction extends AllocationFunction, ArrayFunction, DataFlow
1616
hasGlobalName([
1717
// --- C library allocation
1818
"strdup", // strdup(str)
19+
"strdupa", // strdupa(str) - returns stack allocated buffer
1920
"wcsdup", // wcsdup(str)
2021
"_strdup", // _strdup(str)
2122
"_wcsdup", // _wcsdup(str)
@@ -31,18 +32,20 @@ private class StrdupFunction extends AllocationFunction, ArrayFunction, DataFlow
3132
input.isParameterDeref(0) and
3233
output.isReturnValueDeref()
3334
}
35+
36+
override predicate requiresDealloc() { not hasGlobalName("strdupa") }
3437
}
3538

3639
/**
3740
* A `strndup` style allocation function.
3841
*/
3942
private class StrndupFunction extends AllocationFunction, ArrayFunction, DataFlowFunction {
4043
StrndupFunction() {
41-
exists(string name |
42-
hasGlobalName(name) and
43-
// --- C library allocation
44-
name = "strndup" // strndup(str, maxlen)
45-
)
44+
hasGlobalName([
45+
// -- C library allocation
46+
"strndup", // strndup(str, maxlen)
47+
"strndupa" // strndupa(str, maxlen) -- returns stack allocated buffer
48+
])
4649
}
4750

4851
override predicate hasArrayInput(int bufParam) { bufParam = 0 }
@@ -56,4 +59,6 @@ private class StrndupFunction extends AllocationFunction, ArrayFunction, DataFlo
5659
) and
5760
output.isReturnValueDeref()
5861
}
62+
63+
override predicate requiresDealloc() { not hasGlobalName("strndupa") }
5964
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
| test.cpp:126:8:126:9 | i2 |
2525
| test.cpp:127:8:127:9 | i3 |
2626
| test.cpp:128:15:128:16 | v4 |
27+
| test.cpp:185:10:185:12 | cpy |
28+
| test.cpp:199:10:199:12 | cpy |
2729
| virtual.cpp:18:10:18:10 | a |
2830
| virtual.cpp:19:10:19:10 | c |
2931
| virtual.cpp:38:10:38:10 | b |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@
1010
| test.cpp:89:18:89:23 | call to malloc | This memory is never freed |
1111
| test.cpp:156:3:156:26 | new | This memory is never freed |
1212
| test.cpp:157:3:157:26 | new[] | This memory is never freed |
13-
| test.cpp:167:14:167:19 | call to strdup | This memory is never freed |
13+
| test.cpp:169:14:169:19 | call to strdup | This memory is never freed |

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ int overloadedNew() {
155155

156156
new(std::nothrow) int(3); // BAD
157157
new(std::nothrow) int[2]; // BAD
158+
159+
return 0;
158160
}
159161

160162
// --- strdup ---
@@ -168,3 +170,31 @@ void test_strdup() {
168170

169171
output_msg(cpy);
170172
}
173+
174+
// --- strdupa ---
175+
char *strdupa(const char *s1);
176+
177+
void test_strdupa_no_dealloc() {
178+
char msg[] = "OctoCat";
179+
char *cpy = strdupa(msg); // GOOD
180+
}
181+
182+
void test_strdupa_dealloc() {
183+
char msg[] = "OctoCat";
184+
char *cpy = strdupa(msg);
185+
free(cpy); // BAD [NOT DETECTED]
186+
}
187+
188+
// --- strndupa ---
189+
char *strndupa(const char *s1, size_t maxsize);
190+
191+
void test_strndupa_no_dealloc() {
192+
char msg[] = "OctoCat";
193+
char *cpy = strndupa(msg, 4); // GOOD
194+
}
195+
196+
void test_strndupa_dealloc() {
197+
char msg[] = "OctoCat";
198+
char *cpy = strndupa(msg, 4);
199+
free(cpy); // BAD [NOT DETECTED]
200+
}

0 commit comments

Comments
 (0)