From 613dc19b68ba01103f807c3cd62db6759a721e7a Mon Sep 17 00:00:00 2001 From: rikardfalkeborn Date: Sun, 14 Oct 2018 18:49:34 +0200 Subject: [PATCH] #4241: Check for address of single character passed as string (#1381) * #4241: Check for address of single character passed as string Add a check that address of a single character is not passed as argument to argument marked as strings (using strz). The check does not warn if the address of a character with known value '\0'. Since ValueFlow currently does not handle global constants (see #7597), do not warn if the variable is global to avoid FPs when the address of a global variable assigned to '\0' is passed to a function expecting a string. Remove comment in docs saying strz is unused. * Change asdf to Hello world * Add test of address to first element in string * Add error reporting function to getErrorMessages * Fix strings in test --- lib/checkfunctions.cpp | 19 +++++++++ lib/checkfunctions.h | 2 + man/manual.docbook | 3 +- test/cfg/std.c | 18 +++++++++ test/testfunctions.cpp | 92 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 132 insertions(+), 2 deletions(-) diff --git a/lib/checkfunctions.cpp b/lib/checkfunctions.cpp index 2b9ee5789..380b4ce7a 100644 --- a/lib/checkfunctions.cpp +++ b/lib/checkfunctions.cpp @@ -125,6 +125,17 @@ void CheckFunctions::invalidFunctionUsage() else if (!mSettings->library.isIntArgValid(functionToken, argnr, 1)) invalidFunctionArgError(argtok, functionToken->str(), argnr, nullptr, mSettings->library.validarg(functionToken, argnr)); } + + if (mSettings->library.isargstrz(functionToken, argnr)) { + if (Token::Match(argtok, "& %var%") && argtok->next() && argtok->next()->valueType()) { + const ValueType * valueType = argtok->next()->valueType(); + const Variable * variable = argtok->next()->variable(); + if (valueType->type == ValueType::Type::CHAR && !variable->isGlobal() && + (!argtok->next()->hasKnownValue() || argtok->next()->getValue(0) == nullptr)) { + invalidFunctionArgStrError(argtok, functionToken->str(), argnr); + } + } + } } } } @@ -167,6 +178,14 @@ void CheckFunctions::invalidFunctionArgBoolError(const Token *tok, const std::st reportError(tok, Severity::error, "invalidFunctionArgBool", errmsg.str(), CWE628, false); } +void CheckFunctions::invalidFunctionArgStrError(const Token *tok, const std::string &functionName, unsigned int argnr) +{ + std::ostringstream errmsg; + errmsg << "$symbol:" << functionName << '\n'; + errmsg << "Invalid $symbol() argument nr " << argnr << ". A nul-terminated string is required."; + reportError(tok, Severity::error, "invalidFunctionArgStr", errmsg.str(), CWE628, false); +} + //--------------------------------------------------------------------------- // Check for ignored return values. //--------------------------------------------------------------------------- diff --git a/lib/checkfunctions.h b/lib/checkfunctions.h index e3420e2a2..f582a4835 100644 --- a/lib/checkfunctions.h +++ b/lib/checkfunctions.h @@ -110,6 +110,7 @@ public: private: void invalidFunctionArgError(const Token *tok, const std::string &functionName, int argnr, const ValueFlow::Value *invalidValue, const std::string &validstr); void invalidFunctionArgBoolError(const Token *tok, const std::string &functionName, int argnr); + void invalidFunctionArgStrError(const Token *tok, const std::string &functionName, unsigned int argnr); void ignoredReturnValueError(const Token* tok, const std::string& function); void mathfunctionCallWarning(const Token *tok, const unsigned int numParam = 1); void mathfunctionCallWarning(const Token *tok, const std::string& oldexp, const std::string& newexp); @@ -126,6 +127,7 @@ private: c.invalidFunctionArgError(nullptr, "func_name", 1, nullptr,"1:4"); c.invalidFunctionArgBoolError(nullptr, "func_name", 1); + c.invalidFunctionArgStrError(nullptr, "func_name", 1); c.ignoredReturnValueError(nullptr, "malloc"); c.mathfunctionCallWarning(nullptr); c.mathfunctionCallWarning(nullptr, "1 - erf(x)", "erfc(x)"); diff --git a/man/manual.docbook b/man/manual.docbook index 314534c22..52f090e4b 100644 --- a/man/manual.docbook +++ b/man/manual.docbook @@ -1586,8 +1586,7 @@ Checking minsize.c...
strz - This setting is not used by Cppcheck currently. But with this - you can say that an argument must be a zero-terminated + With this you can say that an argument must be a zero-terminated string. <?xml version="1.0"?> diff --git a/test/cfg/std.c b/test/cfg/std.c index a238c112f..faecabce7 100644 --- a/test/cfg/std.c +++ b/test/cfg/std.c @@ -3797,6 +3797,24 @@ void invalidFunctionArg(char c) (void)toupper(255); } +void invalidFunctionArgString(char c) +{ + /* cppcheck-suppress invalidFunctionArgStr */ + (void)atoi(&c); + char x = 'x'; + /* cppcheck-suppress invalidFunctionArgStr */ + (void)strlen(&x); + + char y = '\0'; + (void)strlen(&y); + + // #5225 + char str[80] = "hello worl"; + char d='d'; + /* cppcheck-suppress invalidFunctionArgStr */ + (void)strcat(str,&d); +} + void ignoredReturnValue_abs(int i) { // cppcheck-suppress ignoredReturnValue diff --git a/test/testfunctions.cpp b/test/testfunctions.cpp index cbcb448bd..20cc21082 100644 --- a/test/testfunctions.cpp +++ b/test/testfunctions.cpp @@ -62,6 +62,7 @@ private: // Invalid function usage TEST_CASE(invalidFunctionUsage1); + TEST_CASE(invalidFunctionUsageStrings); // Math function usage TEST_CASE(mathfunctionCall_fmod); @@ -447,6 +448,97 @@ private: ASSERT_EQUALS("", errout.str()); } + void invalidFunctionUsageStrings() { + check("size_t f() { char x = 'x'; return strlen(&x); }"); + ASSERT_EQUALS("[test.cpp:1]: (error) Invalid strlen() argument nr 1. A nul-terminated string is required.\n", errout.str()); + + check("size_t f() { return strlen(&x); }"); + ASSERT_EQUALS("", errout.str()); + + check("size_t f(char x) { return strlen(&x); }"); + ASSERT_EQUALS("[test.cpp:1]: (error) Invalid strlen() argument nr 1. A nul-terminated string is required.\n", errout.str()); + + check("size_t f() { char x = '\\0'; return strlen(&x); }"); + ASSERT_EQUALS("", errout.str()); + + check("size_t f() {\n" + " char x;\n" + " if (y)\n" + " x = '\\0';\n" + " else\n" + " x = 'a';\n" + " return strlen(&x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:7]: (error) Invalid strlen() argument nr 1. A nul-terminated string is required.\n", errout.str()); + + check("int f() { char x = '\\0'; return strcmp(\"Hello world\", &x); }"); + ASSERT_EQUALS("", errout.str()); + + check("int f() { char x = 'x'; return strcmp(\"Hello world\", &x); }"); + ASSERT_EQUALS("[test.cpp:1]: (error) Invalid strcmp() argument nr 2. A nul-terminated string is required.\n", errout.str()); + + check("size_t f(char x) { char * y = &x; return strlen(y) }"); + ASSERT_EQUALS("[test.cpp:1]: (error) Invalid strlen() argument nr 1. A nul-terminated string is required.\n", errout.str()); + + check("size_t f(char x) { char * y = &x; char *z = y; return strlen(z) }"); + ASSERT_EQUALS("[test.cpp:1]: (error) Invalid strlen() argument nr 1. A nul-terminated string is required.\n", errout.str()); + + check("size_t f() { char x = 'x'; char * y = &x; char *z = y; return strlen(z) }"); + ASSERT_EQUALS("[test.cpp:1]: (error) Invalid strlen() argument nr 1. A nul-terminated string is required.\n", errout.str()); + + check("size_t f() { char x = '\\0'; char * y = &x; char *z = y; return strlen(z) }"); + ASSERT_EQUALS("", errout.str()); + + check("size_t f() { char x[] = \"Hello world\"; return strlen(x) }"); + ASSERT_EQUALS("", errout.str()); + + check("size_t f(char x[]) { return strlen(x) }"); + ASSERT_EQUALS("", errout.str()); + + check("int f(char x, char y) { return strcmp(&x, &y); }"); + ASSERT_EQUALS("[test.cpp:1]: (error) Invalid strcmp() argument nr 1. A nul-terminated string is required.\n" + "[test.cpp:1]: (error) Invalid strcmp() argument nr 2. A nul-terminated string is required.\n", errout.str()); + + check("size_t f() { char x[] = \"Hello world\"; return strlen(&x[0]) }"); + ASSERT_EQUALS("", errout.str()); + + check("struct S {\n" + " char x;\n" + "};\n" + "size_t f() {\n" + " S s1 = {0};\n" + " S s2;\n;" + " s2.x = 'x';\n" + " size_t l1 = strlen(&s1.x);\n" + " size_t l2 = strlen(&s2.x);\n" + " return l1 + l2;\n" + "}\n"); + TODO_ASSERT_EQUALS("[test.cpp:9]: (error) Invalid strlen() argument nr 1. A nul-terminated string is required.\n", "", errout.str()); + + check("const char x = 'x'; size_t f() { return strlen(&x); }"); + TODO_ASSERT_EQUALS("[test.cpp:1]: (error) Invalid strlen() argument nr 1. A nul-terminated string is required.\n", "", errout.str()); + + check("const char x = 'x'; size_t f() { char y = x; return strlen(&y); }"); + ASSERT_EQUALS("[test.cpp:1]: (error) Invalid strlen() argument nr 1. A nul-terminated string is required.\n", errout.str()); + + check("const char x = '\\0'; size_t f() { return strlen(&x); }"); + ASSERT_EQUALS("", errout.str()); + + check("const char x = '\\0'; size_t f() { char y = x; return strlen(&y); }"); + ASSERT_EQUALS("", errout.str()); + + // #5225 + check("int main(void)\n" + "{\n" + " char str[80] = \"hello worl\";\n" + " char d = 'd';\n" + " strcat(str, &d);\n" + " puts(str);\n" + " return 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) Invalid strcat() argument nr 2. A nul-terminated string is required.\n", errout.str()); + } + void mathfunctionCall_sqrt() { // sqrt, sqrtf, sqrtl check("void foo()\n"