Skip to content

Removing high FP case in IncorrectUsageOfRtlCompareMemory, altering t… #328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
bdrodes wants to merge 1 commit into
base: main
Choose a base branch
from
+1 −10
Draft

Removing high FP case in IncorrectUsageOfRtlCompareMemory, altering t… #328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions cpp/ql/src/Microsoft/Likely Bugs/Drivers/IncorrectUsageOfRtlCompareMemory.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,6 @@ where
msg =
"as an operand in an equality operation where the other operand is a boolean value (high precision result)"
)
or
exists(EqualityOperation bao, Expr e2 | e = bao |
bao.hasOperands(fc, e2) and
(e2.(Literal).getValue().toInt() = 1 or e2.(Literal).getValue().toInt() = 0) and
not isLiteralABooleanMacro(e2) and
msg =
"as an operand in an equality operation where the other operand is likely a boolean value (lower precision result, needs to be reviewed)"
)
)
select e,
"This $@ is being handled $@ instead of the number of matching bytes. Please review the usage of this function and consider replacing it with `RtlEqualMemory`.",
Expand Down
1 change: 0 additions & 1 deletion .../test/query-tests/Microsoft/Likely Bugs/Drivers/IncorrectUsageOfRtlCompareMemory.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
| test.c:29:6:29:46 | ... && ... | This $@ is being handled $@ instead of the number of matching bytes. Please review the usage of this function and consider replacing it with `RtlEqualMemory`. | test.c:29:15:29:30 | call to RtlCompareMemory | call to `RtlCompareMemory` | test.c:29:6:29:46 | ... && ... | as an operand in a binary logical operation |
| test.c:34:6:34:38 | ! ... | This $@ is being handled $@ instead of the number of matching bytes. Please review the usage of this function and consider replacing it with `RtlEqualMemory`. | test.c:34:7:34:22 | call to RtlCompareMemory | call to `RtlCompareMemory` | test.c:34:6:34:38 | ! ... | as an operand in an unary logical operation |
| test.c:39:6:39:21 | call to RtlCompareMemory | This $@ is being handled $@ instead of the number of matching bytes. Please review the usage of this function and consider replacing it with `RtlEqualMemory`. | test.c:39:6:39:21 | call to RtlCompareMemory | call to `RtlCompareMemory` | test.c:39:6:39:21 | call to RtlCompareMemory | as the controlling expression in an If statement |
| test.c:49:6:49:42 | ... == ... | This $@ is being handled $@ instead of the number of matching bytes. Please review the usage of this function and consider replacing it with `RtlEqualMemory`. | test.c:49:11:49:26 | call to RtlCompareMemory | call to `RtlCompareMemory` | test.c:49:6:49:42 | ... == ... | as an operand in an equality operation where the other operand is likely a boolean value (lower precision result, needs to be reviewed) |
| test.c:75:6:75:37 | (bool)... | This $@ is being handled $@ instead of the number of matching bytes. Please review the usage of this function and consider replacing it with `RtlEqualMemory`. | test.c:75:6:75:21 | call to RtlCompareMemory | call to `RtlCompareMemory` | test.c:75:6:75:37 | (bool)... | as a boolean |
| test.c:77:6:77:46 | ... == ... | This $@ is being handled $@ instead of the number of matching bytes. Please review the usage of this function and consider replacing it with `RtlEqualMemory`. | test.c:77:15:77:30 | call to RtlCompareMemory | call to `RtlCompareMemory` | test.c:77:6:77:46 | ... == ... | as an operand in an equality operation where the other operand is a boolean value (high precision result) |
| test.c:84:6:84:37 | (BOOLEAN)... | This $@ is being handled $@ instead of the number of matching bytes. Please review the usage of this function and consider replacing it with `RtlEqualMemory`. | test.c:84:6:84:21 | call to RtlCompareMemory | call to `RtlCompareMemory` | test.c:84:6:84:37 | (BOOLEAN)... | as a boolean |
Expand Down
2 changes: 1 addition & 1 deletion cpp/ql/test/query-tests/Microsoft/Likely Bugs/Drivers/test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ int Test(const void* ptr)
t++;
}

if (0 == RtlCompareMemory("test", ptr, 4)) // potentially a bug (lower precision)
if (0 == RtlCompareMemory("test", ptr, 4)) // FALSE NEGATIVE: potentially a bug but results in too many false positives (lower precision, perhaps != 0 is a good case but == 0 isn't?)
{
t++;
}
Expand Down