From d813cb63e7684a31b64f9505574606d337614a21 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 26 Sep 2018 15:29:07 +0200 Subject: [PATCH 1/4] C++: Upper-case Boolean and around HRESULT --- .../Security/CWE/CWE-253/HResultBooleanConversion.qhelp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.qhelp b/cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.qhelp index d5b949691f01..fdc316089223 100644 --- a/cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.qhelp +++ b/cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.qhelp @@ -4,13 +4,13 @@ -

This query indicates that an HRESULT is being cast to a boolean type or vice versa.

-

The typical success value (S_OK) of an HRESULT equals 0. However, 0 indicates failure for a boolean type.

-

Casting an HRESULT to a boolean type and then using it in a test expression will yield an incorrect result.

+

This query indicates that an HRESULT is being cast to a Boolean type or vice versa.

+

The typical success value (S_OK) of an HRESULT equals 0. However, 0 indicates failure for a Boolean type.

+

Casting an HRESULT to a Boolean type and then using it in a test expression will yield an incorrect result.

 -

To check if a call that returns an HRESULT succeeded use the FAILED macro.

+

To check if a call that returns an HRESULT succeeded use the FAILED macro.

 From 532a64f211b6b328966ee6088f3d573c1658eae5 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Mon, 1 Oct 2018 12:12:00 +0200 Subject: [PATCH 2/4] C++: Name/description of HResultBooleanConversion This commit changes the name and description of the new `HResultBooleanConversion` query to follow our internal guidelines. --- .../src/Security/CWE/CWE-253/HResultBooleanConversion.ql | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.ql b/cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.ql index 7678cff23f0f..165fe0eed969 100644 --- a/cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.ql +++ b/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 \ No newline at end of file +select e1, msg From 54cd173da8ff558f6b70b7896544cf1ae5f674e5 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Mon, 1 Oct 2018 12:13:14 +0200 Subject: [PATCH 3/4] C++: Changelog entries for two new queries --- change-notes/1.19/analysis-cpp.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/change-notes/1.19/analysis-cpp.md b/change-notes/1.19/analysis-cpp.md index 5a3340603c87..f45a4ee92ea9 100644 --- a/change-notes/1.19/analysis-cpp.md +++ b/change-notes/1.19/analysis-cpp.md @@ -6,7 +6,8 @@ | **Query** | **Tags** | **Purpose** | |-----------------------------|-----------|--------------------------------------------------------------------| -| *@name of query (Query ID)* | *Tags* |*Aim of the new query and whether it is enabled by default or not* | +| 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. | ## Changes to existing queries From 308631e8ff45e8f6b2a771a50f9087985d41bf4f Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Mon, 1 Oct 2018 13:38:13 +0200 Subject: [PATCH 4/4] C++: Add two recent queries to query suites --- cpp/config/suites/c/correctness | 1 + cpp/config/suites/cpp/correctness | 1 + cpp/config/suites/security/cwe-253 | 3 +++ cpp/config/suites/security/cwe-732 | 2 ++ cpp/config/suites/security/default | 1 + 5 files changed, 8 insertions(+) create mode 100644 cpp/config/suites/security/cwe-253 diff --git a/cpp/config/suites/c/correctness b/cpp/config/suites/c/correctness index f3f9bebf4a5b..e53ccda11870 100644 --- a/cpp/config/suites/c/correctness +++ b/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 diff --git a/cpp/config/suites/cpp/correctness b/cpp/config/suites/cpp/correctness index e11954426239..ceb15f552606 100644 --- a/cpp/config/suites/cpp/correctness +++ b/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 diff --git a/cpp/config/suites/security/cwe-253 b/cpp/config/suites/security/cwe-253 new file mode 100644 index 000000000000..1ce40c4e71ae --- /dev/null +++ b/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) diff --git a/cpp/config/suites/security/cwe-732 b/cpp/config/suites/security/cwe-732 index 90af49984383..77de440b90ea 100644 --- a/cpp/config/suites/security/cwe-732 +++ b/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) diff --git a/cpp/config/suites/security/default b/cpp/config/suites/security/default index 4225ae9808b1..baa8a3f5bc82 100644 --- a/cpp/config/suites/security/default +++ b/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"