walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Refactorized checking of scanf field width specifiers (#3946): 

- Removed duplicate check
- Changed severity to portability, when a crash only happens with certain libc versions
- Fixed handling of * in format string (#3877)
- Added support for [...] pattern
- Removed garbage from tests
This commit is contained in:
PKEuS 2012-07-07 04:34:37 -07:00
parent dec4844c10
commit d3c44c20ff
3 changed files with 84 additions and 101 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
lib/checkio.cpp
View File

@ -288,6 +288,7 @@ void CheckIO::invalidScanf()
{ {
if (!_settings->isEnabled("style")) if (!_settings->isEnabled("style"))
return; return;
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
const Token *formatToken = 0; const Token *formatToken = 0;
if (Token::Match(tok, "scanf|vscanf ( %str% ,")) if (Token::Match(tok, "scanf|vscanf ( %str% ,"))
@ -312,40 +313,60 @@ void CheckIO::invalidScanf()
else if (!format) else if (!format)
continue; continue;
else if (std::isdigit(formatstr[i])) { else if (std::isdigit(formatstr[i]) || formatstr[i] == '*') {
format = false; format = false;
} }
else if (std::isalpha(formatstr[i])) { else if (std::isalpha(formatstr[i]) || formatstr[i] == '[') {
if (formatstr[i] != 'c') // #3490 - field width limits are not necessary for %c 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); 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; format = false;
} }
} }
} }
} }
void CheckIO::invalidScanfError(const Token *tok) void CheckIO::invalidScanfError(const Token *tok, bool portability)
{ {
reportError(tok, Severity::warning, if (portability)
"invalidscanf", "scanf without field width limits can crash with huge input data\n" reportError(tok, Severity::portability,
"scanf without field width limits can crash with huge input data. To fix this error " "invalidscanf", "scanf without field width limits can crash with huge input data on some versions of libc.\n"
"message add a field width specifier:\n" "scanf without field width limits can crash with huge input data on libc versions older than 2.13-25. Add a field "
" %s => %20s\n" "width specifier to fix this problem:\n"
" %i => %3i\n" " %i => %3i\n"
"\n" "\n"
"Sample program that can crash:\n" "Sample program that can crash:\n"
"\n" "\n"
"#include <stdio.h>\n" "#include <stdio.h>\n"
"int main()\n" "int main()\n"
"{\n" "{\n"
" int a;\n" " int a;\n"
" scanf(\"%i\", &a);\n" " scanf(\"%i\", &a);\n"
" return 0;\n" " return 0;\n"
"}\n" "}\n"
"\n" "\n"
"To make it crash:\n" "To make it crash:\n"
"perl -e 'print \"5\"x2100000' | ./a.out"); "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 <stdio.h>\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); invalidScanfArgTypeError(tok, tok->str(), numFormat);
if (*i == 's' && variableInfo && isKnownType(variableInfo, varTypeTok) && variableInfo->isArray() && (variableInfo->dimensions().size() == 1)) { if (*i == 's' && variableInfo && isKnownType(variableInfo, varTypeTok) && variableInfo->isArray() && (variableInfo->dimensions().size() == 1)) {
if (width.empty()) if (!width.empty()) {
missingScanfFormatWidthError(tok, tok->str(), numFormat, variableInfo);
else {
int numWidth = std::atoi(width.c_str()); int numWidth = std::atoi(width.c_str());
if (numWidth != (variableInfo->dimension(0) - 1)) if (numWidth != (variableInfo->dimension(0) - 1))
invalidScanfFormatWidthError(tok, tok->str(), numFormat, numWidth, variableInfo); 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"; 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()); 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) void CheckIO::invalidScanfFormatWidthError(const Token* tok, const std::string &functionName, unsigned int numFormat, int width, const Variable *var)
{ {
std::ostringstream errmsg; std::ostringstream errmsg;

6
lib/checkio.h
View File

@ -77,7 +77,7 @@ private:
void readWriteOnlyFileError(const Token *tok); void readWriteOnlyFileError(const Token *tok);
void writeReadOnlyFileError(const Token *tok); void writeReadOnlyFileError(const Token *tok);
void useClosedFileError(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, void wrongPrintfScanfArgumentsError(const Token* tok,
const std::string &function, const std::string &function,
unsigned int numFormat, unsigned int numFormat,
@ -88,7 +88,6 @@ private:
void invalidPrintfArgTypeError_p(const Token* tok, unsigned int numFormat); void invalidPrintfArgTypeError_p(const Token* tok, unsigned int numFormat);
void invalidPrintfArgTypeError_int(const Token* tok, unsigned int numFormat, char c); void invalidPrintfArgTypeError_int(const Token* tok, unsigned int numFormat, char c);
void invalidPrintfArgTypeError_float(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 invalidScanfFormatWidthError(const Token* tok, const std::string &functionName, unsigned int numFormat, int width, const Variable *var);
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const { void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
@ -100,7 +99,7 @@ private:
c.readWriteOnlyFileError(0); c.readWriteOnlyFileError(0);
c.writeReadOnlyFileError(0); c.writeReadOnlyFileError(0);
c.useClosedFileError(0); c.useClosedFileError(0);
c.invalidScanfError(0); c.invalidScanfError(0, false);
c.wrongPrintfScanfArgumentsError(0,"printf",3,2); c.wrongPrintfScanfArgumentsError(0,"printf",3,2);
c.invalidScanfArgTypeError(0, "scanf", 1); c.invalidScanfArgTypeError(0, "scanf", 1);
c.invalidPrintfArgTypeError_s(0, 1); c.invalidPrintfArgTypeError_s(0, 1);
@ -108,7 +107,6 @@ private:
c.invalidPrintfArgTypeError_p(0, 1); c.invalidPrintfArgTypeError_p(0, 1);
c.invalidPrintfArgTypeError_int(0, 1, 'u'); c.invalidPrintfArgTypeError_int(0, 1, 'u');
c.invalidPrintfArgTypeError_float(0, 1, 'f'); c.invalidPrintfArgTypeError_float(0, 1, 'f');
c.missingScanfFormatWidthError(0, "scanf", 1, NULL);
c.invalidScanfFormatWidthError(0, "scanf", 10, 5, NULL); c.invalidScanfFormatWidthError(0, "scanf", 10, 5, NULL);
} }

98
test/testio.cpp
View File

@ -42,19 +42,21 @@ private:
TEST_CASE(testScanf1); // Scanf without field limiters TEST_CASE(testScanf1); // Scanf without field limiters
TEST_CASE(testScanf2); TEST_CASE(testScanf2);
TEST_CASE(testScanf3); TEST_CASE(testScanf3);
TEST_CASE(testScanf4);
TEST_CASE(testScanfArgument); TEST_CASE(testScanfArgument);
TEST_CASE(testPrintfArgument); 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.. // Clear the error buffer..
errout.str(""); errout.str("");
Settings settings; Settings settings;
settings.addEnabled("style"); settings.addEnabled("style");
if (portability)
settings.addEnabled("portability");
settings.inconclusive = inconclusive; settings.inconclusive = inconclusive;
settings.platform(platform);
// Tokenize.. // Tokenize..
Tokenizer tokenizer(&settings, this); Tokenizer tokenizer(&settings, this);
@ -354,70 +356,46 @@ private:
void testScanf1() { void testScanf1() {
check("#include <stdio.h>\n" check("void foo() {\n"
"int main(int argc, char **argv)\n"
"{\n"
" int a, b;\n" " int a, b;\n"
" FILE *file = fopen(\"test\", \"r\");\n" " FILE *file = fopen(\"test\", \"r\");\n"
" b = fscanf(file, \"aa %ds\", &a);\n" " a = fscanf(file, \"aa %s\", bar);\n"
" c = scanf(\"aa %ds\", &a);\n" " b = scanf(\"aa %S\", bar);\n"
" b = fscanf(file, \"aa%%ds\", &a);\n" " b = scanf(\"aa %ls\", bar);\n"
" sscanf(foo, \"%[^~]\", bar);\n"
" scanf(\"%dx%s\", &b, bar);\n"
" fclose(file);\n" " fclose(file);\n"
" return b;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:8]: (warning) fscanf format string has 0 parameters but 1 are given\n" ASSERT_EQUALS("[test.cpp:4]: (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:5]: (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: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() { void testScanf2() {
check("#include <stdio.h>\n" check("void foo() {\n"
"int main(int argc, char **argv)\n" " scanf(\"%5s\", bar);\n" // Width specifier given
"{\n" " scanf(\"%5[^~]\", bar);\n" // Width specifier given
" int a, b;\n" " scanf(\"aa%%s\", bar);\n" // No %s
" FILE *file = fopen(\"test\", \"r\");\n" " scanf(\"aa%d\", &a);\n" // No %s
" b = fscanf(file, \"aa%%%ds\", &a);\n" " scanf(\"aa%ld\", &a);\n" // No %s
" c = scanf(\"aa %%%ds\", &a);\n" " scanf(\"%*[^~]\");\n" // Ignore input
" b = fscanf(file, \"aa%%ds\", &a);\n"
" fclose(file);\n"
" return b;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:8]: (warning) fscanf format string has 0 parameters but 1 are given\n" ASSERT_EQUALS("[test.cpp:4]: (warning) scanf format string has 0 parameters but 1 are given\n", errout.str());
"[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());
} }
void testScanf3() { void testScanf3() {
check("#include <stdio.h>\n" check("void foo() {\n"
"int main(int argc, char **argv)\n" " scanf(\"%d\", &a);\n"
"{\n" " scanf(\"%n\", &a);\n" // No warning on %n, since it doesn't expect user input
" char a[32];\n" " scanf(\"%c\", &c);\n" // No warning on %c; it expects only one character
" int b, c;\n" "}", false, true, Settings::Unspecified);
" FILE *file = fopen(\"test\", \"r\");\n" 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());
" c = fscanf(file, \"%[^ ] %d\n\", a, &b);\n"
" fclose(file);\n"
" return c;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("#include <stdio.h>\n" check("void foo() {\n"
"int main(int argc, char **argv)\n" " scanf(\"%d\", &a);\n"
"{\n" "}", false, true, Settings::Win32A);
" 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"
"}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
@ -431,7 +409,7 @@ private:
" scanf(\"%1u%1u\", &foo, bar());\n" " scanf(\"%1u%1u\", &foo, bar());\n"
" scanf(\"%*1x %1x %29s\", &count, KeyName);\n" // #3373 " scanf(\"%*1x %1x %29s\", &count, KeyName);\n" // #3373
" fscanf(f, \"%7ms\", &ref);\n" // #3461 " fscanf(f, \"%7ms\", &ref);\n" // #3461
" sscanf(ip_port, \"%*[^:]:%d\", &port);\n" // #3468 " sscanf(ip_port, \"%*[^:]:%4d\", &port);\n" // #3468
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
@ -460,14 +438,11 @@ private:
check("void foo() {\n" check("void foo() {\n"
" char input[10];\n" " char input[10];\n"
" char output[5];\n" " char output[5];\n"
" sscanf(input, \"%s\", output);\n"
" sscanf(input, \"%3s\", output);\n" " sscanf(input, \"%3s\", output);\n"
" sscanf(input, \"%4s\", output);\n" " sscanf(input, \"%4s\", output);\n"
" sscanf(input, \"%5s\", output);\n" " sscanf(input, \"%5s\", output);\n"
"}", false); "}", 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" 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());
"[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" check("void foo() {\n"
" char input[10];\n" " char input[10];\n"
@ -477,10 +452,9 @@ private:
" sscanf(input, \"%4s\", output);\n" " sscanf(input, \"%4s\", output);\n"
" sscanf(input, \"%5s\", output);\n" " sscanf(input, \"%5s\", output);\n"
"}", true); "}", 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" ASSERT_EQUALS("[test.cpp:5]: (warning, inconclusive) sscanf width 3 in format string (no. 1) is smaller than 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: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() { void testPrintfArgument() {