diff --git a/lib/checkio.cpp b/lib/checkio.cpp index 30553f5a7..394e42d70 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -288,6 +288,7 @@ void CheckIO::invalidScanf() { if (!_settings->isEnabled("style")) return; + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { const Token *formatToken = 0; if (Token::Match(tok, "scanf|vscanf ( %str% ,")) @@ -312,40 +313,60 @@ void CheckIO::invalidScanf() else if (!format) continue; - else if (std::isdigit(formatstr[i])) { + else if (std::isdigit(formatstr[i]) || formatstr[i] == '*') { format = false; } - else if (std::isalpha(formatstr[i])) { - if (formatstr[i] != 'c') // #3490 - field width limits are not necessary for %c - invalidScanfError(tok); + else if (std::isalpha(formatstr[i]) || formatstr[i] == '[') { + if (formatstr[i] == 's' || formatstr[i] == '[' || formatstr[i] == 'S' || (formatstr[i] == 'l' && formatstr[i+1] == 's')) // #3490 - field width limits are only necessary for string input + invalidScanfError(tok, false); + else if (formatstr[i] != 'n' && formatstr[i] != 'c' && _settings->platformType != Settings::Win32A && _settings->platformType != Settings::Win32W && _settings->platformType != Settings::Win64 && _settings->isEnabled("portability")) + invalidScanfError(tok, true); // Warn about libc bug in versions prior to 2.13-25 format = false; } } } } -void CheckIO::invalidScanfError(const Token *tok) +void CheckIO::invalidScanfError(const Token *tok, bool portability) { - reportError(tok, Severity::warning, - "invalidscanf", "scanf without field width limits can crash with huge input data\n" - "scanf without field width limits can crash with huge input data. To fix this error " - "message add a field width specifier:\n" - " %s => %20s\n" - " %i => %3i\n" - "\n" - "Sample program that can crash:\n" - "\n" - "#include \n" - "int main()\n" - "{\n" - " int a;\n" - " scanf(\"%i\", &a);\n" - " return 0;\n" - "}\n" - "\n" - "To make it crash:\n" - "perl -e 'print \"5\"x2100000' | ./a.out"); + if (portability) + reportError(tok, Severity::portability, + "invalidscanf", "scanf without field width limits can crash with huge input data on some versions of libc.\n" + "scanf without field width limits can crash with huge input data on libc versions older than 2.13-25. Add a field " + "width specifier to fix this problem:\n" + " %i => %3i\n" + "\n" + "Sample program that can crash:\n" + "\n" + "#include \n" + "int main()\n" + "{\n" + " int a;\n" + " scanf(\"%i\", &a);\n" + " return 0;\n" + "}\n" + "\n" + "To make it crash:\n" + "perl -e 'print \"5\"x2100000' | ./a.out"); + else + reportError(tok, Severity::warning, + "invalidscanf", "scanf without field width limits can crash with huge input data.\n" + "scanf without field width limits can crash with huge input data. Add a field width " + "specifier to fix this problem:\n" + " %s => %20s\n" + "\n" + "Sample program that can crash:\n" + "\n" + "#include \n" + "int main()\n" + "{\n" + " char c[5];\n" + " scanf(\"%s\", c);\n" + " return 0;\n" + "}\n" + "\n" + "To make it crash, type in more than 5 characters."); } //--------------------------------------------------------------------------- @@ -481,9 +502,7 @@ void CheckIO::checkWrongPrintfScanfArguments() 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 { + if (!width.empty()) { int numWidth = std::atoi(width.c_str()); if (numWidth != (variableInfo->dimension(0) - 1)) invalidScanfFormatWidthError(tok, tok->str(), numFormat, numWidth, variableInfo); @@ -615,14 +634,6 @@ 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; diff --git a/lib/checkio.h b/lib/checkio.h index d0b9af0d9..92ec8c0dd 100644 --- a/lib/checkio.h +++ b/lib/checkio.h @@ -77,7 +77,7 @@ private: void readWriteOnlyFileError(const Token *tok); void writeReadOnlyFileError(const Token *tok); void useClosedFileError(const Token *tok); - void invalidScanfError(const Token *tok); + void invalidScanfError(const Token *tok, bool portability); void wrongPrintfScanfArgumentsError(const Token* tok, const std::string &function, unsigned int numFormat, @@ -88,7 +88,6 @@ 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 { @@ -100,7 +99,7 @@ private: c.readWriteOnlyFileError(0); c.writeReadOnlyFileError(0); c.useClosedFileError(0); - c.invalidScanfError(0); + c.invalidScanfError(0, false); c.wrongPrintfScanfArgumentsError(0,"printf",3,2); c.invalidScanfArgTypeError(0, "scanf", 1); c.invalidPrintfArgTypeError_s(0, 1); @@ -108,7 +107,6 @@ 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); } diff --git a/test/testio.cpp b/test/testio.cpp index cfe9856e7..28f99c5bf 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -42,19 +42,21 @@ private: TEST_CASE(testScanf1); // Scanf without field limiters TEST_CASE(testScanf2); TEST_CASE(testScanf3); - TEST_CASE(testScanf4); TEST_CASE(testScanfArgument); TEST_CASE(testPrintfArgument); } - void check(const char code[], bool inconclusive = false) { + void check(const char code[], bool inconclusive = false, bool portability = false, Settings::PlatformType platform = Settings::Unspecified) { // Clear the error buffer.. errout.str(""); Settings settings; settings.addEnabled("style"); + if (portability) + settings.addEnabled("portability"); settings.inconclusive = inconclusive; + settings.platform(platform); // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -354,70 +356,46 @@ private: void testScanf1() { - check("#include \n" - "int main(int argc, char **argv)\n" - "{\n" + check("void foo() {\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" + " a = fscanf(file, \"aa %s\", bar);\n" + " b = scanf(\"aa %S\", bar);\n" + " b = scanf(\"aa %ls\", bar);\n" + " sscanf(foo, \"%[^~]\", bar);\n" + " scanf(\"%dx%s\", &b, bar);\n" " fclose(file);\n" - " return b;\n" "}"); - ASSERT_EQUALS("[test.cpp:8]: (warning) fscanf format string has 0 parameters but 1 are given\n" - "[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()); + ASSERT_EQUALS("[test.cpp:4]: (warning) scanf without field width limits can crash with huge input data.\n" + "[test.cpp:5]: (warning) scanf without field width limits can crash with huge input data.\n" + "[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" + "[test.cpp:8]: (warning) 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" + check("void foo() {\n" + " scanf(\"%5s\", bar);\n" // Width specifier given + " scanf(\"%5[^~]\", bar);\n" // Width specifier given + " scanf(\"aa%%s\", bar);\n" // No %s + " scanf(\"aa%d\", &a);\n" // No %s + " scanf(\"aa%ld\", &a);\n" // No %s + " scanf(\"%*[^~]\");\n" // Ignore input "}"); - ASSERT_EQUALS("[test.cpp:8]: (warning) fscanf format string has 0 parameters but 1 are given\n" - "[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()); + ASSERT_EQUALS("[test.cpp:4]: (warning) scanf format string has 0 parameters but 1 are given\n", errout.str()); } void testScanf3() { - check("#include \n" - "int main(int argc, char **argv)\n" - "{\n" - " char a[32];\n" - " int b, c;\n" - " FILE *file = fopen(\"test\", \"r\");\n" - " c = fscanf(file, \"%[^ ] %d\n\", a, &b);\n" - " fclose(file);\n" - " return c;\n" - "}"); - ASSERT_EQUALS("", errout.str()); + check("void foo() {\n" + " scanf(\"%d\", &a);\n" + " scanf(\"%n\", &a);\n" // No warning on %n, since it doesn't expect user input + " scanf(\"%c\", &c);\n" // No warning on %c; it expects only one character + "}", false, true, Settings::Unspecified); + ASSERT_EQUALS("[test.cpp:2]: (portability) scanf without field width limits can crash with huge input data on some versions of libc.\n", errout.str()); - check("#include \n" - "int main(int argc, char **argv)\n" - "{\n" - " char a[32];\n" - " int b;\n" - " FILE *file = fopen(\"test\", \"r\");\n" - " b = fscanf(file, \"%[^ \n\", a);\n" - " fclose(file);\n" - " return b;\n" - "}"); - ASSERT_EQUALS("[test.cpp:7]: (warning) fscanf format string has 0 parameters but 1 are given\n", errout.str()); - } - - void testScanf4() { - check("void f() {\n" - " char c;\n" - " scanf(\"%c\", &c);\n" - "}"); + check("void foo() {\n" + " scanf(\"%d\", &a);\n" + "}", false, true, Settings::Win32A); ASSERT_EQUALS("", errout.str()); } @@ -431,7 +409,7 @@ private: " scanf(\"%1u%1u\", &foo, bar());\n" " scanf(\"%*1x %1x %29s\", &count, KeyName);\n" // #3373 " fscanf(f, \"%7ms\", &ref);\n" // #3461 - " sscanf(ip_port, \"%*[^:]:%d\", &port);\n" // #3468 + " sscanf(ip_port, \"%*[^:]:%4d\", &port);\n" // #3468 "}"); ASSERT_EQUALS("", errout.str()); @@ -460,14 +438,11 @@ private: 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()); + ASSERT_EQUALS("[test.cpp:6]: (error) sscanf width 5 in format string (no. 1) is larger than destination, use %4s to prevent overflowing destination: output[5]\n", errout.str()); check("void foo() {\n" " char input[10];\n" @@ -477,10 +452,9 @@ private: " 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" + ASSERT_EQUALS("[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()); + "[test.cpp:4]: (warning) scanf without field width limits can crash with huge input data.\n", errout.str()); } void testPrintfArgument() {