diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 129cba54f..8653a277a 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -176,6 +176,11 @@ void CheckBufferOverrun::bufferNotZeroTerminatedError(const Token *tok, const st reportError(tok, Severity::warning, "bufferNotZeroTerminated", errmsg, true); } +void CheckBufferOverrun::argumentSizeError(const Token *tok, const std::string &functionName, const std::string &varname) +{ + reportError(tok, Severity::style, "argumentSize", "the array " + varname + " is too small, the function " + functionName + " expects a bigger array"); +} + //--------------------------------------------------------------------------- @@ -620,7 +625,8 @@ void CheckBufferOverrun::checkFunctionParameter(const Token &tok, unsigned int p // Calling a user function? // only 1-dimensional arrays can be checked currently else if (arrayInfo.num().size() == 1) { - const Function* func = _tokenizer->getSymbolDatabase()->findFunctionByName(tok.str(), tok.scope());; + const Function* func = _tokenizer->getSymbolDatabase()->findFunctionByName(tok.str(), tok.scope()); + if (func && func->hasBody) { // Get corresponding parameter.. const Variable* parameter = func->getArgumentVar(par-1); @@ -686,6 +692,36 @@ void CheckBufferOverrun::checkFunctionParameter(const Token &tok, unsigned int p } } } + + // Check 'float x[10]' arguments in declaration + if (_settings->isEnabled("style")) { + const Function* func = _tokenizer->getSymbolDatabase()->findFunctionByName(tok.str(), tok.scope()); + + // If argument is '%type% a[num]' then check bounds against num + if (func) { + const Variable* argument = func->getArgumentVar(par-1); + if (Token::Match(argument->typeStartToken(), "%type% %var% [ %num% ] [,)[]")) { + const Token *tok2 = argument->nameToken()->next(); + + MathLib::bigint argsize = _tokenizer->sizeOfType(argument->typeStartToken()); + if (argsize == 100) // unknown size + argsize = 0; + while (Token::Match(tok2, "[ %num% ] [,)[]")) { + argsize *= MathLib::toLongNumber(tok2->strAt(1)); + tok2 = tok2->tokAt(3); + } + + MathLib::bigint arraysize = arrayInfo.element_size(); + if (arraysize == 100) // unknown size + arraysize = 0; + for (unsigned int i = 0; i < arrayInfo.num().size(); i++) + arraysize *= arrayInfo.num(i); + + if (Token::Match(tok2, "[,)]") && arraysize > 0 && argsize > arraysize) + argumentSizeError(&tok, tok.str(), arrayInfo.varname()); + } + } + } } diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 1f6b530bd..068e20d80 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -225,6 +225,7 @@ private: void arrayIndexThenCheckError(const Token *tok, const std::string &indexName); void possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst, bool cat); void possibleReadlinkBufferOverrunError(const Token *tok, const std::string &funcname, const std::string &varname); + void argumentSizeError(const Token *tok, const std::string &functionName, const std::string &varname); public: void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const { @@ -244,6 +245,7 @@ public: c.arrayIndexThenCheckError(0, "index"); c.possibleBufferOverrunError(0, "source", "destination", false); c.possibleReadlinkBufferOverrunError(0, "readlink", "buffer"); + c.argumentSizeError(0, "function", "array"); } private: diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 4744a1522..3e76464ad 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -133,8 +133,8 @@ private: TEST_CASE(array_index_cast); // FP after cast. #2841 TEST_CASE(array_index_string_literal); - TEST_CASE(buffer_overrun_1); - TEST_CASE(buffer_overrun_2); + TEST_CASE(buffer_overrun_1_standard_functions); + TEST_CASE(buffer_overrun_2_struct); TEST_CASE(buffer_overrun_3); TEST_CASE(buffer_overrun_4); TEST_CASE(buffer_overrun_5); @@ -159,6 +159,7 @@ private: TEST_CASE(buffer_overrun_24); // #4106 TEST_CASE(buffer_overrun_25); // #4096 TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch + TEST_CASE(buffer_overrun_function_array_argument); TEST_CASE(possible_buffer_overrun_1); // #3035 // It is undefined behaviour to point out of bounds of an array @@ -1987,7 +1988,7 @@ private: } - void buffer_overrun_1() { + void buffer_overrun_1_standard_functions() { check("void f()\n" "{\n" " char str[3];\n" @@ -2121,7 +2122,7 @@ private: } - void buffer_overrun_2() { + void buffer_overrun_2_struct() { check("struct ABC\n" "{\n" " char str[5];\n" @@ -2219,7 +2220,7 @@ private: "void f2()\n" "{\n" " char s[3];\n" - " f1(s,3);\n" + " f1(s,20);\n" "}\n"); TODO_ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:3]: (error) Buffer is accessed out of bounds.\n", "", errout.str()); @@ -2670,6 +2671,36 @@ private: ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:3]: (error) Array 'a[10]' accessed at index 100, which is out of bounds.\n", errout.str()); } + void buffer_overrun_function_array_argument() { + check("void f(char a[10]);\n" + "void g() {\n" + " char a[2];\n" + " f(a);\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (style) the array a is too small, the function f expects a bigger array\n", errout.str()); + + check("void f(float a[10][20]);\n" + "void g() {\n" + " float a[2][3];\n" + " f(a);\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (style) the array a is too small, the function f expects a bigger array\n", errout.str()); + + check("void f(char a[20]);\n" + "void g() {\n" + " int a[2];\n" + " f(a);\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (style) the array a is too small, the function f expects a bigger array\n", errout.str()); + + check("void f(char a[20]);\n" + "void g() {\n" + " int a[5];\n" + " f(a);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } + void possible_buffer_overrun_1() { // #3035 check("void foo() {\n" " char * data = (char *)alloca(50);\n"