Skip to content

Commit 18bb669

Browse files
Fixing conditional only issue.
I changed to detect any logical operation usage (i.e. !, ==), but I kept usage in a conditional directly as a separate detection condition. I found no false positives on the projects you shared with me previously.
1 parent 880306c commit 18bb669

File tree

4 files changed

+63
-32
lines changed

4 files changed

+63
-32
lines changed

cpp/ql/src/Likely Bugs/Likely Typos/UsingStrcpyAsBoolean.ql

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,11 @@ predicate isStringCopyCastedAsBoolean( FunctionCall func, Expr expr1, string msg
4040
and msg = "Return Value of " + func.getTarget().getQualifiedName() + " used as boolean."
4141
}
4242

43-
predicate isStringCopyUsedInCondition( FunctionCall func, Expr expr1, string msg ) {
44-
exists( ConditionalStmt condstmt |
45-
condstmt.getAChild() = expr1 |
43+
predicate isStringCopyUsedInLogicalOperationOrCondition( FunctionCall func, Expr expr1, string msg ) {
4644
isStringComparisonFunction( func.getTarget().getQualifiedName() )
47-
and (
48-
// The string copy function is used directly as the conditional expression
49-
func = condstmt.getChild(0)
50-
// ... or it is being used in an equality or logical operation
51-
or exists( EqualityOperation eop |
45+
and (((
46+
// it is being used in an equality or logical operation
47+
exists( EqualityOperation eop |
5248
eop = expr1
5349
and func = eop.getAChild()
5450
)
@@ -61,14 +57,21 @@ predicate isStringCopyUsedInCondition( FunctionCall func, Expr expr1, string msg
6157
and func = ble.getAChild()
6258
)
6359
)
64-
and msg = "Return Value of " + func.getTarget().getQualifiedName() + " used in a conditional."
65-
)
60+
and msg = "Return Value of " + func.getTarget().getQualifiedName() + " used in a logical operation."
61+
)
62+
or
63+
exists( ConditionalStmt condstmt |
64+
condstmt.getAChild() = expr1 |
65+
// or the string copy function is used directly as the conditional expression
66+
func = condstmt.getChild(0)
67+
and msg = "Return Value of " + func.getTarget().getQualifiedName() + " used directly in a conditional expression."
68+
))
6669
}
6770

6871
from FunctionCall func, Expr expr1, string msg
6972
where
7073
( isStringCopyCastedAsBoolean(func, expr1, msg) and
71-
not isStringCopyUsedInCondition(func, expr1, _)
74+
not isStringCopyUsedInLogicalOperationOrCondition(func, expr1, _)
7275
)
73-
or isStringCopyUsedInCondition(func, expr1, msg)
76+
or isStringCopyUsedInLogicalOperationOrCondition(func, expr1, msg)
7477
select expr1, msg
Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,34 @@
1-
| test.c:33:9:33:14 | call to strcpy | Return Value of strcpy used in a conditional. |
2-
| test.c:37:9:37:31 | ! ... | Return Value of strcpy used in a conditional. |
3-
| test.c:41:9:41:35 | ... == ... | Return Value of strcpy used in a conditional. |
4-
| test.c:45:9:45:48 | ... && ... | Return Value of strcpy used in a conditional. |
5-
| test.c:49:9:49:15 | call to strncpy | Return Value of strncpy used in a conditional. |
6-
| test.c:53:6:53:34 | ! ... | Return Value of strncpy used in a conditional. |
7-
| test.cpp:75:9:75:14 | call to strcpy | Return Value of strcpy used in a conditional. |
8-
| test.cpp:79:9:79:31 | ! ... | Return Value of strcpy used in a conditional. |
1+
| test.c:34:9:34:14 | call to strcpy | Return Value of strcpy used directly in a conditional expression. |
2+
| test.c:38:9:38:31 | ! ... | Return Value of strcpy used in a logical operation. |
3+
| test.c:42:9:42:35 | ... == ... | Return Value of strcpy used in a logical operation. |
4+
| test.c:46:9:46:48 | ... && ... | Return Value of strcpy used in a logical operation. |
5+
| test.c:50:9:50:15 | call to strncpy | Return Value of strncpy used directly in a conditional expression. |
6+
| test.c:54:6:54:34 | ! ... | Return Value of strncpy used in a logical operation. |
7+
| test.c:58:11:58:39 | ! ... | Return Value of strncpy used in a logical operation. |
8+
| test.c:60:11:60:37 | ... && ... | Return Value of strcpy used in a logical operation. |
9+
| test.c:62:11:62:37 | ... == ... | Return Value of strcpy used in a logical operation. |
10+
| test.c:64:11:64:37 | ... != ... | Return Value of strcpy used in a logical operation. |
11+
| test.cpp:75:9:75:14 | call to strcpy | Return Value of strcpy used directly in a conditional expression. |
12+
| test.cpp:79:9:79:31 | ! ... | Return Value of strcpy used in a logical operation. |
913
| test.cpp:79:10:79:15 | call to strcpy | Return Value of strcpy used as boolean. |
10-
| test.cpp:83:9:83:35 | ... == ... | Return Value of strcpy used in a conditional. |
11-
| test.cpp:87:9:87:48 | ... && ... | Return Value of strcpy used in a conditional. |
14+
| test.cpp:83:9:83:35 | ... == ... | Return Value of strcpy used in a logical operation. |
15+
| test.cpp:87:9:87:48 | ... && ... | Return Value of strcpy used in a logical operation. |
1216
| test.cpp:87:27:87:32 | call to strcpy | Return Value of strcpy used as boolean. |
13-
| test.cpp:91:9:91:37 | call to wcscpy | Return Value of wcscpy used in a conditional. |
14-
| test.cpp:95:9:95:14 | call to wcscpy | Return Value of wcscpy used in a conditional. |
15-
| test.cpp:99:9:99:15 | call to _mbscpy | Return Value of _mbscpy used in a conditional. |
16-
| test.cpp:103:9:103:15 | call to strncpy | Return Value of strncpy used in a conditional. |
17-
| test.cpp:107:9:107:15 | call to wcsncpy | Return Value of wcsncpy used in a conditional. |
18-
| test.cpp:111:9:111:16 | call to _mbsncpy | Return Value of _mbsncpy used in a conditional. |
19-
| test.cpp:115:9:115:18 | call to _strncpy_l | Return Value of _strncpy_l used in a conditional. |
20-
| test.cpp:119:9:119:18 | call to _wcsncpy_l | Return Value of _wcsncpy_l used in a conditional. |
21-
| test.cpp:123:9:123:18 | call to _mbsncpy_l | Return Value of _mbsncpy_l used in a conditional. |
22-
| test.cpp:127:6:127:34 | ! ... | Return Value of strncpy used in a conditional. |
17+
| test.cpp:91:9:91:37 | call to wcscpy | Return Value of wcscpy used directly in a conditional expression. |
18+
| test.cpp:95:9:95:14 | call to wcscpy | Return Value of wcscpy used directly in a conditional expression. |
19+
| test.cpp:99:9:99:15 | call to _mbscpy | Return Value of _mbscpy used directly in a conditional expression. |
20+
| test.cpp:103:9:103:15 | call to strncpy | Return Value of strncpy used directly in a conditional expression. |
21+
| test.cpp:107:9:107:15 | call to wcsncpy | Return Value of wcsncpy used directly in a conditional expression. |
22+
| test.cpp:111:9:111:16 | call to _mbsncpy | Return Value of _mbsncpy used directly in a conditional expression. |
23+
| test.cpp:115:9:115:18 | call to _strncpy_l | Return Value of _strncpy_l used directly in a conditional expression. |
24+
| test.cpp:119:9:119:18 | call to _wcsncpy_l | Return Value of _wcsncpy_l used directly in a conditional expression. |
25+
| test.cpp:123:9:123:18 | call to _mbsncpy_l | Return Value of _mbsncpy_l used directly in a conditional expression. |
26+
| test.cpp:127:6:127:34 | ! ... | Return Value of strncpy used in a logical operation. |
2327
| test.cpp:127:7:127:13 | call to strncpy | Return Value of strncpy used as boolean. |
2428
| test.cpp:131:11:131:17 | call to strncpy | Return Value of strncpy used as boolean. |
29+
| test.cpp:133:16:133:44 | ! ... | Return Value of strncpy used in a logical operation. |
30+
| test.cpp:133:17:133:23 | call to strncpy | Return Value of strncpy used as boolean. |
31+
| test.cpp:135:11:135:16 | call to strcpy | Return Value of strcpy used as boolean. |
32+
| test.cpp:135:11:135:37 | ... && ... | Return Value of strcpy used in a logical operation. |
33+
| test.cpp:137:11:137:37 | ... == ... | Return Value of strcpy used in a logical operation. |
34+
| test.cpp:139:11:139:37 | ... != ... | Return Value of strcpy used in a logical operation. |

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/UsingStrcpyAsBoolean/test.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ void PositiveCases()
2929
{
3030
char szbuf1[100];
3131
char szbuf2[100];
32+
int result;
3233

3334
if (strcpy(szbuf1, "test")) // Bug, direct usage
3435
{
@@ -53,6 +54,14 @@ void PositiveCases()
5354
if (!strncpy(szbuf1, "test", 100)) // Bug
5455
{
5556
}
57+
58+
result = !strncpy(szbuf1, "test", 100);
59+
60+
result = strcpy(szbuf1, "test") && 1;
61+
62+
result = strcpy(szbuf1, "test") == 0;
63+
64+
result = strcpy(szbuf1, "test") != 0;
5665
}
5766

5867
void NegativeCases()

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/UsingStrcpyAsBoolean/test.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,15 @@ void PositiveCases()
129129
}
130130

131131
bool b = strncpy(szbuf1, "test", 100);
132+
133+
bool result = !strncpy(szbuf1, "test", 100);
134+
135+
result = strcpy(szbuf1, "test") && 1;
136+
137+
result = strcpy(szbuf1, "test") == 0;
138+
139+
result = strcpy(szbuf1, "test") != 0;
140+
132141
}
133142

134143
void NegativeCases()

0 commit comments

Comments
 (0)