-
Notifications
You must be signed in to change notification settings - Fork 31
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
8ce22c1
commit aa5a9fc
Showing
4 changed files
with
450 additions
and
0 deletions.
There are no files selected for viewing
37 changes: 37 additions & 0 deletions
37
src/drivers/general/queries/ImproperNotOperatorOnZero/ImproperNotOperatorOnZero.qhelp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd"> | ||
<qhelp> | ||
<overview> | ||
<p> | ||
The type for which !0 is being used does not treat it as failure case. Returning a status value such as !TRUE is not the same as returning a status value that indicates failure. | ||
</p> | ||
</overview> | ||
<recommendation> | ||
<p> | ||
Certain data types such as NTSTATUS and HRESULT have associated macros that classify values of these types into SUCCESS or FAILURE. These macros check the most significant bit of the returned value or values to determine this. Thus, 0 and 1 are both classified as SUCCESS values. | ||
The proper way to fix this warning is to return a proper error code instead of a generic value such as -1. | ||
</p> | ||
</recommendation> | ||
<example> | ||
<p> | ||
Returning !0 instead of a proper error code. | ||
</p> | ||
<sample language="c"> <![CDATA[ | ||
NTSTATUS test_func() | ||
{ | ||
return !0; | ||
} | ||
}]]> | ||
</sample> | ||
</example> | ||
<semmleNotes> | ||
<p> | ||
</p> | ||
</semmleNotes> | ||
<references> | ||
<li> | ||
<a href="https://learn.microsoft.com/en-us/windows-hardware/drivers/devtest/28650-generic-value-is-not-treated-as-failure"> | ||
C28650 | ||
</a> | ||
</li> | ||
</references> | ||
</qhelp> |
31 changes: 31 additions & 0 deletions
31
src/drivers/general/queries/ImproperNotOperatorOnZero/ImproperNotOperatorOnZero.ql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
// Copyright (c) Microsoft Corporation. | ||
// Licensed under the MIT license. | ||
/** | ||
* @id cpp/drivers/improper-not-operator-on-zero | ||
* @kind problem | ||
* @name Improper Not Operator On Zero | ||
* @description The type for which !0 is being used does not treat it as failure case. | ||
* @platform Desktop | ||
* @feature.area Multiple | ||
* @impact Insecure Coding Practice | ||
* @repro.text Returning a status value such as !TRUE is not the same as returning a status value that indicates failure. | ||
* @owner.email: [email protected] | ||
* @opaqueid CQLD-TODO | ||
* @problem.severity warning | ||
* @precision medium | ||
* @tags correctness | ||
* @scope domainspecific | ||
* @query-version v1 | ||
*/ | ||
|
||
import cpp | ||
import semmle.code.cpp.ir.IR | ||
|
||
from NotExpr ne, Conversion c, ReturnStmt rs | ||
where | ||
c.getType().toString().matches(["NTSTATUS", "HRESULT"]) and | ||
c.getUnconverted().getType() instanceof IntegralType and | ||
ne.getConversion() = c and | ||
ne.getOperand().getValue() = ["0", "1", "TRUE", "FALSE", "-1"] and | ||
rs.getAChild*() = ne | ||
select c, "Improper use of the not operator on return value" |
Oops, something went wrong.