Skip to content

C++: Finalise docs for cpp/hresult-boolean-conversion and cpp/unsafe-dacl-security-descriptor #235

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

Merged
felicitymay merged 5 commits into from
Oct 11, 2018
Merged

C++: Finalise docs for cpp/hresult-boolean-conversion and cpp/unsafe-dacl-security-descriptor #235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d813cb6
C++: Upper-case Boolean and <code> around HRESULT
jbj Sep 26, 2018
532a64f
C++: Name/description of HResultBooleanConversion
jbj Oct 1, 2018
54cd173
C++: Changelog entries for two new queries
jbj Oct 1, 2018
308631e
C++: Add two recent queries to query suites
jbj Oct 1, 2018
4b59c0c
Merge branch 'master' into hresult-boolean-qhelp
jbj Oct 9, 2018
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
2 changes: 2 additions & 0 deletions change-notes/1.19/analysis-cpp.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

| **Query** | **Tags** | **Purpose** |
|-----------------------------|-----------|--------------------------------------------------------------------|
| Cast between HRESULT and a Boolean type (`cpp/hresult-boolean-conversion`) | external/cwe/cwe-253 | Finds logic errors caused by mistakenly treating the Windows `HRESULT` type as a Boolean instead of testing it with the appropriate macros. Enabled by default. |
| Setting a DACL to `NULL` in a `SECURITY_DESCRIPTOR` (`cpp/unsafe-dacl-security-descriptor`) | external/cwe/cwe-732 | This query finds code that creates world-writable objects on Windows by setting their DACL to `NULL`. Enabled by default. |
Copy link
Contributor Author

@jbj jbj Oct 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two queries should have the "security" tag also.

| Cast from char* to wchar_t* | security, external/cwe/cwe-704 | Detects potentially dangerous casts from char* to wchar_t*. Enabled by default on LGTM. |

## Changes to existing queries
Expand Down
1 change: 1 addition & 0 deletions cpp/config/suites/c/correctness
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
+ semmlecode-cpp-queries/Likely Bugs/Arithmetic/IntMultToLong.ql: /Correctness/Dangerous Conversions
+ semmlecode-cpp-queries/Likely Bugs/Conversion/NonzeroValueCastToPointer.ql: /Correctness/Dangerous Conversions
+ semmlecode-cpp-queries/Likely Bugs/Conversion/ImplicitDowncastFromBitfield.ql: /Correctness/Dangerous Conversions
+ semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /Correctness/Dangerous Conversions
# Consistent Use
+ semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
+ semmlecode-cpp-queries/Likely Bugs/InconsistentCheckReturnNull.ql: /Correctness/Consistent Use
Expand Down
1 change: 1 addition & 0 deletions cpp/config/suites/cpp/correctness
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
+ semmlecode-cpp-queries/Likely Bugs/Conversion/NonzeroValueCastToPointer.ql: /Correctness/Dangerous Conversions
+ semmlecode-cpp-queries/Likely Bugs/Conversion/ImplicitDowncastFromBitfield.ql: /Correctness/Dangerous Conversions
+ semmlecode-cpp-queries/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql: /Correctness/Dangerous Conversions
+ semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /Correctness/Dangerous Conversions
# Consistent Use
+ semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
+ semmlecode-cpp-queries/Likely Bugs/InconsistentCheckReturnNull.ql: /Correctness/Consistent Use
Expand Down
3 changes: 3 additions & 0 deletions cpp/config/suites/security/cwe-253
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# CWE-253: Incorrect Check of Function Return Value
+ semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /CWE/CWE-253
@name Cast between HRESULT and a Boolean type (CWE-253)
2 changes: 2 additions & 0 deletions cpp/config/suites/security/cwe-732
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# CWE-732: Incorrect Permission Assignment for Critical Resource
+ semmlecode-cpp-queries/Security/CWE/CWE-732/DoNotCreateWorldWritable.ql: /CWE/CWE-732
@name File created without restricting permissions (CWE-732)
+ semmlecode-cpp-queries/Security/CWE/CWE-732/UnsafeDaclSecurityDescriptor.ql: /CWE/CWE-732
@name Setting a DACL to NULL in a SECURITY_DESCRIPTOR (CWE-732)
1 change: 1 addition & 0 deletions cpp/config/suites/security/default
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
@import "cwe-170"
@import "cwe-190"
@import "cwe-242"
@import "cwe-253"
@import "cwe-290"
@import "cwe-311"
@import "cwe-327"
Expand Down
8 changes: 4 additions & 4 deletions cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.qhelp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
<qhelp>

<overview>
<p>This query indicates that an <code>HRESULT</code> is being cast to a boolean type or vice versa.</p>
<p>The typical success value (<code>S_OK</code>) of an <code>HRESULT</code> equals 0. However, 0 indicates failure for a boolean type.</p>
<p>Casting an <code>HRESULT</code> to a boolean type and then using it in a test expression will yield an incorrect result.</p>
<p>This query indicates that an <code>HRESULT</code> is being cast to a Boolean type or vice versa.</p>
<p>The typical success value (<code>S_OK</code>) of an <code>HRESULT</code> equals 0. However, 0 indicates failure for a Boolean type.</p>
<p>Casting an <code>HRESULT</code> to a Boolean type and then using it in a test expression will yield an incorrect result.</p>
</overview>

<recommendation>
<p>To check if a call that returns an HRESULT succeeded use the <code>FAILED</code> macro.</p>
<p>To check if a call that returns an <code>HRESULT</code> succeeded use the <code>FAILED</code> macro.</p>
</recommendation>

<example>
Expand Down
8 changes: 3 additions & 5 deletions cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
/**
* @name Cast between semantically different integer types: HRESULT to/from a Boolean type
* @description Cast between semantically different integer types: HRESULT to/from a Boolean type.
* Boolean types indicate success by a non-zero value, whereas success (S_OK) in HRESULT is indicated by a value of 0.
* Casting an HRESULT to/from a Boolean type and then using it in a test expression will yield an incorrect result.
* @name Cast between HRESULT and a Boolean type
* @description Casting an HRESULT to/from a Boolean type and then using it in a test expression will yield an incorrect result because success (S_OK) in HRESULT is indicated by a value of 0.
* @kind problem
* @id cpp/hresult-boolean-conversion
* @problem.severity error
Expand Down Expand Up @@ -68,4 +66,4 @@ where exists
)
and not isHresultBooleanConverted(e1)
)
select e1, msg
select e1, msg