Fixed #984 (improve output if strncpy is followed by strncat)

lib/checkother.cpp

@@ -1352,7 +1352,7 @@ class CheckUninitVar : public ExecutionPath
 public:
     // Startup constructor
     CheckUninitVar(Check *c)
-            : ExecutionPath(c, 0), pointer(false), array(false), alloc(false)
+            : ExecutionPath(c, 0), pointer(false), array(false), alloc(false), strncpy_(false)
     {
     }
 
@@ -1367,7 +1367,7 @@ private:
 
     // internal constructor for creating extra checks
     CheckUninitVar(Check *c, unsigned int v, const std::string &name, bool p, bool a)
-            : ExecutionPath(c, v), varname(name), pointer(p), array(a), alloc(false)
+            : ExecutionPath(c, v), varname(name), pointer(p), array(a), alloc(false), strncpy_(false)
     {
     }
 
@@ -1375,6 +1375,7 @@ private:
     const bool pointer;
     const bool array;
     bool  alloc;
+    bool  strncpy_;
 
     // p = malloc ..
     static void alloc_pointer(std::list<ExecutionPath *> &checks, unsigned int varid)
@@ -1488,6 +1489,26 @@ private:
     }
 
 
+    /** Initialize an array with strncpy.. */
+    static void init_strncpy(std::list<ExecutionPath *> &checks, const Token *tok)
+    {
+        const unsigned int varid(tok->varId());
+        if (!varid)
+            return;
+
+        std::list<ExecutionPath *>::const_iterator it;
+        for (it = checks.begin(); it != checks.end(); ++it)
+        {
+            CheckUninitVar *c = dynamic_cast<CheckUninitVar *>(*it);
+            if (c && c->varId == varid)
+            {
+                c->strncpy_ = true;
+            }
+        }
+    }
+
+
+
     /**
      * use - called from the use* functions below.
      * @param foundError this is set to true if an error is found
@@ -1526,7 +1547,9 @@ private:
                 CheckOther *checkOther = dynamic_cast<CheckOther *>(c->owner);
                 if (checkOther)
                 {
-                    if (c->pointer && c->alloc)
+                    if (c->strncpy_)
+                        checkOther->uninitstringError(tok, c->varname);
+                    else if (c->pointer && c->alloc)
                         checkOther->uninitdataError(tok, c->varname);
                     else
                         checkOther->uninitvarError(tok, c->varname);
@@ -1794,8 +1817,11 @@ private:
             }
 
             // strncpy doesn't 0-terminate first parameter
-            if (Token::Match(&tok, "strncpy ("))
+            if (Token::Match(&tok, "strncpy ( %var% ,"))
+            {
+                init_strncpy(checks, tok.tokAt(2));
                 return tok.next()->link();
+            }
 
             if (Token::Match(&tok, "asm ( )"))
             {
@@ -2178,6 +2204,11 @@ void CheckOther::nullPointerError(const Token *tok, const std::string &varname,
     reportError(tok, Severity::error, "nullPointer", "Possible null pointer dereference: " + varname + " - otherwise it is redundant to check if " + varname + " is null at line " + MathLib::toString<long>(line));
 }
 
+void CheckOther::uninitstringError(const Token *tok, const std::string &varname)
+{
+    reportError(tok, Severity::error, "uninitstring", "Dangerous usage of '" + varname + "' (strncpy doesn't always 0-terminate it)");
+}
+
 void CheckOther::uninitdataError(const Token *tok, const std::string &varname)
 {
     reportError(tok, Severity::error, "uninitdata", "Data is allocated but not initialized: " + varname);

lib/checkother.h

@@ -155,6 +155,7 @@ public:
     void nullPointerError(const Token *tok);  // variable name unknown / doesn't exist
     void nullPointerError(const Token *tok, const std::string &varname);
     void nullPointerError(const Token *tok, const std::string &varname, const int line);
+    void uninitstringError(const Token *tok, const std::string &varname);
     void uninitdataError(const Token *tok, const std::string &varname);
     void uninitvarError(const Token *tok, const std::string &varname);
     void zerodivError(const Token *tok);
@@ -166,6 +167,7 @@ public:
         sprintfOverlappingDataError(0, "varname");
         udivError(0);
         nullPointerError(0, "pointer");
+        uninitstringError(0, "varname");
         uninitdataError(0, "varname");
         uninitvarError(0, "varname");
         zerodivError(0);

test/testother.cpp

@@ -77,6 +77,7 @@ private:
         TEST_CASE(uninitvar_enum);      // enum variables
         TEST_CASE(uninitvar_if);        // handling if/while/switch
         TEST_CASE(uninitvar_references); // references
+        TEST_CASE(uninitvar_strncpy);   // strncpy doesn't always 0-terminate
         TEST_CASE(uninitvar_func);      // analyse functions
 
         TEST_CASE(oldStylePointerCast);
@@ -1540,14 +1541,6 @@ private:
                        "    strchr(s, ' ');\n"
                        "};\n");
         ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: s\n", errout.str());
-
-        checkUninitVar("void f()\n"
-                       "{\n"
-                       "    char s[20];\n"
-                       "    strncpy(s, \"abcde\", 2);\n"
-                       "    strcat(s, \"abc\");\n"
-                       "};\n");
-        ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: s\n", errout.str());
     }
 
     // alloc..
@@ -1698,6 +1691,18 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    // strncpy doesn't always 0-terminate..
+    void uninitvar_strncpy()
+    {
+        checkUninitVar("void f()\n"
+                       "{\n"
+                       "    char a[100];\n"
+                       "    strncpy(a, s, 20);\n"
+                       "    strncat(a, s, 20);\n"
+                       "}\n");
+        ASSERT_EQUALS("[test.cpp:5]: (error) Dangerous usage of 'a' (strncpy doesn't always 0-terminate it)\n", errout.str());
+    }
+
 
     std::string analyseFunctions(const char code[])
     {