Merge pull request #235 from jbj/hresult-boolean-qhelp

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

Author: Felicity Chapman

change-notes/1.19/analysis-cpp.md

@@ -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. |
 | 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

cpp/config/suites/c/correctness

@@ -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

cpp/config/suites/cpp/correctness

@@ -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

cpp/config/suites/security/cwe-253

@@ -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)

cpp/config/suites/security/cwe-732

@@ -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)

cpp/config/suites/security/default

@@ -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"

cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.qhelp

@@ -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>

cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.ql

@@ -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
@@ -68,4 +66,4 @@ where exists
     )
     and not isHresultBooleanConverted(e1)
   )
-select e1, msg
+select e1, msg