hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-29 10:45:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

C++: Add more memcpy, memset, strcat and strcpy models. Also refine which strcpy functions can live in the std namespace.

Browse Source
This commit is contained in:
Mathias Vorreiter Pedersen
2021-02-01 08:44:10 +01:00
parent 064d89735b
commit 6c3f44bba8
6 changed files with 168 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
cpp/ql/src/semmle/code/cpp/models/implementations/Memcpy.qll
View File

@@ -24,7 +24,8 @@ private class MemcpyFunction extends ArrayFunction, DataFlowFunction, SideEffect
or
// bcopy(src, dest, num)
// mempcpy(dest, src, num)
this.hasGlobalName(["bcopy", mempcpy(), "__builtin___memcpy_chk"])
// memccpy(dest, src, c, n)
this.hasGlobalName(["bcopy", mempcpy(), "memccpy", "__builtin___memcpy_chk"])
}
/**
@@ -41,7 +42,7 @@ private class MemcpyFunction extends ArrayFunction, DataFlowFunction, SideEffect
/**
* Gets the index of the parameter that is the size of the copy (in bytes).
*/
int getParamSize() { result = 2 }
int getParamSize() { if this.hasGlobalName("memccpy") then result = 3 else result = 2 }
override predicate hasArrayInput(int bufParam) { bufParam = getParamSrc() }
@@ -71,7 +72,10 @@ private class MemcpyFunction extends ArrayFunction, DataFlowFunction, SideEffect
override predicate hasOnlySpecificWriteSideEffects() { any() }
override predicate hasSpecificWriteSideEffect(ParameterIndex i, boolean buffer, boolean mustWrite) {
i = getParamDest() and buffer = true and mustWrite = true
i = getParamDest() and
buffer = true and
// memccpy only writes until a given character `c` is found
(if this.hasGlobalName("memccpy") then mustWrite = false else mustWrite = true)
}
override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
@@ -97,7 +101,7 @@ private class MemcpyFunction extends ArrayFunction, DataFlowFunction, SideEffect
}
override predicate parameterIsAlwaysReturned(int index) {
not this.hasGlobalName(["bcopy", mempcpy()]) and
not this.hasGlobalName(["bcopy", mempcpy(), "memccpy"]) and
index = getParamDest()
}
}

14
cpp/ql/src/semmle/code/cpp/models/implementations/Memset.qll
View File

@@ -15,7 +15,7 @@ import semmle.code.cpp.models.interfaces.SideEffect
private class MemsetFunction extends ArrayFunction, DataFlowFunction, AliasFunction,
SideEffectFunction {
MemsetFunction() {
hasGlobalName(["memset", "wmemset", "bzero", "__builtin_memset", "__builtin_memset_chk"]) or
hasGlobalName(["memset", "wmemset", bzero(), "__builtin_memset", "__builtin_memset_chk"]) or
hasQualifiedName("std", ["memset", "wmemset"])
}
@@ -28,17 +28,17 @@ private class MemsetFunction extends ArrayFunction, DataFlowFunction, AliasFunct
override predicate hasArrayWithVariableSize(int bufParam, int countParam) {
bufParam = 0 and
(if hasGlobalName("bzero") then countParam = 1 else countParam = 2)
(if hasGlobalName(bzero()) then countParam = 1 else countParam = 2)
}
override predicate parameterNeverEscapes(int index) { hasGlobalName("bzero") and index = 0 }
override predicate parameterNeverEscapes(int index) { hasGlobalName(bzero()) and index = 0 }
override predicate parameterEscapesOnlyViaReturn(int index) {
not hasGlobalName("bzero") and index = 0
not hasGlobalName(bzero()) and index = 0
}
override predicate parameterIsAlwaysReturned(int index) {
not hasGlobalName("bzero") and index = 0
not hasGlobalName(bzero()) and index = 0
}
override predicate hasOnlySpecificReadSideEffects() { any() }
@@ -51,6 +51,8 @@ private class MemsetFunction extends ArrayFunction, DataFlowFunction, AliasFunct
override ParameterIndex getParameterSizeIndex(ParameterIndex i) {
i = 0 and
if hasGlobalName("bzero") then result = 1 else result = 2
if hasGlobalName(bzero()) then result = 1 else result = 2
}
}
private string bzero() { result = ["bzero", "explicit_bzero"] }

6
cpp/ql/src/semmle/code/cpp/models/implementations/Strcat.qll
View File

@@ -21,7 +21,9 @@ class StrcatFunction extends TaintFunction, DataFlowFunction, ArrayFunction, Sid
"_mbscat", // _mbscat(dst, src)
"wcsncat", // wcsncat(dst, src, max_amount)
"_mbsncat", // _mbsncat(dst, src, max_amount)
"_mbsncat_l" // _mbsncat_l(dst, src, max_amount, locale)
"_mbsncat_l", // _mbsncat_l(dst, src, max_amount, locale)
"_mbsnbcat", // _mbsnbcat(dest, src, count)
"_mbsnbcat_l" // _mbsnbcat_l(dest, src, count, locale)
]
}
@@ -50,7 +52,7 @@ class StrcatFunction extends TaintFunction, DataFlowFunction, ArrayFunction, Sid
input.isParameter(2) and
output.isParameterDeref(0)
or
getName() = "_mbsncat_l" and
getName() = ["_mbsncat_l", "_mbsnbcat_l"] and
input.isParameter(3) and
output.isParameterDeref(0)
or

43
cpp/ql/src/semmle/code/cpp/models/implementations/Strcpy.qll
View File

@@ -13,25 +13,36 @@ import semmle.code.cpp.models.interfaces.SideEffect
*/
class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction, SideEffectFunction {
StrcpyFunction() {
getName() =
[
this.hasGlobalOrStdName([
"strcpy", // strcpy(dst, src)
"wcscpy", // wcscpy(dst, src)
"_mbscpy", // _mbscpy(dst, src)
"strncpy", // strncpy(dst, src, max_amount)
"_strncpy_l", // _strncpy_l(dst, src, max_amount, locale)
"wcsncpy", // wcsncpy(dst, src, max_amount)
"strxfrm", // strxfrm(dest, src, max_amount)
"wcsxfrm" // wcsxfrm(dest, src, max_amount)
])
or
this.hasGlobalName([
"_mbscpy", // _mbscpy(dst, src)
"_strncpy_l", // _strncpy_l(dst, src, max_amount, locale)
"_wcsncpy_l", // _wcsncpy_l(dst, src, max_amount, locale)
"_mbsncpy", // _mbsncpy(dst, src, max_amount)
"_mbsncpy_l"
] // _mbsncpy_l(dst, src, max_amount, locale)
"_mbsncpy_l", // _mbsncpy_l(dst, src, max_amount, locale)
"_strxfrm_l", // _strxfrm_l(dest, src, max_amount, locale)
"wcsxfrm_l", // _strxfrm_l(dest, src, max_amount, locale)
"_mbsnbcpy", // _mbsnbcpy(dest, src, max_amount)
"stpcpy", // stpcpy(dest, src)
"stpncpy" // stpcpy(dest, src, max_amount)
])
or
getName() =
[
"strcpy_s", // strcpy_s(dst, max_amount, src)
"wcscpy_s", // wcscpy_s(dst, max_amount, src)
"_mbscpy_s"
] and // _mbscpy_s(dst, max_amount, src)
(
this.hasGlobalOrStdName([
"strcpy_s", // strcpy_s(dst, max_amount, src)
"wcscpy_s" // wcscpy_s(dst, max_amount, src)
])
or
this.hasGlobalName("_mbscpy_s") // _mbscpy_s(dst, max_amount, src)
) and
// exclude the 2-parameter template versions
// that find the size of a fixed size destination buffer.
getNumberOfParameters() = 3
@@ -48,10 +59,10 @@ class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction, Sid
int getParamSize() {
if isSVariant()
then result = 1
else
if exists(getName().indexOf("ncpy"))
then result = 2
else none()
else (
getName().matches(["%ncpy%", "%nbcpy%", "%xfrm%"]) and
result = 2
)
}
/**

69
cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected
View File

@@ -5883,6 +5883,75 @@
| taint.cpp:510:10:510:15 | source | taint.cpp:510:2:510:8 | call to _strset | |
| taint.cpp:510:18:510:18 | 0 | taint.cpp:510:2:510:8 | call to _strset | TAINT |
| taint.cpp:510:18:510:18 | 0 | taint.cpp:510:10:510:15 | ref arg source | |
| taint.cpp:518:24:518:29 | source | taint.cpp:520:14:520:19 | source | |
| taint.cpp:519:6:519:6 | x | taint.cpp:520:11:520:11 | x | |
| taint.cpp:519:6:519:6 | x | taint.cpp:521:7:521:7 | x | |
| taint.cpp:520:10:520:11 | & ... | taint.cpp:520:2:520:8 | call to mempcpy | |
| taint.cpp:520:10:520:11 | ref arg & ... | taint.cpp:520:11:520:11 | x [inner post update] | |
| taint.cpp:520:10:520:11 | ref arg & ... | taint.cpp:521:7:521:7 | x | |
| taint.cpp:520:11:520:11 | x | taint.cpp:520:10:520:11 | & ... | |
| taint.cpp:520:14:520:19 | source | taint.cpp:520:2:520:8 | call to mempcpy | TAINT |
| taint.cpp:520:14:520:19 | source | taint.cpp:520:10:520:11 | ref arg & ... | TAINT |
| taint.cpp:528:24:528:29 | source | taint.cpp:530:16:530:21 | source | |
| taint.cpp:529:6:529:9 | dest | taint.cpp:530:10:530:13 | dest | |
| taint.cpp:529:6:529:9 | dest | taint.cpp:530:35:530:38 | dest | |
| taint.cpp:529:6:529:9 | dest | taint.cpp:531:7:531:10 | dest | |
| taint.cpp:530:10:530:13 | dest | taint.cpp:530:2:530:8 | call to memccpy | |
| taint.cpp:530:10:530:13 | ref arg dest | taint.cpp:531:7:531:10 | dest | |
| taint.cpp:530:16:530:21 | source | taint.cpp:530:2:530:8 | call to memccpy | TAINT |
| taint.cpp:530:16:530:21 | source | taint.cpp:530:10:530:13 | ref arg dest | TAINT |
| taint.cpp:538:24:538:28 | dest1 | taint.cpp:539:9:539:13 | dest1 | |
| taint.cpp:538:24:538:28 | dest1 | taint.cpp:540:7:540:11 | dest1 | |
| taint.cpp:538:37:538:41 | dest2 | taint.cpp:542:9:542:13 | dest2 | |
| taint.cpp:538:37:538:41 | dest2 | taint.cpp:543:7:543:11 | dest2 | |
| taint.cpp:538:50:538:54 | clean | taint.cpp:542:16:542:20 | clean | |
| taint.cpp:538:63:538:68 | source | taint.cpp:539:16:539:21 | source | |
| taint.cpp:539:9:539:13 | dest1 | taint.cpp:539:2:539:7 | call to strcat | |
| taint.cpp:539:9:539:13 | dest1 | taint.cpp:539:9:539:13 | ref arg dest1 | TAINT |
| taint.cpp:539:9:539:13 | ref arg dest1 | taint.cpp:540:7:540:11 | dest1 | |
| taint.cpp:539:16:539:21 | source | taint.cpp:539:9:539:13 | ref arg dest1 | TAINT |
| taint.cpp:542:9:542:13 | dest2 | taint.cpp:542:2:542:7 | call to strcat | |
| taint.cpp:542:9:542:13 | dest2 | taint.cpp:542:9:542:13 | ref arg dest2 | TAINT |
| taint.cpp:542:9:542:13 | ref arg dest2 | taint.cpp:543:7:543:11 | dest2 | |
| taint.cpp:542:16:542:20 | clean | taint.cpp:542:9:542:13 | ref arg dest2 | TAINT |
| taint.cpp:550:37:550:41 | dest1 | taint.cpp:552:36:552:40 | dest1 | |
| taint.cpp:550:37:550:41 | dest1 | taint.cpp:553:7:553:11 | dest1 | |
| taint.cpp:550:37:550:41 | dest1 | taint.cpp:554:8:554:12 | dest1 | |
| taint.cpp:550:65:550:67 | ptr | taint.cpp:552:43:552:45 | ptr | |
| taint.cpp:550:65:550:67 | ptr | taint.cpp:558:43:558:45 | ptr | |
| taint.cpp:550:85:550:89 | dest3 | taint.cpp:558:36:558:40 | dest3 | |
| taint.cpp:550:85:550:89 | dest3 | taint.cpp:559:7:559:11 | dest3 | |
| taint.cpp:550:85:550:89 | dest3 | taint.cpp:560:8:560:12 | dest3 | |
| taint.cpp:551:32:551:36 | clean | taint.cpp:558:51:558:55 | clean | |
| taint.cpp:551:49:551:54 | source | taint.cpp:552:51:552:56 | source | |
| taint.cpp:551:61:551:61 | n | taint.cpp:552:48:552:48 | n | |
| taint.cpp:551:61:551:61 | n | taint.cpp:558:48:558:48 | n | |
| taint.cpp:552:25:552:34 | call to _mbsncat_l | taint.cpp:555:7:555:11 | dest2 | |
| taint.cpp:552:25:552:34 | call to _mbsncat_l | taint.cpp:556:8:556:12 | dest2 | |
| taint.cpp:552:36:552:40 | dest1 | taint.cpp:552:25:552:34 | call to _mbsncat_l | |
| taint.cpp:552:36:552:40 | dest1 | taint.cpp:552:36:552:40 | ref arg dest1 | TAINT |
| taint.cpp:552:36:552:40 | ref arg dest1 | taint.cpp:553:7:553:11 | dest1 | |
| taint.cpp:552:36:552:40 | ref arg dest1 | taint.cpp:554:8:554:12 | dest1 | |
| taint.cpp:552:43:552:45 | ptr | taint.cpp:552:36:552:40 | ref arg dest1 | TAINT |
| taint.cpp:552:48:552:48 | n | taint.cpp:552:36:552:40 | ref arg dest1 | TAINT |
| taint.cpp:552:51:552:56 | source | taint.cpp:552:36:552:40 | ref arg dest1 | TAINT |
| taint.cpp:553:7:553:11 | ref arg dest1 | taint.cpp:554:8:554:12 | dest1 | |
| taint.cpp:554:8:554:12 | dest1 | taint.cpp:554:7:554:12 | * ... | TAINT |
| taint.cpp:555:7:555:11 | ref arg dest2 | taint.cpp:556:8:556:12 | dest2 | |
| taint.cpp:556:8:556:12 | dest2 | taint.cpp:556:7:556:12 | * ... | TAINT |
| taint.cpp:558:25:558:34 | call to _mbsncat_l | taint.cpp:561:7:561:11 | dest4 | |
| taint.cpp:558:25:558:34 | call to _mbsncat_l | taint.cpp:562:8:562:12 | dest4 | |
| taint.cpp:558:36:558:40 | dest3 | taint.cpp:558:25:558:34 | call to _mbsncat_l | |
| taint.cpp:558:36:558:40 | dest3 | taint.cpp:558:36:558:40 | ref arg dest3 | TAINT |
| taint.cpp:558:36:558:40 | ref arg dest3 | taint.cpp:559:7:559:11 | dest3 | |
| taint.cpp:558:36:558:40 | ref arg dest3 | taint.cpp:560:8:560:12 | dest3 | |
| taint.cpp:558:43:558:45 | ptr | taint.cpp:558:36:558:40 | ref arg dest3 | TAINT |
| taint.cpp:558:48:558:48 | n | taint.cpp:558:36:558:40 | ref arg dest3 | TAINT |
| taint.cpp:558:51:558:55 | clean | taint.cpp:558:36:558:40 | ref arg dest3 | TAINT |
| taint.cpp:559:7:559:11 | ref arg dest3 | taint.cpp:560:8:560:12 | dest3 | |
| taint.cpp:560:8:560:12 | dest3 | taint.cpp:560:7:560:12 | * ... | TAINT |
| taint.cpp:561:7:561:11 | ref arg dest4 | taint.cpp:562:8:562:12 | dest4 | |
| taint.cpp:562:8:562:12 | dest4 | taint.cpp:562:7:562:12 | * ... | TAINT |
| vector.cpp:16:43:16:49 | source1 | vector.cpp:17:26:17:32 | source1 | |
| vector.cpp:16:43:16:49 | source1 | vector.cpp:31:38:31:44 | source1 | |
| vector.cpp:17:21:17:33 | call to vector | vector.cpp:19:14:19:14 | v | |

53
cpp/ql/test/library-tests/dataflow/taint-tests/taint.cpp
View File

@@ -509,4 +509,55 @@ void test_strset_1(char* ptr, char source) {
void test_strset_2(char* source) {
_strset(source, 0);
sink(source); // $ ast,ir
}
}
// --- mempcpy ---
void *mempcpy(void *dest, const void *src, size_t n);
void test_mempcpy(int *source) {
int x;
mempcpy(&x, source, sizeof(int));
sink(x); // $ ast=518:24 MISSING: ir SPURIOUS: ast=519:6
}
// --- memccpy ---
void *memccpy(void *dest, const void *src, int c, size_t n);
void test_memccpy(int *source) {
int dest[16];
memccpy(dest, source, 42, sizeof(dest));
sink(dest); // $ ast=528:24 MISSING: ir SPURIOUS: ast=529:6
}
// --- strcat and related functions ---
char* strcat (char*, const char*);
void test_strcat(char* dest1, char* dest2, char* clean, char* source) {
strcat(dest1, source);
sink(dest1); // $ ast,ir
strcat(dest2, clean);
sink(dest2);
}
typedef void* _locale_t;
unsigned char *_mbsncat_l(unsigned char *, const unsigned char *, int, _locale_t);
void test__mbsncat_l(unsigned char* dest1, unsigned const char* ptr, unsigned char* dest3,
_locale_t clean, _locale_t source, int n) {
unsigned char* dest2 = _mbsncat_l(dest1, ptr, n, source);
sink(dest1); // $ SPURIOUS: ast,ir
sink(*dest1); // $ ast,ir
sink(dest2); // $ SPURIOUS: ir
sink(*dest2); // $ ir
unsigned char* dest4 = _mbsncat_l(dest3, ptr, n, clean);
sink(dest3);
sink(*dest3);
sink(dest4);
sink(*dest4);
}