Skip to content

Commit df41900

Browse files
committed
Use Strcpy.qll in StrncpyFlippedArgs.ql
As a result, the query gets access to more types of strncpy-like functions, as demonstrated by test.cpp, which now "fails" (i.e. works) for the new test cases instroduced in the previous commit.
1 parent 554aea1 commit df41900

File tree

2 files changed

+8
-32
lines changed

2 files changed

+8
-32
lines changed

cpp/ql/src/Likely Bugs/Memory Management/StrncpyFlippedArgs.ql

Lines changed: 5 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import cpp
1919
import Buffer
2020
private import semmle.code.cpp.valuenumbering.GlobalValueNumbering
21+
private import semmle.code.cpp.models.implementations.Strcpy
2122

2223
predicate isSizePlus(Expr e, BufferSizeExpr baseSize, int plus) {
2324
// baseSize
@@ -41,33 +42,6 @@ predicate isSizePlus(Expr e, BufferSizeExpr baseSize, int plus) {
4142
)
4243
}
4344

44-
predicate strncpyFunction(Function f, int argDest, int argSrc, int argLimit) {
45-
exists(string name | name = f.getName() |
46-
name =
47-
[
48-
"strcpy_s", // strcpy_s(dst, max_amount, src)
49-
"wcscpy_s", // wcscpy_s(dst, max_amount, src)
50-
"_mbscpy_s" // _mbscpy_s(dst, max_amount, src)
51-
] and
52-
argDest = 0 and
53-
argSrc = 2 and
54-
argLimit = 1
55-
or
56-
name =
57-
[
58-
"strncpy", // strncpy(dst, src, max_amount)
59-
"strncpy_l", // strncpy_l(dst, src, max_amount, locale)
60-
"wcsncpy", // wcsncpy(dst, src, max_amount)
61-
"_wcsncpy_l", // _wcsncpy_l(dst, src, max_amount, locale)
62-
"_mbsncpy", // _mbsncpy(dst, src, max_amount)
63-
"_mbsncpy_l" // _mbsncpy_l(dst, src, max_amount, locale)
64-
] and
65-
argDest = 0 and
66-
argSrc = 1 and
67-
argLimit = 2
68-
)
69-
}
70-
7145
string nthString(int num) {
7246
num = 0 and
7347
result = "first"
@@ -96,11 +70,13 @@ int arrayExprFixedSize(Expr e) {
9670
}
9771

9872
from
99-
Function f, FunctionCall fc, int argDest, int argSrc, int argLimit, int charSize, Access copyDest,
73+
StrcpyFunction f, FunctionCall fc, int argDest, int argSrc, int argLimit, int charSize, Access copyDest,
10074
Access copySource, string name, string nth
10175
where
10276
f = fc.getTarget() and
103-
strncpyFunction(f, argDest, argSrc, argLimit) and
77+
argDest = f.getParamDest() and
78+
argSrc = f.getParamSrc() and
79+
argLimit = f.getParamSize() and
10480
copyDest = fc.getArgument(argDest) and
10581
copySource = fc.getArgument(argSrc) and
10682
// Some of the functions operate on a larger char type, like `wchar_t`, so we

cpp/ql/test/query-tests/Likely Bugs/Memory Management/StrncpyFlippedArgs/test.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,10 @@ void test9()
102102
wchar_t buf2[20];
103103
const wchar_t *str = L"01234567890123456789";
104104

105-
wcsxfrm_l(buf1, str, sizeof(buf1), nullptr); // (bad, but not a strncpyflippedargs bug)
106-
wcsxfrm_l(buf1, str, sizeof(buf1) / sizeof(wchar_t), nullptr);
105+
wcsxfrm_l(buf1, str, sizeof(buf1), nullptr); // BAD (but not a StrncpyFlippedArgs bug)
106+
wcsxfrm_l(buf1, str, sizeof(buf1) / sizeof(wchar_t), nullptr); // GOOD
107107
wcsxfrm_l(buf1, str, wcslen(str), nullptr); // BAD
108108
wcsxfrm_l(buf1, str, wcslen(str) + 1, nullptr); // BAD
109109
wcsxfrm_l(buf1, buf2, sizeof(buf2), nullptr); // BAD
110-
wcsxfrm_l(buf1, buf2, sizeof(buf2) / sizeof(wchar_t), nullptr); // BAD [NOT DETECTED]
110+
wcsxfrm_l(buf1, buf2, sizeof(buf2) / sizeof(wchar_t), nullptr); // BAD
111111
}

0 commit comments

Comments
 (0)