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..df69886e97bd --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.cpp @@ -0,0 +1,35 @@ +// 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 error will be handled. +void goodFunction(const int *source, std::size_t length) noexcept { + try { + 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 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::memset(dest, 0, length); +// .. +} 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..9e6cb2d89cec --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.qhelp @@ -0,0 +1,27 @@ + + + +

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.

+ +
+ + +

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

+ +
+ +

The following example 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..dd9c16fac114 --- /dev/null +++ b/cpp/ql/src/experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql @@ -0,0 +1,87 @@ +/** + * @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 + * @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() + or + this.getCondition().(NEExpr).getAChild().getValue() = "0" and + this.getThen().getAChild*() instanceof ReturnStmt + } +} + +/** + * 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 ts | this.getEnclosingStmt() = ts.getStmt().getAChild*()) + } + + /** + * 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" 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..e4aa8cf29766 --- /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]; // BAD [NOT DETECTED] + if(!(new int[100])) // BAD [NOT DETECTED] + return; + } +} +void badNew_0_1() +{ + int * i = new int[100]; // BAD + if(i == 0) + return; + if(!i) + return; + if(i == NULL) + return; + int * j; + j = new int[100]; // BAD + if(j == 0) + return; + if(!j) + return; + if(j == NULL) + return; +} +void badNew_1_0() +{ + try { + while (true) { + new(std::nothrow) int[100]; // BAD + int* p = new(std::nothrow) int[100]; // BAD + int* p1; + p1 = new(std::nothrow) int[100]; // BAD + } + } 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]; // BAD [NOT DETECTED] + new(std::nothrow) int[100]; // BAD [NOT DETECTED] + } +} + +void goodNew_0_0() +{ + try { + while (true) { + new int[100]; // GOOD + } + } 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]; // GOOD + if (p == nullptr) { +// std::cout << "Allocation returned nullptr\n"; + break; + } + int* p1; + p1 = new(std::nothrow) int[100]; // GOOD + if (p1 == nullptr) { +// std::cout << "Allocation returned nullptr\n"; + break; + } + if (new(std::nothrow) int[100] == nullptr) { // GOOD +// std::cout << "Allocation returned nullptr\n"; + break; + } + } +}