Fixed #1409 (False positive: Buffer access out-of-bounds with strncpy and an array in typedef'ed struct)

lib/checkbufferoverrun.cpp

@@ -81,7 +81,7 @@ void CheckBufferOverrun::arrayIndexOutOfBounds(int size, int index)
         reportError(_callStack, severity, "arrayIndexOutOfBounds", "Array index out of bounds");
 }
 
-void CheckBufferOverrun::bufferOverrun(const Token *tok)
+void CheckBufferOverrun::bufferOverrun(const Token *tok, const std::string &varnames)
 {
     Severity::e severity;
     if (_callStack.size() > 0)
@@ -95,7 +95,15 @@ void CheckBufferOverrun::bufferOverrun(const Token *tok)
         severity = Severity::error;
     }
 
-    reportError(tok, severity, "bufferAccessOutOfBounds", "Buffer access out-of-bounds");
+    std::string v = varnames;
+    while (v.find(" ") != std::string::npos)
+        v.erase(v.find(" "), 1);
+
+    std::string errmsg("Buffer access out-of-bounds");
+    if (!v.empty())
+        errmsg += ": " + v;
+
+    reportError(tok, severity, "bufferAccessOutOfBounds",  errmsg);
 }
 
 void CheckBufferOverrun::dangerousStdCin(const Token *tok)
@@ -273,26 +281,22 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
                     const std::string num = tok->strAt(6);
                     if (MathLib::toLongNumber(num) < 0 || MathLib::toLongNumber(num) > total_size)
                     {
-                        bufferOverrun(tok);
+                        bufferOverrun(tok, varnames);
                     }
                 }
             }
         }
-        else if (Token::Match(tok, "memset|memcpy|memmove|memcmp|strncpy|fgets"))
+        else if (Token::Match(tok, ("memset|memcpy|memmove|memcmp|strncpy|fgets ( " + varnames + " , %num% , %num% )").c_str()) ||
+                 Token::Match(tok, ("memcpy|memcmp ( %var% , " + varnames + " , %num% )").c_str()))
         {
-            if (Token::Match(tok->next(), ("( " + varnames + " , %num% , %num% )").c_str()) ||
+            const std::string num  = tok->strAt(varc + 6);
-                Token::Match(tok->next(), ("( %var% , " + varnames + " , %num% )").c_str()))
+            if (MathLib::toLongNumber(num) < 0 || MathLib::toLongNumber(num) > total_size)
             {
-                const std::string num  = tok->strAt(varc + 6);
+                bufferOverrun(tok, varnames);
-                if (MathLib::toLongNumber(num) < 0 || MathLib::toLongNumber(num) > total_size)
-                {
-                    bufferOverrun(tok);
-                }
             }
             continue;
         }
 
-
         // Loop..
         if (Token::simpleMatch(tok, "for ("))
         {
@@ -503,7 +507,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
 
                 if (condition_out_of_bounds && Token::Match(tok2, pattern.str().c_str(), varid))
                 {
-                    bufferOverrun(tok2);
+                    bufferOverrun(tok2, varid > 0 ? "" : varnames.c_str());
                     break;
                 }
 
@@ -556,7 +560,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
             size_t len = Token::getStrLength(tok->tokAt(varc + 4));
             if (len >= static_cast<size_t>(total_size))
             {
-                bufferOverrun(tok);
+                bufferOverrun(tok, varid > 0 ? "" : varnames.c_str());
                 continue;
             }
         }

lib/checkbufferoverrun.h

@@ -83,7 +83,7 @@ private:
 
     void arrayIndexOutOfBounds(const Token *tok, int size, int index);
     void arrayIndexOutOfBounds(int size, int index);
-    void bufferOverrun(const Token *tok);
+    void bufferOverrun(const Token *tok, const std::string &varnames = "");
     void dangerousStdCin(const Token *tok);
     void strncatUsage(const Token *tok);
     void outOfBounds(const Token *tok, const std::string &what);
@@ -93,7 +93,7 @@ private:
     void getErrorMessages()
     {
         arrayIndexOutOfBounds(0, 2, 2);
-        bufferOverrun(0);
+        bufferOverrun(0, std::string("buffer"));
         dangerousStdCin(0);
         strncatUsage(0);
         outOfBounds(0, "index");

lib/checkclass.cpp

@@ -1490,7 +1490,7 @@ void CheckClass::checkConst()
                     continue;
 
                 // don't warn if type is LP..
-                if (tok2->str().compare(0,2,"LP") == 0)
+                if (tok2->str().compare(0, 2, "LP") == 0)
                     continue;
 
                 // member function?

test/testbufferoverrun.cpp

@@ -953,7 +953,7 @@ private:
               "{\n"
               "    strcpy( abc->str, \"abcdef\" );\n"
               "}\n");
-        ASSERT_EQUALS("[test.cpp:8]: (error) Buffer access out-of-bounds\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:8]: (error) Buffer access out-of-bounds: abc.str\n", errout.str());
     }
 
 
@@ -1574,6 +1574,13 @@ private:
               " strncpy(c,\"hello!\",sizeof(c)+1);\n"
               "}\n");
         ASSERT_EQUALS("[test.cpp:4]: (error) Buffer access out-of-bounds\n", errout.str());
+
+        check("struct AB { char a[10]; };\n"
+              "void foo(AB *ab)\n"
+              "{\n"
+              "    strncpy(x, ab->a, 100);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void unknownType()

test/testclass.cpp

@@ -1798,7 +1798,7 @@ private:
                    "};\n");
         ASSERT_EQUALS("", errout.str());
     }
-    
+
     // A function that returns LPVOID can't be const
     void constLPVOID()
     {