From 2ab8de26500c62aaf891009f908d25f8281cc83f Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Sat, 20 Aug 2022 07:56:31 +0200 Subject: [PATCH] Fix #11145 FP negativeMemoryAllocationSize with possible value (#4387) --- lib/checkbufferoverrun.cpp | 11 +++++++---- lib/checkbufferoverrun.h | 5 ++--- test/testbufferoverrun.cpp | 9 +++++++++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 37caaab6e..f939312e3 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -1141,7 +1141,7 @@ void CheckBufferOverrun::negativeArraySize() continue; const ValueFlow::Value* sz = valOperand->getValueLE(-1, mSettings); if (sz) - negativeMemoryAllocationSizeError(tok); + negativeMemoryAllocationSizeError(tok, sz); } } } @@ -1155,8 +1155,11 @@ void CheckBufferOverrun::negativeArraySizeError(const Token* tok) "Declaration of array '" + arrayName + "' with negative size is undefined behaviour", CWE758, Certainty::safe); } -void CheckBufferOverrun::negativeMemoryAllocationSizeError(const Token* tok) +void CheckBufferOverrun::negativeMemoryAllocationSizeError(const Token* tok, const ValueFlow::Value* value) { - reportError(tok, Severity::error, "negativeMemoryAllocationSize", - "Memory allocation size is negative.", CWE131, Certainty::safe); + const std::string msg = "Memory allocation size is negative."; + const ErrorPath errorPath = getErrorPath(tok, value, msg); + const bool inconclusive = value != nullptr && !value->isKnown(); + reportError(errorPath, inconclusive ? Severity::warning : Severity::error, "negativeMemoryAllocationSize", + msg, CWE131, inconclusive ? Certainty::inconclusive : Certainty::safe); } diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 900d7aa31..9c1b7d155 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -87,9 +87,8 @@ public: c.bufferOverflowError(nullptr, nullptr, Certainty::normal); c.objectIndexError(nullptr, nullptr, true); c.argumentSizeError(nullptr, "function", 1, "buffer", nullptr, nullptr); - c.negativeMemoryAllocationSizeError(nullptr); + c.negativeMemoryAllocationSizeError(nullptr, nullptr); c.negativeArraySizeError(nullptr); - c.negativeMemoryAllocationSizeError(nullptr); } /** @brief Parse current TU and extract file info */ @@ -125,7 +124,7 @@ private: void negativeArraySize(); void negativeArraySizeError(const Token* tok); - void negativeMemoryAllocationSizeError(const Token* tok); // provide a negative value to memory allocation function + void negativeMemoryAllocationSizeError(const Token* tok, const ValueFlow::Value* value); // provide a negative value to memory allocation function void objectIndex(); void objectIndexError(const Token *tok, const ValueFlow::Value *v, bool known); diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index c1cb1fc78..cc6f264aa 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -4911,6 +4911,15 @@ private: " a = (int *)alloca( -10 );\n" "}"); TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Memory allocation size is negative.\n", "", errout.str()); + + check("int* f(int n) {\n" // #11145 + " int d = -1;\n" + " for (int i = 0; i < n; ++i)\n" + " d = std::max(i, d);\n" + " int* p = new int[d];\n" + " return p;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:5]: (warning, inconclusive) Memory allocation size is negative.\n", errout.str()); } void negativeArraySize() {