diff --git a/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.ql b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.ql index f7eca2304b3..a3a2c5b83be 100644 --- a/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.ql +++ b/cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.ql @@ -18,6 +18,7 @@ import cpp import Buffer private import semmle.code.cpp.valuenumbering.GlobalValueNumbering +private import semmle.code.cpp.models.implementations.Strcpy predicate isSizePlus(Expr e, BufferSizeExpr baseSize, int plus) { // baseSize @@ -41,33 +42,6 @@ predicate isSizePlus(Expr e, BufferSizeExpr baseSize, int plus) { ) } -predicate strncpyFunction(Function f, int argDest, int argSrc, int argLimit) { - exists(string name | name = f.getName() | - name = - [ - "strcpy_s", // strcpy_s(dst, max_amount, src) - "wcscpy_s", // wcscpy_s(dst, max_amount, src) - "_mbscpy_s" // _mbscpy_s(dst, max_amount, src) - ] and - argDest = 0 and - argSrc = 2 and - argLimit = 1 - or - name = - [ - "strncpy", // strncpy(dst, src, max_amount) - "strncpy_l", // strncpy_l(dst, src, max_amount, locale) - "wcsncpy", // wcsncpy(dst, src, max_amount) - "_wcsncpy_l", // _wcsncpy_l(dst, src, max_amount, locale) - "_mbsncpy", // _mbsncpy(dst, src, max_amount) - "_mbsncpy_l" // _mbsncpy_l(dst, src, max_amount, locale) - ] and - argDest = 0 and - argSrc = 1 and - argLimit = 2 - ) -} - string nthString(int num) { num = 0 and result = "first" @@ -96,11 +70,13 @@ int arrayExprFixedSize(Expr e) { } from - Function f, FunctionCall fc, int argDest, int argSrc, int argLimit, int charSize, Access copyDest, - Access copySource, string name, string nth + StrcpyFunction f, FunctionCall fc, int argDest, int argSrc, int argLimit, int charSize, + Access copyDest, Access copySource, string name, string nth where f = fc.getTarget() and - strncpyFunction(f, argDest, argSrc, argLimit) and + argDest = f.getParamDest() and + argSrc = f.getParamSrc() and + argLimit = f.getParamSize() and copyDest = fc.getArgument(argDest) and copySource = fc.getArgument(argSrc) and // Some of the functions operate on a larger char type, like `wchar_t`, so we diff --git a/cpp/ql/src/change-notes/2021-08-10-use-strcpyfunction-in-bad-strncpy-size.md b/cpp/ql/src/change-notes/2021-08-10-use-strcpyfunction-in-bad-strncpy-size.md new file mode 100644 index 00000000000..3468fec4c8d --- /dev/null +++ b/cpp/ql/src/change-notes/2021-08-10-use-strcpyfunction-in-bad-strncpy-size.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The query `cpp/bad-strncpy-size` now covers more `strncpy`-like functions than before, including `strxfrm`(`_l`), `wcsxfrm`(`_l`), and `stpncpy`. Users of this query may see an increase in results. diff --git a/cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/StrncpyFlippedArgs.expected b/cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/StrncpyFlippedArgs.expected index 88a953b0196..1fe46acbdde 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/StrncpyFlippedArgs.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/StrncpyFlippedArgs.expected @@ -1,18 +1,23 @@ | test.c:22:2:22:8 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | | test.c:33:2:33:8 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | -| test.cpp:19:2:19:8 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | -| test.cpp:20:2:20:8 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | | test.cpp:21:2:21:8 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | -| test.cpp:30:2:30:8 | call to wcsncpy | Potentially unsafe call to wcsncpy; third argument should be size of destination. | +| test.cpp:22:2:22:8 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | +| test.cpp:23:2:23:8 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | | test.cpp:32:2:32:8 | call to wcsncpy | Potentially unsafe call to wcsncpy; third argument should be size of destination. | -| test.cpp:33:2:33:8 | call to wcsncpy | Potentially unsafe call to wcsncpy; third argument should be size of destination. | | test.cpp:34:2:34:8 | call to wcsncpy | Potentially unsafe call to wcsncpy; third argument should be size of destination. | | test.cpp:35:2:35:8 | call to wcsncpy | Potentially unsafe call to wcsncpy; third argument should be size of destination. | -| test.cpp:45:2:45:9 | call to strcpy_s | Potentially unsafe call to strcpy_s; second argument should be size of destination. | -| test.cpp:46:2:46:9 | call to strcpy_s | Potentially unsafe call to strcpy_s; second argument should be size of destination. | +| test.cpp:36:2:36:8 | call to wcsncpy | Potentially unsafe call to wcsncpy; third argument should be size of destination. | +| test.cpp:37:2:37:8 | call to wcsncpy | Potentially unsafe call to wcsncpy; third argument should be size of destination. | | test.cpp:47:2:47:9 | call to strcpy_s | Potentially unsafe call to strcpy_s; second argument should be size of destination. | -| test.cpp:60:3:60:9 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | -| test.cpp:63:3:63:9 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | -| test.cpp:68:2:68:8 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | -| test.cpp:79:3:79:9 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | -| test.cpp:82:3:82:9 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | +| test.cpp:48:2:48:9 | call to strcpy_s | Potentially unsafe call to strcpy_s; second argument should be size of destination. | +| test.cpp:49:2:49:9 | call to strcpy_s | Potentially unsafe call to strcpy_s; second argument should be size of destination. | +| test.cpp:62:3:62:9 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | +| test.cpp:65:3:65:9 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | +| test.cpp:70:2:70:8 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | +| test.cpp:81:3:81:9 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | +| test.cpp:84:3:84:9 | call to strncpy | Potentially unsafe call to strncpy; third argument should be size of destination. | +| test.cpp:105:2:105:10 | call to wcsxfrm_l | Potentially unsafe call to wcsxfrm_l; third argument should be size of destination. | +| test.cpp:107:2:107:10 | call to wcsxfrm_l | Potentially unsafe call to wcsxfrm_l; third argument should be size of destination. | +| test.cpp:108:2:108:10 | call to wcsxfrm_l | Potentially unsafe call to wcsxfrm_l; third argument should be size of destination. | +| test.cpp:109:2:109:10 | call to wcsxfrm_l | Potentially unsafe call to wcsxfrm_l; third argument should be size of destination. | +| test.cpp:110:2:110:10 | call to wcsxfrm_l | Potentially unsafe call to wcsxfrm_l; third argument should be size of destination. | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/test.cpp b/cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/test.cpp index b31e8762467..ad2e39b748e 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/test.cpp +++ b/cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/test.cpp @@ -1,9 +1,11 @@ typedef unsigned int size_t; typedef unsigned int errno_t; +typedef void *locale_t; char *strncpy(char *__restrict destination, const char *__restrict source, size_t num); wchar_t *wcsncpy(wchar_t *__restrict destination, const wchar_t *__restrict source, size_t num); +size_t wcsxfrm_l(wchar_t *ws1, const wchar_t *ws2, size_t n, locale_t locale); errno_t strcpy_s(char *strDestination, size_t numberOfElements, const char *strSource); size_t strlen(const char *str); @@ -93,3 +95,17 @@ void test8(char x[], char y[]) { // that it will be a false positive if we report it. strncpy(x, y, 32); } + +void test9() +{ + wchar_t buf1[10]; + wchar_t buf2[20]; + const wchar_t *str = L"01234567890123456789"; + + wcsxfrm_l(buf1, str, sizeof(buf1), nullptr); // BAD (but not a StrncpyFlippedArgs bug) + wcsxfrm_l(buf1, str, sizeof(buf1) / sizeof(wchar_t), nullptr); // GOOD + wcsxfrm_l(buf1, str, wcslen(str), nullptr); // BAD + wcsxfrm_l(buf1, str, wcslen(str) + 1, nullptr); // BAD + wcsxfrm_l(buf1, buf2, sizeof(buf2), nullptr); // BAD + wcsxfrm_l(buf1, buf2, sizeof(buf2) / sizeof(wchar_t), nullptr); // BAD +}