From dc4982115a013438a816158f3d13fa73da2f57b4 Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Sat, 23 Jun 2012 07:52:52 +0200 Subject: [PATCH] Improved checking of scanf format strings --- lib/checkio.cpp | 47 ++++++++++++++++++++++++++++++++++++++++++++++- lib/checkio.h | 7 ++++++- test/testio.cpp | 28 +++++++++++++++++++++++++++- 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/lib/checkio.cpp b/lib/checkio.cpp index 8322190b9..951f427d4 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -25,6 +25,7 @@ #include "symboldatabase.h" #include +#include //--------------------------------------------------------------------------- @@ -443,6 +444,7 @@ void CheckIO::checkWrongPrintfScanfArguments() percent = false; bool _continue = false; + std::string width; while (i != formatString.end() && *i != ']' && !std::isalpha(*i)) { if (*i == '*') { if (scan) @@ -452,7 +454,8 @@ void CheckIO::checkWrongPrintfScanfArguments() if (argListTok) argListTok = argListTok->nextArgument(); } - } + } else if (std::isdigit(*i)) + width += *i; ++i; } if (i == formatString.end()) @@ -473,6 +476,16 @@ void CheckIO::checkWrongPrintfScanfArguments() if (scan && varTypeTok) { if ((!variableInfo->isPointer() && !variableInfo->isArray()) || varTypeTok->strAt(-1) == "const") invalidScanfArgTypeError(tok, tok->str(), numFormat); + + if (*i == 's' && variableInfo && isKnownType(variableInfo, varTypeTok) && variableInfo->isArray() && (variableInfo->dimensions().size() == 1)) { + if (width.empty()) + missingScanfFormatWidthError(tok, tok->str(), numFormat, variableInfo); + else { + int numWidth = std::atoi(width.c_str()); + if (numWidth != (variableInfo->dimension(0) - 1)) + invalidScanfFormatWidthError(tok, tok->str(), numFormat, numWidth, variableInfo); + } + } } else if (!scan) { switch (*i) { case 's': @@ -599,3 +612,35 @@ void CheckIO::invalidPrintfArgTypeError_float(const Token* tok, unsigned int num errmsg << "%" << c << " in format string (no. " << numFormat << ") requires a floating point number given in the argument list"; reportError(tok, Severity::warning, "invalidPrintfArgType_float", errmsg.str()); } +void CheckIO::missingScanfFormatWidthError(const Token* tok, const std::string &functionName, unsigned int numFormat, const Variable *var) +{ + std::ostringstream errmsg; + errmsg << functionName << " %s in format string (no. " << numFormat << ") does not specify a width"; + if (var) + errmsg << ", use %" << (var->dimension(0) - 1) << "s to prevent overflowing destination: " << var->name() << "[" << var->dimension(0) << "]"; + reportError(tok, Severity::warning, "missingScanfFormatWidth", errmsg.str()); +} +void CheckIO::invalidScanfFormatWidthError(const Token* tok, const std::string &functionName, unsigned int numFormat, int width, const Variable *var) +{ + std::ostringstream errmsg; + Severity::SeverityType severity = Severity::warning; + bool inconclusive = false; + + if (var) { + if (var->dimension(0) > width) { + if (!_settings->inconclusive) + return; + inconclusive = true; + errmsg << functionName << " width " << width << " in format string (no. " << numFormat << ") is smaller than destination" + << ": " << var->name() << "[" << var->dimension(0) << "]"; + } else { + errmsg << functionName << " width " << width << " in format string (no. " << numFormat << ") is larger than destination" + << ", use %" << (var->dimension(0) - 1) << "s to prevent overflowing destination: " << var->name() << "[" << var->dimension(0) << "]"; + severity = Severity::error; + } + + } else + errmsg << functionName << " width " << width << " in format string (no. " << numFormat << ") doesn't match destination"; + + reportError(tok, severity, "invalidScanfFormatWidth", errmsg.str(), inconclusive); +} diff --git a/lib/checkio.h b/lib/checkio.h index 002874c97..d0b9af0d9 100644 --- a/lib/checkio.h +++ b/lib/checkio.h @@ -24,10 +24,11 @@ #include "check.h" #include "config.h" +class Variable; + /// @addtogroup Checks /// @{ - /** @brief %Check input output operations. */ class CPPCHECKLIB CheckIO : public Check { public: @@ -87,6 +88,8 @@ private: void invalidPrintfArgTypeError_p(const Token* tok, unsigned int numFormat); void invalidPrintfArgTypeError_int(const Token* tok, unsigned int numFormat, char c); void invalidPrintfArgTypeError_float(const Token* tok, unsigned int numFormat, char c); + void missingScanfFormatWidthError(const Token* tok, const std::string &functionName, unsigned int numFormat, const Variable *var); + void invalidScanfFormatWidthError(const Token* tok, const std::string &functionName, unsigned int numFormat, int width, const Variable *var); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const { CheckIO c(0, settings, errorLogger); @@ -105,6 +108,8 @@ private: c.invalidPrintfArgTypeError_p(0, 1); c.invalidPrintfArgTypeError_int(0, 1, 'u'); c.invalidPrintfArgTypeError_float(0, 1, 'f'); + c.missingScanfFormatWidthError(0, "scanf", 1, NULL); + c.invalidScanfFormatWidthError(0, "scanf", 10, 5, NULL); } std::string myName() const { diff --git a/test/testio.cpp b/test/testio.cpp index 6ef863fd9..cee49cfd1 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -48,12 +48,13 @@ private: TEST_CASE(testPrintfArgument); } - void check(const char code[]) { + void check(const char code[], bool inconclusive = false) { // Clear the error buffer.. errout.str(""); Settings settings; settings.addEnabled("style"); + settings.inconclusive = inconclusive; // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -443,6 +444,31 @@ private: "[test.cpp:3]: (error) scanf format string has 2 parameters but only 1 are given\n" "[test.cpp:4]: (error) sscanf format string has 2 parameters but only 1 are given\n" "[test.cpp:5]: (error) scanf format string has 2 parameters but only 1 are given\n", errout.str()); + + check("void foo() {\n" + " char input[10];\n" + " char output[5];\n" + " sscanf(input, \"%s\", output);\n" + " sscanf(input, \"%3s\", output);\n" + " sscanf(input, \"%4s\", output);\n" + " sscanf(input, \"%5s\", output);\n" + "}", false); + ASSERT_EQUALS("[test.cpp:4]: (warning) sscanf %s in format string (no. 1) does not specify a width, use %4s to prevent overflowing destination: output[5]\n" + "[test.cpp:7]: (error) sscanf width 5 in format string (no. 1) is larger than destination, use %4s to prevent overflowing destination: output[5]\n" + "[test.cpp:4]: (warning) scanf without field width limits can crash with huge input data\n", errout.str()); + + check("void foo() {\n" + " char input[10];\n" + " char output[5];\n" + " sscanf(input, \"%s\", output);\n" + " sscanf(input, \"%3s\", output);\n" + " sscanf(input, \"%4s\", output);\n" + " sscanf(input, \"%5s\", output);\n" + "}", true); + ASSERT_EQUALS("[test.cpp:4]: (warning) sscanf %s in format string (no. 1) does not specify a width, use %4s to prevent overflowing destination: output[5]\n" + "[test.cpp:5]: (warning, inconclusive) sscanf width 3 in format string (no. 1) is smaller than destination: output[5]\n" + "[test.cpp:7]: (error) sscanf width 5 in format string (no. 1) is larger than destination, use %4s to prevent overflowing destination: output[5]\n" + "[test.cpp:4]: (warning) scanf without field width limits can crash with huge input data\n", errout.str()); } void testPrintfArgument() {