Fixed #3180 (New check: Detect nullpointers given to printf via variable argument list)

lib/checknullpointer.cpp

@@ -22,6 +22,7 @@
 #include "executionpath.h"
 #include "mathlib.h"
 #include "symboldatabase.h"
+#include <cctype>
 //---------------------------------------------------------------------------
 
 // Register this check class (by creating a static instance of it)
@@ -78,6 +79,9 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
         functionNames1.insert("fgetpos");
         functionNames1.insert("fsetpos");
         functionNames1.insert("rewind");
+        functionNames1.insert("scanf");
+        functionNames1.insert("fscanf");
+        functionNames1.insert("sscanf");
     }
 
     // standard functions that dereference second parameter..
@@ -95,6 +99,10 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
         functionNames2.insert("strcpy");
         functionNames2.insert("strncpy");
         functionNames2.insert("strstr");
+        functionNames2.insert("sprintf");
+        functionNames2.insert("fprintf");
+        functionNames2.insert("fscanf");
+        functionNames2.insert("sscanf");
     }
 
     // 1st parameter..
@@ -103,9 +111,9 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
     {
         if (functionNames1.find(tok.str()) != functionNames1.end())
             var.push_back(tok.tokAt(2));
-        else if (value == 0 && Token::Match(&tok, "memchr|memcmp|memcpy|memmove|memset|strcpy|printf|sprintf"))
+        else if (value == 0 && Token::Match(&tok, "memcpy|memmove|memset|strcpy|printf|sprintf|vsprintf|vprintf|fprintf|vfprintf"))
             var.push_back(tok.tokAt(2));
-        else if (value == 0 && Token::simpleMatch(&tok, "snprintf") && tok.strAt(4) != "0")
+        else if (value == 0 && Token::Match(&tok, "snprintf|vsnprintf|fnprintf|vfnprintf") && tok.strAt(4) != "0")
             var.push_back(tok.tokAt(2));
         else if (value != 0 && Token::simpleMatch(&tok, "fflush"))
             var.push_back(tok.tokAt(2));
@@ -119,13 +127,57 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::list<const Token
             var.push_back(tok.tokAt(4));
     }
 
-    // TODO: Handle sprintf/printf better.
+    if (Token::Match(&tok, "printf|sprintf|snprintf|fprintf|fnprintf|scanf|sscanf|fscanf"))
-    if (Token::Match(&tok, "printf ( %str% , %var% ,|)") && tok.tokAt(4)->varId() > 0)
     {
-        const std::string &formatstr(tok.tokAt(2)->str());
+        const Token* argListTok = 0; // Points to first va_list argument
-        const std::string::size_type pos = formatstr.find("%");
+        std::string formatString;
-        if (pos != std::string::npos && formatstr.compare(pos,2,"%s") == 0)
+        bool scan = Token::Match(&tok, "scanf|sscanf|fscanf");
-            var.push_back(tok.tokAt(4));
+        if (Token::Match(&tok, "printf|scanf ( %str% , %any%"))
+        {
+            formatString = tok.strAt(2);
+            argListTok = tok.tokAt(4);
+        }
+        else if (Token::Match(&tok, "sprintf|fprintf|sscanf|fscanf ( %var% , %str% , %any%"))
+        {
+            formatString = tok.strAt(4);
+            argListTok = tok.tokAt(6);
+        }
+        else if (Token::Match(&tok, "snprintf|fnprintf ( %var% , %any% , %str% , %any%"))
+        {
+            formatString = tok.strAt(6);
+            argListTok = tok.tokAt(8);
+        }
+        if (argListTok)
+        {
+            bool percent = false;
+            for (std::string::iterator i = formatString.begin(); i != formatString.end(); ++i)
+            {
+                if (*i == '%')
+                {
+                    percent = !percent;
+                }
+                else if (percent && std::isalpha(*i))
+                {
+                    if (*i == 'n' || *i == 's' || scan)
+                    {
+                        if ((value == 0 && argListTok->str() == "0") || (Token::Match(argListTok, "%var%") && argListTok->varId() > 0))
+                        {
+                            var.push_back(argListTok);
+                        }
+                    }
+
+                    for (; argListTok; argListTok = argListTok->next()) // Find next argument
+                    {
+                        if (argListTok->str() == ",")
+                        {
+                            argListTok = argListTok->next();
+                            break;
+                        }
+                    }
+                    percent = false;
+                }
+            }
+        }
     }
 }
 

test/testnullpointer.cpp

@@ -47,11 +47,13 @@ private:
         TEST_CASE(nullpointer9);
         TEST_CASE(nullpointer10);
         TEST_CASE(nullpointer11); // ticket #2812
-        TEST_CASE(pointerCheckAndDeRef);	// check if pointer is null and then dereference it
+        TEST_CASE(pointerCheckAndDeRef);     // check if pointer is null and then dereference it
-        TEST_CASE(nullConstantDereference);		// Dereference NULL constant
+        TEST_CASE(nullConstantDereference);  // Dereference NULL constant
-        TEST_CASE(gcc_statement_expression);    // Don't crash
+        TEST_CASE(gcc_statement_expression); // Don't crash
         TEST_CASE(snprintf_with_zero_size);
         TEST_CASE(snprintf_with_non_zero_size);
+        TEST_CASE(printf_with_invalid_va_argument);
+        TEST_CASE(scanf_with_invalid_va_argument);
     }
 
     void check(const char code[], bool inconclusive = false)
@@ -1356,6 +1358,78 @@ private:
               "}");
         ASSERT_EQUALS("[test.cpp:2]: (error) Null pointer dereference\n", errout.str());
     }
+
+    void printf_with_invalid_va_argument()
+    {
+        check("void f() {\n"
+              "    printf(\"%s\", 0);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (error) Null pointer dereference\n", errout.str());
+
+        check("void f(char* s) {\n"
+              "    printf(\"%s\", s);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    char* s = 0;\n"
+              "    printf(\"%s\", s);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: s\n", errout.str());
+
+
+        check("void f() {\n"
+              "    printf(\"%u%s\", 0, 0);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (error) Null pointer dereference\n", errout.str());
+
+        check("void f(char* s) {\n"
+              "    printf(\"%u%s\", 0, s);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void f() {\n"
+              "    char* s = 0;\n"
+              "    printf(\"%u%s\", 123, s);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: s\n", errout.str());
+
+
+        check("void f() {\n"
+              "    printf(\"%%%s%%\", 0);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (error) Null pointer dereference\n", errout.str());
+
+        check("void f(char* s) {\n"
+              "    printf(\"text: %s, %s\", s, 0);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (error) Null pointer dereference\n", errout.str());
+
+
+        check("void f() {\n"
+              "    char* s = \"blabla\";\n"
+              "    printf(\"%s\", s);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void scanf_with_invalid_va_argument()
+    {
+        check("void f(char* s) {\n"
+              "    sscanf(s, \"%s\", 0);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (error) Null pointer dereference\n", errout.str());
+
+        check("void f() {\n"
+              "    scanf(\"%d\", 0);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (error) Null pointer dereference\n", errout.str());
+
+        check("void f(char* s) {\n"
+              "    printf(\"%s\", s);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
 };
 
 REGISTER_TEST(TestNullPointer)