diff --git a/lib/checkother.cpp b/lib/checkother.cpp index ad77bd89f..04bf212ef 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -431,6 +431,51 @@ void CheckOther::invalidFunctionUsage() } //--------------------------------------------------------------------------- +void CheckOther::invalidScanf() +{ + if (!_settings->_checkCodingStyle) + return; + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + const Token *formatToken = 0; + if (Token::Match(tok, "scanf|vscanf ( %str% ,")) + formatToken = tok->tokAt(2); + else if (Token::Match(tok, "fscanf|vfscanf ( %var% , %str% ,")) + formatToken = tok->tokAt(4); + else + continue; + + bool format = false; + + // scan the string backwards, so we dont need to keep states + const std::string &formatstr(formatToken->str()); + for (unsigned int i = 1; i < formatstr.length(); i++) + { + if (formatstr[i] == '%') + format = !format; + + else if (!format) + continue; + + else if (std::isdigit(formatstr[i])) + { + format = false; + } + + else if (std::isalpha(formatstr[i])) + { + invalidScanfError(tok); + format = false; + } + } + } +} + +void CheckOther::invalidScanfError(const Token *tok) +{ + reportError(tok, Severity::style, + "invalidscanf", "scanf without field width limits can crash with huge input data"); +} //--------------------------------------------------------------------------- // Check for unsigned divisions diff --git a/lib/checkother.h b/lib/checkother.h index cd560e71d..3a4182809 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -82,6 +82,7 @@ public: checkOther.checkZeroDivision(); checkOther.checkMathFunctions(); checkOther.checkFflushOnInputStream(); + checkOther.invalidScanf(); checkOther.nullConstantDereference(); @@ -184,6 +185,10 @@ public: void sizeofCalculation(); void sizeofCalculationError(const Token *tok); + /** @brief scanf can crash if width specifiers are not used */ + void invalidScanf(); + void invalidScanfError(const Token *tok); + /** @brief %Check for assigning to the same variable twice in a switch statement*/ void checkRedundantAssignmentInSwitch(); @@ -246,6 +251,7 @@ public: sizeofCalculationError(0); emptyCatchBlockError(0); redundantAssignmentInSwitchError(0, "varname"); + invalidScanfError(0); // optimisations postIncrementError(0, "varname", true); @@ -273,6 +279,7 @@ public: "* redundant if\n" "* bad usage of the function 'strtol'\n" "* [[CheckUnsignedDivision|unsigned division]]\n" + "* Dangerous usage of 'scanf'\n" "* unused struct member\n" "* passing parameter by value\n" "* [[IncompleteStatement|Incomplete statement]]\n" diff --git a/test/testother.cpp b/test/testother.cpp index 343fad459..f8bc7fde5 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -103,6 +103,9 @@ private: TEST_CASE(emptyCatchBlock); TEST_CASE(switchRedundantAssignmentTest); + + TEST_CASE(testScanf1); + TEST_CASE(testScanf2); } void check(const char code[]) @@ -133,6 +136,7 @@ private: checkOther.checkMathFunctions(); checkOther.checkEmptyStringTest(); checkOther.checkFflushOnInputStream(); + checkOther.invalidScanf(); } @@ -3007,6 +3011,40 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); } + + void testScanf1() + { + check("#include \n" + "int main(int argc, char **argv)\n" + "{\n" + " int a, b;\n" + " FILE *file = fopen(\"test\", \"r\");\n" + " b = fscanf(file, \"aa %ds\", &a);\n" + " c = scanf(\"aa %ds\", &a);\n" + " b = fscanf(file, \"aa%%ds\", &a);\n" + " fclose(file);\n" + " return b;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:6]: (style) scanf without field width limits can crash with huge input data\n" + "[test.cpp:7]: (style) scanf without field width limits can crash with huge input data\n", errout.str()); + } + + void testScanf2() + { + check("#include \n" + "int main(int argc, char **argv)\n" + "{\n" + " int a, b;\n" + " FILE *file = fopen(\"test\", \"r\");\n" + " b = fscanf(file, \"aa%%%ds\", &a);\n" + " c = scanf(\"aa %%%ds\", &a);\n" + " b = fscanf(file, \"aa%%ds\", &a);\n" + " fclose(file);\n" + " return b;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:6]: (style) scanf without field width limits can crash with huge input data\n" + "[test.cpp:7]: (style) scanf without field width limits can crash with huge input data\n", errout.str()); + } }; REGISTER_TEST(TestOther)