diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 7efedad2b..c0d9d0f48 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -113,6 +113,13 @@ void CheckBufferOverrun::bufferOverrun(const Token *tok, const std::string &varn reportError(tok, Severity::error, "bufferAccessOutOfBounds", errmsg); } +void CheckBufferOverrun::possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst) +{ + reportError(tok, Severity::warning, "possibleBufferAccessOutOfBounds", + "Possible buffer overflow if strlen(" + src + ") is larger than sizeof(" + dst + ")-strlen(" + dst +").\n" + "The source buffer is larger than the destination buffer so there is the potential for overflowing the destination buffer."); +} + void CheckBufferOverrun::strncatUsage(const Token *tok) { if (_settings && !_settings->isEnabled("style")) @@ -970,7 +977,21 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector 0 && Token::Match(tok, "strcpy|strcat ( %varid% , %var% )", varid)) || + (varid == 0 && Token::Match(tok, ("strcpy|strcat ( " + varnames + " , %var% )").c_str()))) + { + const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(tok->tokAt(4)->varId()); + if (var && var->isArray() && var->dimensions().size() == 1) + { + const std::size_t len = var->dimension(0); + if (total_size > 0 && len > (unsigned int)total_size) + { + if (_settings->inconclusive) + possibleBufferOverrunError(tok, tok->strAt(4), tok->strAt(2)); + continue; + } + } + } // Detect few strcat() calls const std::string strcatPattern = varid > 0 ? std::string("strcat ( %varid% , %str% ) ;") : ("strcat ( " + varnames + " , %str% ) ;"); diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 77739b252..2cb32c95b 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -221,6 +221,7 @@ public: void cmdLineArgsError(const Token *tok); void pointerOutOfBounds(const Token *tok, const std::string &object); // UB when result of calculation is out of bounds void arrayIndexThenCheckError(const Token *tok, const std::string &indexName); + void possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) { @@ -235,6 +236,7 @@ public: c.cmdLineArgsError(0); c.pointerOutOfBounds(0, "array"); c.arrayIndexThenCheckError(0, "index"); + c.possibleBufferOverrunError(0, "source", "destination"); } std::string myName() const diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index ec7d320ff..d892a4a24 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -42,6 +42,7 @@ private: errout.str(""); Settings settings; + settings.inconclusive = true; settings.experimental = experimental; settings.addEnabled("style"); @@ -142,6 +143,7 @@ private: TEST_CASE(buffer_overrun_19); // #2597 - class member with unknown type TEST_CASE(buffer_overrun_20); // #2986 (segmentation fault) TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch + TEST_CASE(possible_buffer_overrun_1); // #3035 // It is undefined behaviour to point out of bounds of an array // the address beyond the last element is in bounds @@ -2014,6 +2016,39 @@ private: ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:3]: (error) Array 'a[10]' index 100 out of bounds\n", errout.str()); } + void possible_buffer_overrun_1() // #3035 + { + check("void foo() {\n" + " char * data = (char *)alloca(50);\n" + " char src[100];\n" + " memset(src, 'C', 100-1);\n" + " src[100-1] = '\\0';\n" + " strcat(data, src);\n" + "}"); + ASSERT_EQUALS("[test.cpp:6]: (warning) Possible buffer overflow if strlen(src) is larger than sizeof(data)-strlen(data).\n", errout.str()); + + check("void foo() {\n" + " char * data = (char *)alloca(100);\n" + " char src[100];\n" + " memset(src, 'C', 100-1);\n" + " src[100-1] = '\\0';\n" + " strcat(data, src);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("void foo(char src[100]) {\n" + " char * data = (char *)alloca(50);\n" + " strcat(data, src);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Possible buffer overflow if strlen(src) is larger than sizeof(data)-strlen(data).\n", errout.str()); + + check("void foo(char src[100]) {\n" + " char * data = (char *)alloca(100);\n" + " strcat(data, src);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } + void pointer_out_of_bounds_1() { check("void f() {\n"