From 9071ba2f99ef3f09732999b517a72d45b30c210e Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Mon, 25 Jan 2021 00:06:19 +0300 Subject: [PATCH 01/14] Add files via upload --- ...ctingAndHandlingMemoryAllocationErrors.cpp | 32 +++++++ ...ingAndHandlingMemoryAllocationErrors.qhelp | 29 ++++++ ...ectingAndHandlingMemoryAllocationErrors.ql | 93 +++++++++++++++++++ 3 files changed, 154 insertions(+) create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp create mode 100644 cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp new file mode 100644 index 000000000000..6fc3bc2f893a --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp @@ -0,0 +1,32 @@ +// BAD: no memory allocation errors are detected +void f(const int *array, std::size_t size) noexcept { + int *copy = new int[size]; + std::memcpy(copy, array, size * sizeof(*copy)); + // ... + delete [] copy; +} +// GOOD: memory allocation errors are detected +void f(const int *array, std::size_t size) noexcept { + int *copy; + try { + copy = new int[size]; + } catch(std::bad_alloc) { + // Handle error + return; + } + // At this point, copy has been initialized to allocated memory + std::memcpy(copy, array, size * sizeof(*copy)); + // ... + delete [] copy; +} +// GOOD: memory allocation errors are detected +void f(const int *array, std::size_t size) noexcept { + int *copy = new (std::nothrow) int[size]; + if (!copy) { + // Handle error + return; + } + std::memcpy(copy, array, size * sizeof(*copy)); + // ... + delete [] copy; +} diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp new file mode 100644 index 000000000000..4b7941a58463 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp @@ -0,0 +1,29 @@ + + + +

when using the new operator to allocate memory, you need to pay attention to the different way of detecting errors. so ::operator new(std::size_t) throws an exception on error, and ::operator new(std::size_t, const std::nothrow_t &) returns zero on error. the programmer can get confused and check the error that occurs when allocating memory incorrectly. That can lead to an unhandled program termination or to a violation of the program logic.

+ +

Loss of detection probably refers to use cases where memory allocation using your own solutions with strong nesting. It is also possible when using a buffer in the form of fields of different structures with the same names.

+ +
+ + +

We recommend using the error detection method, depending on the selected memory allocation method..

+ +
+ +

The following file demonstrates various approaches to detecting memory allocation errors using the new operator.

+ + +
+ + +
  • + CERT C++ Coding Standard: +MEM52-CPP. Detect and handle memory allocation errors. +
  • + +
    +
    diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql new file mode 100644 index 000000000000..383c8a1f1287 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql @@ -0,0 +1,93 @@ +/** + * @name Сonfusion In Detecting And Handling Memory Allocation Errors + * @description --::operator new(std::size_t) throws an exception on error, and ::operator new(std::size_t, const std::nothrow_t &) returns zero on error. + * --the programmer can get confused when check the error that occurs when allocating memory incorrectly. + * --Making a call of this type may result in a zero byte being written just outside the buffer. + * @kind problem + * @id cpp/detect-and-handle-memory-allocation-errors + * @problem.severity warning + * @precision medium + * @tags correctness + * security + * external/cwe/cwe-570 + */ + +import cpp + +/** + * Lookup if condition compare with 0 + */ +class IfCompareWithZero extends IfStmt { + IfCompareWithZero() { + this.getCondition().(EQExpr).getAChild().getValue() = "0" + or + this.getCondition().(NEExpr).getAChild().getValue() = "0" and + this.hasElse() + } +} + +/** + * lookup for calls to `operator new`, with incorrect error handling. + */ +class WrongCheckErrorOperatorNew extends FunctionCall { + Expr exp; + + WrongCheckErrorOperatorNew() { + this = exp.(NewOrNewArrayExpr).getAChild().(FunctionCall) and + ( + this.getTarget().hasGlobalOrStdName("operator new") + or + this.getTarget().hasGlobalOrStdName("operator new[]") + ) + } + + /** + * Holds if handler `try ... catch` exists. + */ + predicate isExistsTryCatchBlock() { + exists(TryStmt tb, AssignExpr aex, Initializer it | + tb.getAChild*() = exp + or + exp = it.getExpr() and + tb.getAChild*().(DeclStmt).getADeclaration() = it.getDeclaration() + or + aex.getAChild*() = exp and + tb.getAChild*().(AssignExpr) = aex + ) + } + + /** + * Holds if results call `operator new` check in `operator if`. + */ + predicate isExistsIfCondition() { + exists(IfCompareWithZero ifc, AssignExpr aex, Initializer it | + // call `operator new` directly from the condition of `operator if`. + this = ifc.getCondition().getAChild() + or + // check results call `operator new` with variable appropriation + postDominates(ifc, this) and + aex.getAChild() = exp and + ifc.getCondition().getAChild().(VariableAccess).getTarget() = + aex.getLValue().(VariableAccess).getTarget() + or + // check results call `operator new` with declaration variable + postDominates(ifc, this) and + exp = it.getExpr() and + it.getDeclaration() = ifc.getCondition().getAChild().(VariableAccess).getTarget() + ) + } + + /** + * Holds if `(std::nothrow)` exists in call `operator new`. + */ + predicate isExistsNothrow() { this.getAChild().toString() = "nothrow" } +} + +from WrongCheckErrorOperatorNew op +where + // use call `operator new` with `(std::nothrow)` and checking error using `try ... catch` block and not `operator if` + op.isExistsNothrow() and not op.isExistsIfCondition() and op.isExistsTryCatchBlock() + or + // use call `operator new` without `(std::nothrow)` and checking error using `operator if` and not `try ... catch` block + not op.isExistsNothrow() and not op.isExistsTryCatchBlock() and op.isExistsIfCondition() +select op, "memory allocation error check is incorrect or missing" From 20e19ec4677c1e414fceb17d16de31327967fe5a Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Mon, 25 Jan 2021 00:09:55 +0300 Subject: [PATCH 02/14] Add files via upload --- ...AndHandlingMemoryAllocationErrors.expected | 5 + ...ingAndHandlingMemoryAllocationErrors.qlref | 1 + .../CWE/CWE-570/semmle/tests/test.cpp | 97 +++++++++++++++++++ 3 files changed, 103 insertions(+) create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/WrongInDetectingAndHandlingMemoryAllocationErrors.expected create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/WrongInDetectingAndHandlingMemoryAllocationErrors.qlref create mode 100644 cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/test.cpp diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/WrongInDetectingAndHandlingMemoryAllocationErrors.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/WrongInDetectingAndHandlingMemoryAllocationErrors.expected new file mode 100644 index 000000000000..80e82cff2129 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/WrongInDetectingAndHandlingMemoryAllocationErrors.expected @@ -0,0 +1,5 @@ +| test.cpp:30:15:30:26 | call to operator new[] | memory allocation error check is incorrect or missing | +| test.cpp:38:9:38:20 | call to operator new[] | memory allocation error check is incorrect or missing | +| test.cpp:50:13:50:38 | call to operator new[] | memory allocation error check is incorrect or missing | +| test.cpp:51:22:51:47 | call to operator new[] | memory allocation error check is incorrect or missing | +| test.cpp:53:18:53:43 | call to operator new[] | memory allocation error check is incorrect or missing | diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/WrongInDetectingAndHandlingMemoryAllocationErrors.qlref b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/WrongInDetectingAndHandlingMemoryAllocationErrors.qlref new file mode 100644 index 000000000000..fc3252ef1220 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/WrongInDetectingAndHandlingMemoryAllocationErrors.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/test.cpp new file mode 100644 index 000000000000..6f03f8960245 --- /dev/null +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/test.cpp @@ -0,0 +1,97 @@ +#define NULL ((void*)0) +class exception {}; + +namespace std{ + struct nothrow_t {}; + typedef unsigned long size_t; + class bad_alloc{ + const char* what() const throw(); + }; + extern const std::nothrow_t nothrow; +} + +using namespace std; + +void* operator new(std::size_t _Size); +void* operator new[](std::size_t _Size); +void* operator new( std::size_t count, const std::nothrow_t& tag ); +void* operator new[]( std::size_t count, const std::nothrow_t& tag ); + +void badNew_0_0() +{ + while (true) { + new int[100]; + if(!(new int[100])) + return; + } +} +void badNew_0_1() +{ + int * i = new int[100]; + if(i == 0) + return; + if(!i) + return; + if(i == NULL) + return; + int * j; + j = new int[100]; + if(j == 0) + return; + if(!j) + return; + if(j == NULL) + return; +} +void badNew_1_0() +{ + try { + while (true) { + new(std::nothrow) int[100]; + int* p = new(std::nothrow) int[100]; + int* p1; + p1 = new(std::nothrow) int[100]; + } + } catch (const exception &){//const std::bad_alloc& e) { +// std::cout << e.what() << '\n'; + } +} +void badNew_1_1() +{ + while (true) { + int* p = new(std::nothrow) int[100]; + new(std::nothrow) int[100]; + } +} + +void goodNew_0_0() +{ + try { + while (true) { + new int[100]; + } + } catch (const exception &){//const std::bad_alloc& e) { +// std::cout << e.what() << '\n'; + } +} + +void goodNew_1_0() +{ + while (true) { + int* p = new(std::nothrow) int[100]; + if (p == nullptr) { +// std::cout << "Allocation returned nullptr\n"; + break; + } + int* p1; + p1 = new(std::nothrow) int[100]; + if (p1 == nullptr) { +// std::cout << "Allocation returned nullptr\n"; + break; + } + if (new(std::nothrow) int[100] == nullptr) { +// std::cout << "Allocation returned nullptr\n"; + break; + } + } +} From 8737c1442b9f52d5f1c3a29e2762db0b850873eb Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Wed, 27 Jan 2021 14:48:23 +0300 Subject: [PATCH 03/14] Update WrongInDetectingAndHandlingMemoryAllocationErrors.cpp --- ...ctingAndHandlingMemoryAllocationErrors.cpp | 53 +++++++++---------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp index 6fc3bc2f893a..0232fc131ebd 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp @@ -1,32 +1,31 @@ -// BAD: no memory allocation errors are detected -void f(const int *array, std::size_t size) noexcept { - int *copy = new int[size]; - std::memcpy(copy, array, size * sizeof(*copy)); - // ... - delete [] copy; +// BAD: on memory allocation error, the program terminates. +void badFunction(const int *source, std::size_t length) noexcept { + int * dest = new int[length]; + std::memset(dest, 0, length); +// .. } -// GOOD: memory allocation errors are detected -void f(const int *array, std::size_t size) noexcept { - int *copy; +// GOOD: memory allocation error will be handled. +void goodFunction(const int *source, std::size_t length) noexcept { try { - copy = new int[size]; - } catch(std::bad_alloc) { - // Handle error - return; - } - // At this point, copy has been initialized to allocated memory - std::memcpy(copy, array, size * sizeof(*copy)); - // ... - delete [] copy; + int * dest = new int[length]; + } catch(std::bad_alloc) + std::memset(dest, 0, length); +// .. +} +// BAD: memory allocation error will not be handled. +void badFunction(const int *source, std::size_t length) noexcept { + try { + int * dest = new (std::nothrow) int[length]; + } catch(std::bad_alloc) + std::memset(dest, 0, length); +// .. } -// GOOD: memory allocation errors are detected -void f(const int *array, std::size_t size) noexcept { - int *copy = new (std::nothrow) int[size]; - if (!copy) { - // Handle error - return; +// GOOD: memory allocation error will be handled. +void goodFunction(const int *source, std::size_t length) noexcept { + int * dest = new (std::nothrow) int[length]; + if (!dest) { + return; } - std::memcpy(copy, array, size * sizeof(*copy)); - // ... - delete [] copy; + std::memset(dest, 0, length); +// .. } From bec00643968bad1ae39d53e0256111187569b907 Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Wed, 27 Jan 2021 14:54:47 +0300 Subject: [PATCH 04/14] Update test.cpp --- .../CWE/CWE-570/semmle/tests/test.cpp | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/test.cpp index 6f03f8960245..e4aa8cf29766 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-570/semmle/tests/test.cpp @@ -20,14 +20,14 @@ void* operator new[]( std::size_t count, const std::nothrow_t& tag ); void badNew_0_0() { while (true) { - new int[100]; - if(!(new int[100])) + new int[100]; // BAD [NOT DETECTED] + if(!(new int[100])) // BAD [NOT DETECTED] return; } } void badNew_0_1() { - int * i = new int[100]; + int * i = new int[100]; // BAD if(i == 0) return; if(!i) @@ -35,7 +35,7 @@ void badNew_0_1() if(i == NULL) return; int * j; - j = new int[100]; + j = new int[100]; // BAD if(j == 0) return; if(!j) @@ -47,10 +47,10 @@ void badNew_1_0() { try { while (true) { - new(std::nothrow) int[100]; - int* p = new(std::nothrow) int[100]; + new(std::nothrow) int[100]; // BAD + int* p = new(std::nothrow) int[100]; // BAD int* p1; - p1 = new(std::nothrow) int[100]; + p1 = new(std::nothrow) int[100]; // BAD } } catch (const exception &){//const std::bad_alloc& e) { // std::cout << e.what() << '\n'; @@ -59,8 +59,8 @@ void badNew_1_0() void badNew_1_1() { while (true) { - int* p = new(std::nothrow) int[100]; - new(std::nothrow) int[100]; + int* p = new(std::nothrow) int[100]; // BAD [NOT DETECTED] + new(std::nothrow) int[100]; // BAD [NOT DETECTED] } } @@ -68,7 +68,7 @@ void goodNew_0_0() { try { while (true) { - new int[100]; + new int[100]; // GOOD } } catch (const exception &){//const std::bad_alloc& e) { // std::cout << e.what() << '\n'; @@ -78,18 +78,18 @@ void goodNew_0_0() void goodNew_1_0() { while (true) { - int* p = new(std::nothrow) int[100]; + int* p = new(std::nothrow) int[100]; // GOOD if (p == nullptr) { // std::cout << "Allocation returned nullptr\n"; break; } int* p1; - p1 = new(std::nothrow) int[100]; + p1 = new(std::nothrow) int[100]; // GOOD if (p1 == nullptr) { // std::cout << "Allocation returned nullptr\n"; break; } - if (new(std::nothrow) int[100] == nullptr) { + if (new(std::nothrow) int[100] == nullptr) { // GOOD // std::cout << "Allocation returned nullptr\n"; break; } From 25de82c78cd5c19fd1957a8926ba7e395edf2815 Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Wed, 27 Jan 2021 15:05:01 +0300 Subject: [PATCH 05/14] Apply suggestions from code review Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com> --- .../WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp index 4b7941a58463..7be7fcd5b357 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp @@ -3,18 +3,18 @@ "qhelp.dtd"> -

    when using the new operator to allocate memory, you need to pay attention to the different way of detecting errors. so ::operator new(std::size_t) throws an exception on error, and ::operator new(std::size_t, const std::nothrow_t &) returns zero on error. the programmer can get confused and check the error that occurs when allocating memory incorrectly. That can lead to an unhandled program termination or to a violation of the program logic.

    +

    When using the new operator to allocate memory, you need to pay attention to the different ways of detecting errors. ::operator new(std::size_t) throws an exception on error, whereas ::operator new(std::size_t, const std::nothrow_t &) returns zero on error. The programmer can get confused and check the error that occurs when allocating memory incorrectly. That can lead to an unhandled program termination or to a violation of the program logic.

    Loss of detection probably refers to use cases where memory allocation using your own solutions with strong nesting. It is also possible when using a buffer in the form of fields of different structures with the same names.

    -

    We recommend using the error detection method, depending on the selected memory allocation method..

    +

    Use the correct error detection method corresponding with the memory allocation.

    -

    The following file demonstrates various approaches to detecting memory allocation errors using the new operator.

    +

    The following example demonstrates various approaches to detecting memory allocation errors using the new operator.

    From 5d163b4c1575115a5f1c14eba0145b74908ee292 Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Wed, 27 Jan 2021 15:05:58 +0300 Subject: [PATCH 06/14] Update WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp --- .../WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp | 2 -- 1 file changed, 2 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp index 7be7fcd5b357..c3e543a1b589 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp @@ -5,8 +5,6 @@

    When using the new operator to allocate memory, you need to pay attention to the different ways of detecting errors. ::operator new(std::size_t) throws an exception on error, whereas ::operator new(std::size_t, const std::nothrow_t &) returns zero on error. The programmer can get confused and check the error that occurs when allocating memory incorrectly. That can lead to an unhandled program termination or to a violation of the program logic.

    -

    Loss of detection probably refers to use cases where memory allocation using your own solutions with strong nesting. It is also possible when using a buffer in the form of fields of different structures with the same names.

    -
    From 16d058f49865acb62d36c1a9b1af0aa6810d9eac Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Wed, 27 Jan 2021 15:06:57 +0300 Subject: [PATCH 07/14] Update WrongInDetectingAndHandlingMemoryAllocationErrors.ql --- .../CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql index 383c8a1f1287..c32c14c1dd01 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql @@ -2,7 +2,6 @@ * @name Сonfusion In Detecting And Handling Memory Allocation Errors * @description --::operator new(std::size_t) throws an exception on error, and ::operator new(std::size_t, const std::nothrow_t &) returns zero on error. * --the programmer can get confused when check the error that occurs when allocating memory incorrectly. - * --Making a call of this type may result in a zero byte being written just outside the buffer. * @kind problem * @id cpp/detect-and-handle-memory-allocation-errors * @problem.severity warning From bdfdcbd6735a841c4329a26b15bc4130c10e8f67 Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Wed, 27 Jan 2021 15:48:18 +0300 Subject: [PATCH 08/14] Update WrongInDetectingAndHandlingMemoryAllocationErrors.ql --- ...rongInDetectingAndHandlingMemoryAllocationErrors.ql | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql index c32c14c1dd01..e165d61985b4 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql @@ -44,14 +44,8 @@ class WrongCheckErrorOperatorNew extends FunctionCall { * Holds if handler `try ... catch` exists. */ predicate isExistsTryCatchBlock() { - exists(TryStmt tb, AssignExpr aex, Initializer it | - tb.getAChild*() = exp - or - exp = it.getExpr() and - tb.getAChild*().(DeclStmt).getADeclaration() = it.getDeclaration() - or - aex.getAChild*() = exp and - tb.getAChild*().(AssignExpr) = aex + exists(TryStmt ts | + this.getEnclosingStmt() = ts.getStmt().getAChild*() ) } From c8eeb5f73e70c567dc062c7a3b143faa8063ec1a Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Fri, 29 Jan 2021 11:51:15 +0300 Subject: [PATCH 09/14] Update WrongInDetectingAndHandlingMemoryAllocationErrors.ql --- .../WrongInDetectingAndHandlingMemoryAllocationErrors.ql | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql index e165d61985b4..04a284d78463 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql @@ -22,6 +22,9 @@ class IfCompareWithZero extends IfStmt { or this.getCondition().(NEExpr).getAChild().getValue() = "0" and this.hasElse() + or + this.getCondition().(NEExpr).getAChild().getValue() = "0" and + this.getThen().getAChild*() instanceof ReturnStmt } } From bdbf5a4fae29511be7d64039847a1a70a7e61545 Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Fri, 29 Jan 2021 13:41:45 +0300 Subject: [PATCH 10/14] Apply suggestions from code review Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com> --- .../WrongInDetectingAndHandlingMemoryAllocationErrors.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp index 0232fc131ebd..df69886e97bd 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp @@ -8,7 +8,9 @@ void badFunction(const int *source, std::size_t length) noexcept { void goodFunction(const int *source, std::size_t length) noexcept { try { int * dest = new int[length]; - } catch(std::bad_alloc) + } catch(std::bad_alloc) { + // ... + } std::memset(dest, 0, length); // .. } @@ -16,7 +18,9 @@ void goodFunction(const int *source, std::size_t length) noexcept { void badFunction(const int *source, std::size_t length) noexcept { try { int * dest = new (std::nothrow) int[length]; - } catch(std::bad_alloc) + } catch(std::bad_alloc) { + // ... + } std::memset(dest, 0, length); // .. } From 2b946aee5a3ff0c7d5e284e2ff7527e4dd79cda6 Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Sun, 31 Jan 2021 15:21:54 +0300 Subject: [PATCH 11/14] Update WrongInDetectingAndHandlingMemoryAllocationErrors.ql --- .../WrongInDetectingAndHandlingMemoryAllocationErrors.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql index 04a284d78463..403d8388b179 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql @@ -58,7 +58,7 @@ class WrongCheckErrorOperatorNew extends FunctionCall { predicate isExistsIfCondition() { exists(IfCompareWithZero ifc, AssignExpr aex, Initializer it | // call `operator new` directly from the condition of `operator if`. - this = ifc.getCondition().getAChild() + this = ifc.getCondition().getAChild*() or // check results call `operator new` with variable appropriation postDominates(ifc, this) and From 2131f3580167e37e23daf77313bc4cde0a6b4615 Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Thu, 4 Feb 2021 15:41:40 +0300 Subject: [PATCH 12/14] Update WrongInDetectingAndHandlingMemoryAllocationErrors.ql --- .../WrongInDetectingAndHandlingMemoryAllocationErrors.ql | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql index 403d8388b179..0181744bb865 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql @@ -1,5 +1,5 @@ /** - * @name Сonfusion In Detecting And Handling Memory Allocation Errors + * @name onfusion In Detecting And Handling Memory Allocation Errors * @description --::operator new(std::size_t) throws an exception on error, and ::operator new(std::size_t, const std::nothrow_t &) returns zero on error. * --the programmer can get confused when check the error that occurs when allocating memory incorrectly. * @kind problem @@ -47,9 +47,7 @@ class WrongCheckErrorOperatorNew extends FunctionCall { * Holds if handler `try ... catch` exists. */ predicate isExistsTryCatchBlock() { - exists(TryStmt ts | - this.getEnclosingStmt() = ts.getStmt().getAChild*() - ) + exists(TryStmt ts | this.getEnclosingStmt() = ts.getStmt().getAChild*()) } /** From a43167faf7cbb82cf0d4f3eaa5ca492818e7a9f0 Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Thu, 4 Feb 2021 15:44:28 +0300 Subject: [PATCH 13/14] Update WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp --- .../WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp index c3e543a1b589..9e6cb2d89cec 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp @@ -3,7 +3,7 @@ "qhelp.dtd"> -

    When using the new operator to allocate memory, you need to pay attention to the different ways of detecting errors. ::operator new(std::size_t) throws an exception on error, whereas ::operator new(std::size_t, const std::nothrow_t &) returns zero on error. The programmer can get confused and check the error that occurs when allocating memory incorrectly. That can lead to an unhandled program termination or to a violation of the program logic.

    +

    When using the new operator to allocate memory, you need to pay attention to the different ways of detecting errors. ::operator new(std::size_t) throws an exception on error, whereas ::operator new(std::size_t, const std::nothrow_t &) returns zero on error. The programmer can get confused and check the error that occurs when allocating memory incorrectly. That can lead to an unhandled program termination or to a violation of the program logic.

    From 43045c1f034b9fe2eeeb7f6746f02a7392921725 Mon Sep 17 00:00:00 2001 From: ihsinme <61293369+ihsinme@users.noreply.github.com> Date: Thu, 4 Feb 2021 15:47:16 +0300 Subject: [PATCH 14/14] Update WrongInDetectingAndHandlingMemoryAllocationErrors.ql --- .../WrongInDetectingAndHandlingMemoryAllocationErrors.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql index 0181744bb865..dd9c16fac114 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql @@ -1,5 +1,5 @@ /** - * @name onfusion In Detecting And Handling Memory Allocation Errors + * @name Detect And Handle Memory Allocation Errors * @description --::operator new(std::size_t) throws an exception on error, and ::operator new(std::size_t, const std::nothrow_t &) returns zero on error. * --the programmer can get confused when check the error that occurs when allocating memory incorrectly. * @kind problem