diff --git a/lib/checkio.cpp b/lib/checkio.cpp index 5e597bc50..02259b55d 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -645,16 +645,13 @@ void CheckIO::checkFormatString(const Token * const tok, } ++i; } + auto bracketBeg = formatString.end(); if (i != formatString.end() && *i == '[') { + bracketBeg = i; while (i != formatString.end()) { - if (*i == ']') { - if (!skip) { - numFormat++; - if (argListTok) - argListTok = argListTok->nextArgument(); - } + if (*i == ']') break; - } + ++i; } if (scanf_s && !skip) { @@ -663,7 +660,6 @@ void CheckIO::checkFormatString(const Token * const tok, argListTok = argListTok->nextArgument(); } } - _continue = true; } if (i == formatString.end()) break; @@ -684,29 +680,30 @@ void CheckIO::checkFormatString(const Token * const tok, // Perform type checks ArgumentInfo argInfo(argListTok, mSettings, mTokenizer->isCPP()); - if (argInfo.typeToken && !argInfo.isLibraryType(mSettings)) { + if ((argInfo.typeToken && !argInfo.isLibraryType(mSettings)) || *i == ']') { if (scan) { std::string specifier; bool done = false; while (!done) { switch (*i) { case 's': - specifier += *i; + case ']': // charset + specifier += (*i == 's' || bracketBeg == formatString.end()) ? std::string{ 's' } : std::string{ bracketBeg, i + 1 }; if (argInfo.variableInfo && argInfo.isKnownType() && argInfo.variableInfo->isArray() && (argInfo.variableInfo->dimensions().size() == 1) && argInfo.variableInfo->dimensions()[0].known) { if (!width.empty()) { const int numWidth = std::atoi(width.c_str()); if (numWidth != (argInfo.variableInfo->dimension(0) - 1)) - invalidScanfFormatWidthError(tok, numFormat, numWidth, argInfo.variableInfo, 's'); + invalidScanfFormatWidthError(tok, numFormat, numWidth, argInfo.variableInfo, specifier); } } - if (argListTok && argListTok->tokType() != Token::eString && + if (argListTok && argListTok->tokType() != Token::eString && argInfo.typeToken && argInfo.isKnownType() && argInfo.isArrayOrPointer() && (!Token::Match(argInfo.typeToken, "char|wchar_t") || argInfo.typeToken->strAt(-1) == "const")) { if (!(argInfo.isArrayOrPointer() && argInfo.element && !argInfo.typeToken->isStandardType())) invalidScanfArgTypeError_s(tok, numFormat, specifier, &argInfo); } - if (scanf_s) { + if (scanf_s && argInfo.typeToken) { numSecure++; if (argListTok) { argListTok = argListTok->nextArgument(); @@ -719,7 +716,7 @@ void CheckIO::checkFormatString(const Token * const tok, if (!width.empty()) { const int numWidth = std::atoi(width.c_str()); if (numWidth > argInfo.variableInfo->dimension(0)) - invalidScanfFormatWidthError(tok, numFormat, numWidth, argInfo.variableInfo, 'c'); + invalidScanfFormatWidthError(tok, numFormat, numWidth, argInfo.variableInfo, std::string(1, *i)); } } if (scanf_s) { @@ -1995,7 +1992,7 @@ void CheckIO::invalidLengthModifierError(const Token* tok, nonneg int numFormat, reportError(tok, Severity::warning, "invalidLengthModifierError", errmsg.str(), CWE704, Certainty::normal); } -void CheckIO::invalidScanfFormatWidthError(const Token* tok, nonneg int numFormat, int width, const Variable *var, char c) +void CheckIO::invalidScanfFormatWidthError(const Token* tok, nonneg int numFormat, int width, const Variable *var, const std::string& specifier) { MathLib::bigint arrlen = 0; std::string varname; @@ -2014,7 +2011,7 @@ void CheckIO::invalidScanfFormatWidthError(const Token* tok, nonneg int numForma reportError(tok, Severity::warning, "invalidScanfFormatWidth_smaller", errmsg.str(), CWE(0U), Certainty::inconclusive); } else { errmsg << "Width " << width << " given in format string (no. " << numFormat << ") is larger than destination buffer '" - << varname << "[" << arrlen << "]', use %" << (c == 'c' ? arrlen : (arrlen - 1)) << c << " to prevent overflowing it."; + << varname << "[" << arrlen << "]', use %" << (specifier == "c" ? arrlen : (arrlen - 1)) << specifier << " to prevent overflowing it."; reportError(tok, Severity::error, "invalidScanfFormatWidth", errmsg.str(), CWE687, Certainty::normal); } } diff --git a/lib/checkio.h b/lib/checkio.h index 5437e0c97..661e7b3f9 100644 --- a/lib/checkio.h +++ b/lib/checkio.h @@ -130,7 +130,7 @@ private: void invalidPrintfArgTypeError_sint(const Token* tok, nonneg int numFormat, const std::string& specifier, const ArgumentInfo* argInfo); void invalidPrintfArgTypeError_float(const Token* tok, nonneg int numFormat, const std::string& specifier, const ArgumentInfo* argInfo); void invalidLengthModifierError(const Token* tok, nonneg int numFormat, const std::string& modifier); - void invalidScanfFormatWidthError(const Token* tok, nonneg int numFormat, int width, const Variable *var, char c); + void invalidScanfFormatWidthError(const Token* tok, nonneg int numFormat, int width, const Variable *var, const std::string& specifier); static void argumentType(std::ostream & os, const ArgumentInfo * argInfo); static Severity::SeverityType getSeverity(const ArgumentInfo *argInfo); @@ -157,8 +157,8 @@ private: c.invalidPrintfArgTypeError_sint(nullptr, 1, "i", nullptr); c.invalidPrintfArgTypeError_float(nullptr, 1, "f", nullptr); c.invalidLengthModifierError(nullptr, 1, "I"); - c.invalidScanfFormatWidthError(nullptr, 10, 5, nullptr, 's'); - c.invalidScanfFormatWidthError(nullptr, 99, -1, nullptr, 's'); + c.invalidScanfFormatWidthError(nullptr, 10, 5, nullptr, "s"); + c.invalidScanfFormatWidthError(nullptr, 99, -1, nullptr, "s"); c.wrongPrintfScanfPosixParameterPositionError(nullptr, "printf", 2, 1); } diff --git a/test/testio.cpp b/test/testio.cpp index 8060b739a..19a49e40a 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -50,6 +50,7 @@ private: TEST_CASE(testScanf2); TEST_CASE(testScanf3); // #3494 TEST_CASE(testScanf4); // #ticket 2553 + TEST_CASE(testScanf5); // #10632 TEST_CASE(testScanfArgument); TEST_CASE(testPrintfArgument); @@ -774,6 +775,15 @@ private: ASSERT_EQUALS("[test.cpp:4]: (error) Width 70 given in format string (no. 1) is larger than destination buffer 'str[8]', use %7s to prevent overflowing it.\n", errout.str()); } + void testScanf5() { // #10632 + check("char s1[42], s2[42];\n" + "void test() {\n" + " scanf(\"%42s%42[a-z]\", s1, s2);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Width 42 given in format string (no. 1) is larger than destination buffer 's1[42]', use %41s to prevent overflowing it.\n" + "[test.cpp:3]: (error) Width 42 given in format string (no. 2) is larger than destination buffer 's2[42]', use %41[a-z] to prevent overflowing it.\n", errout.str()); + } + #define TEST_SCANF_CODE(format, type) \ "void f(){" type " x; scanf(\"" format "\", &x);}" diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index 8715951a2..4d0463979 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -1929,7 +1929,7 @@ private: "}"); ASSERT_EQUALS("", errout.str()); - check("struct nc_rpc nc_rpc_getconfig() {\n" // #13082 + check("struct nc_rpc nc_rpc_getconfig() {\n" // #10382 " struct nc_rpc rpc;\n" " rpc->filter = malloc(1);\n" " return (nc_rpc)rpc;\n"