From 9a84c5845a665f7cc94fae3955e6a81f338fb49d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 2 Dec 2011 17:09:32 +0100 Subject: [PATCH] Fixed #3373 (False posititive: incorrect %* handling in sscanf) --- lib/checknullpointer.cpp | 15 ++++++++++---- lib/checkother.cpp | 17 ++++++++++++---- test/testnullpointer.cpp | 5 +++++ test/testother.cpp | 42 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 8 deletions(-) diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 24bfaccb9..1d5ccbcfc 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -173,13 +173,22 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::listnextArgument(); + if (*i == '*') { + if (scan) + _continue = true; + else + argListTok = argListTok->nextArgument(); + } ++i; if (!argListTok || i == formatString.end()) return; } + if (_continue) + continue; if ((*i == 'n' || *i == 's' || scan) && (!scan || value == 0)) { if ((value == 0 && argListTok->str() == "0") || (Token::Match(argListTok, "%var%") && argListTok->varId() > 0)) { @@ -191,8 +200,6 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::listnextArgument(); // Find next argument if (!argListTok) break; - - percent = false; } } } diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 707a0c562..a49e60c42 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1285,6 +1285,7 @@ void CheckOther::checkWrongPrintfScanfArguments() } // Count format string parameters.. + bool scan = Token::Match(tok, "sscanf|fscanf|scanf"); unsigned int numFormat = 0; bool percent = false; for (std::string::iterator i = formatString.begin(); i != formatString.end(); ++i) { @@ -1302,25 +1303,33 @@ void CheckOther::checkWrongPrintfScanfArguments() if (i == formatString.end()) break; } else if (percent) { + percent = false; + + bool _continue = false; while (i != formatString.end() && !std::isalpha(*i)) { - if (*i == '*') - numFormat++; + if (*i == '*') { + if (scan) + _continue = true; + else + numFormat++; + } ++i; } if (i == formatString.end()) break; + if (_continue) + continue; if (*i != 'm') // %m is a non-standard extension that requires no parameter numFormat++; - percent = false; + // TODO: Perform type checks } } // Count printf/scanf parameters.. unsigned int numFunction = 0; while (argListTok) { - // TODO: Perform type checks numFunction++; argListTok = argListTok->nextArgument(); // Find next argument } diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index c174f8f2a..7ca5fbd55 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -1587,6 +1587,11 @@ private: " sscanf(dummy, \"%d%d\", foo(iVal), iVal);\n" "}"); ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: iVal\n", errout.str()); + + check("void f(char* dummy) {\n" + " sscanf(dummy, \"%*d%u\", 0);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (error) Null pointer dereference\n", errout.str()); } void nullpointer_in_return() { diff --git a/test/testother.cpp b/test/testother.cpp index 963f10aa7..431131308 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -89,6 +89,7 @@ private: TEST_CASE(testScanf2); TEST_CASE(testScanf3); + TEST_CASE(testScanfArgument); TEST_CASE(testPrintfArgument); TEST_CASE(trac1132); @@ -1957,6 +1958,47 @@ private: ASSERT_EQUALS("[test.cpp:7]: (warning) fscanf format string has 0 parameters but 1 are given\n", errout.str()); } + void testScanfArgument() { + check("void foo() {\n" + " scanf(\"%1d\", &foo);\n" + " sscanf(bar, \"%1d\", &foo);\n" + " scanf(\"%1u%1u\", &foo, bar());\n" + " scanf(\"%*1x %1x %29s\", &count, KeyName);\n" // #3373 + "}\n", + "test.cpp", + true + ); + ASSERT_EQUALS("", errout.str()); + + check("void foo() {\n" + " scanf(\"\", &foo);\n" + " scanf(\"%1d\", &foo, &bar);\n" + " fscanf(bar, \"%1d\", &foo, &bar);\n" + " scanf(\"%*1x %1x %29s\", &count, KeyName, foo);\n" + "}\n", + "test.cpp", + true + ); + ASSERT_EQUALS("[test.cpp:2]: (warning) scanf format string has 0 parameters but 1 are given\n" + "[test.cpp:3]: (warning) scanf format string has 1 parameters but 2 are given\n" + "[test.cpp:4]: (warning) fscanf format string has 1 parameters but 2 are given\n" + "[test.cpp:5]: (warning) scanf format string has 2 parameters but 3 are given\n", errout.str()); + + check("void foo() {\n" + " scanf(\"%1d\");\n" + " scanf(\"%1u%1u\", bar());\n" + " sscanf(bar, \"%1d%1d\", &foo);\n" + " scanf(\"%*1x %1x %29s\", &count);\n" + "}\n", + "test.cpp", + true + ); + ASSERT_EQUALS("[test.cpp:2]: (error) scanf format string has 1 parameters but only 0 are given\n" + "[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()); + } + void testPrintfArgument() { check("void foo() {\n" " printf(\"%u\");\n"