From 0c469bae415f846444c030d1f7ddb1fd1ee647e7 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Sat, 5 Nov 2011 07:29:53 +0100 Subject: [PATCH] Fixed #3089 (New Check: Detect wrong usage of printf/scanf) --- lib/checkother.cpp | 88 ++++++++++++++++++++++++++++++++++++++++++++++ lib/checkother.h | 11 ++++++ test/testother.cpp | 46 ++++++++++++++++++++++-- 3 files changed, 143 insertions(+), 2 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 802c2b308..79726fa28 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1218,6 +1218,94 @@ void CheckOther::invalidScanfError(const Token *tok) "perl -e 'print \"5\"x2100000' | ./a.out"); } +//--------------------------------------------------------------------------- +// printf("%u", "xyz"); // Wrong argument type. TODO. +// printf("%u%s", 1); // Too few arguments +// printf("", 1); // Too much arguments +//--------------------------------------------------------------------------- + +void CheckOther::checkWrongPrintfScanfArguments() +{ + if (!_settings->isEnabled("style")) + return; + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { + if (!tok->isName()) continue; + + const Token* argListTok = 0; // Points to first va_list argument + std::string formatString; + + if (Token::Match(tok, "printf|scanf ( %str%")) { + formatString = tok->strAt(2); + if (tok->strAt(3) == ",") + argListTok = tok->tokAt(4); + else + argListTok = 0; + } else if (Token::Match(tok, "sprintf|fprintf|sscanf|fscanf ( %any%")) { + const Token* formatStringTok = tok->tokAt(2)->nextArgument(); // Find second parameter (format string) + if (Token::Match(formatStringTok, "%str%")) { + argListTok = formatStringTok->nextArgument(); // Find third parameter (first argument of va_args) + formatString = formatStringTok->str(); + } + } else if (Token::Match(tok, "snprintf|fnprintf ( %any%")) { + const Token* formatStringTok = tok->tokAt(2); + for (int i = 0; i < 2 && formatStringTok; i++) { + formatStringTok = formatStringTok->nextArgument(); // Find third parameter (format string) + } + if (Token::Match(formatStringTok, "%str%")) { + argListTok = formatStringTok->nextArgument(); // Find fourth parameter (first argument of va_args) + formatString = formatStringTok->str(); + } + } else { + continue; + } + + // Count format string parameters.. + unsigned int numFormat = 0; + bool percent = false; + for (std::string::iterator i = formatString.begin(); i != formatString.end(); ++i) { + if (*i == '%') { + percent = !percent; + } else if (percent && std::isalpha(*i)) { + numFormat++; + percent = false; + } + } + + // Count printf/scanf parameters.. + unsigned int numFunction = 0; + while (argListTok) { + // TODO: Perform type checks + numFunction++; + argListTok = argListTok->nextArgument(); // Find next argument + } + + // Mismatching number of parameters => warning + if (numFormat != numFunction) + wrongPrintfScanfArgumentsError(tok, tok->str(), numFormat, numFunction); + } +} + +void CheckOther::wrongPrintfScanfArgumentsError(const Token* tok, + const std::string &functionName, + unsigned int numFormat, + unsigned int numFunction) +{ + std::ostringstream errmsg; + errmsg << functionName + << " format string has " + << numFormat + << " parameters but " + << (numFormat > numFunction ? "only " : "") + << numFunction + << " are given"; + + reportError(tok, + numFormat > numFunction ? Severity::error : Severity::warning, + "wrongPrintfScanfArgs", + errmsg.str()); +} + //--------------------------------------------------------------------------- // if (!x==3) <- Probably meant to be "x!=3" //--------------------------------------------------------------------------- diff --git a/lib/checkother.h b/lib/checkother.h index 7f44fd07a..d43871286 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -89,6 +89,7 @@ public: checkOther.checkMathFunctions(); checkOther.checkFflushOnInputStream(); checkOther.invalidScanf(); + checkOther.checkWrongPrintfScanfArguments(); checkOther.checkCoutCerrMisusage(); checkOther.checkIncorrectLogicOperator(); @@ -167,6 +168,13 @@ public: void invalidScanf(); void invalidScanfError(const Token *tok); + /** @brief %Checks type and number of arguments given to functions like printf or scanf*/ + void checkWrongPrintfScanfArguments(); + void wrongPrintfScanfArgumentsError(const Token* tok, + const std::string &function, + unsigned int numFormat, + unsigned int numFunction); + /** @brief %Check for assigning to the same variable twice in a switch statement*/ void checkRedundantAssignmentInSwitch(); @@ -340,6 +348,7 @@ public: c.bitwiseOnBooleanError(0, "varname", "&&"); c.comparisonOfBoolExpressionWithIntError(0); c.SuspiciousSemicolonError(0); + c.wrongPrintfScanfArgumentsError(0,"printf",3,2); } std::string myName() const { @@ -360,6 +369,7 @@ public: "* sizeof for numeric given as function argument\n" "* incorrect length arguments for 'substr' and 'strncmp'\n" "* invalid usage of output stream. For example: std::cout << std::cout;'\n" + "* too few arguments given to 'printf' or 'scanf;'\n" // style "* C-style pointer cast in cpp file\n" @@ -392,6 +402,7 @@ public: "* testing is unsigned variable is positive\n" "* using bool in bitwise expression\n" "* Suspicious use of ; at the end of 'if/for/while' statement.\n" + "* too much arguments given to 'printf' or 'scanf'\n" // optimisations "* optimisation: detect post increment/decrement\n"; diff --git a/test/testother.cpp b/test/testother.cpp index a4b0a942c..92beef8eb 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -87,6 +87,8 @@ private: TEST_CASE(testScanf1); TEST_CASE(testScanf2); + TEST_CASE(testPrintfArgument); + TEST_CASE(trac1132); TEST_CASE(testMisusedScopeObjectDoesNotPickFunction1); TEST_CASE(testMisusedScopeObjectDoesNotPickFunction2); @@ -1759,7 +1761,8 @@ private: " return b;\n" "}\n"); ASSERT_EQUALS("[test.cpp:6]: (warning) scanf without field width limits can crash with huge input data\n" - "[test.cpp:7]: (warning) scanf without field width limits can crash with huge input data\n", errout.str()); + "[test.cpp:7]: (warning) scanf without field width limits can crash with huge input data\n" + "[test.cpp:8]: (warning) fscanf format string has 0 parameters but 1 are given\n", errout.str()); } void testScanf2() { @@ -1775,7 +1778,46 @@ private: " return b;\n" "}\n"); ASSERT_EQUALS("[test.cpp:6]: (warning) scanf without field width limits can crash with huge input data\n" - "[test.cpp:7]: (warning) scanf without field width limits can crash with huge input data\n", errout.str()); + "[test.cpp:7]: (warning) scanf without field width limits can crash with huge input data\n" + "[test.cpp:8]: (warning) fscanf format string has 0 parameters but 1 are given\n", errout.str()); + } + + void testPrintfArgument() { + check("void foo() {\n" + " printf(\"%u\");\n" + " printf(\"%u%s\", 123);\n" + " printf(\"%u%s%d\", 0, bar());\n" + " printf(\"%u%%%s%d\", 0, bar());\n" + " printf(\"%udfd%%dfa%s%d\", 0, bar());\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (error) printf format string has 1 parameters but only 0 are given\n" + "[test.cpp:3]: (error) printf format string has 2 parameters but only 1 are given\n" + "[test.cpp:4]: (error) printf format string has 3 parameters but only 2 are given\n" + "[test.cpp:5]: (error) printf format string has 3 parameters but only 2 are given\n" + "[test.cpp:6]: (error) printf format string has 3 parameters but only 2 are given\n", errout.str()); + + + + check("void foo() {\n" + " printf(\"\", 0);\n" + " printf(\"%u\", 123, bar());\n" + " printf(\"%u%s\", 0, bar(), 43123);\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (warning) printf format string has 0 parameters but 1 are given\n" + "[test.cpp:3]: (warning) printf format string has 1 parameters but 2 are given\n" + "[test.cpp:4]: (warning) printf format string has 2 parameters but 3 are given\n", errout.str()); + + check("void foo() {\n" + " printf(\"%u\", 0);\n" + " printf(\"%u%s\", 123, bar());\n" + " printf(\"%u%s%d\", 0, bar(), 43123);\n" + " printf(\"%u%%%s%d\", 0, bar(), 43123);\n" + " printf(\"%udfd%%dfa%s%d\", 0, bar(), 43123);\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); } void trac1132() {