Merge branch 'master' into aleksi/master

src/checkbufferoverrun.cpp

@@ -348,20 +348,12 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con
         // Writing data into array..
         if (Token::Match(tok, ("strcpy|strcat ( " + varnames + " , %str% )").c_str()))
         {
-            int len = 0;
-            const char *str = tok->strAt(varc + 4);
-            while (*str)
-            {
-                if (*str == '\\')
-                    ++str;
-                ++str;
-                ++len;
-            }
-            if (len > 2 && len >= (int)size + 2)
+            size_t len = Token::getStrLength(tok->tokAt(varc + 4));
+            if (len >= static_cast<size_t>(size))
             {
                 bufferOverrun(tok);
+                continue;
             }
-            continue;
         }
 
 
@@ -382,6 +374,23 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con
                 strncatUsage(tok->tokAt(9));
         }
 
+        // Detect few strcat() calls
+        if (varid > 0 && Token::Match(tok, "strcat ( %varid% , %str% ) ;", varid))
+        {
+            size_t charactersAppend = 0;
+            const Token *tok2 = tok;
+
+            while (tok2 && Token::Match(tok2, "strcat ( %varid% , %str% ) ;", varid))
+            {
+                charactersAppend += Token::getStrLength(tok2->tokAt(4));
+                if (charactersAppend >= static_cast<size_t>(size))
+                {
+                    bufferOverrun(tok2);
+                    break;
+                }
+                tok2 = tok2->tokAt(7);
+            }
+        }
 
         // sprintf..
         if (varid > 0 && Token::Match(tok, "sprintf ( %varid% , %str% [,)]", varid))
@@ -420,15 +429,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con
                 {
                     if (tok2->str()[0] == '\"')
                     {
-                        len -= 2;
-                        const char *str = tok2->str().c_str();
-                        while (*str)
-                        {
-                            if (*str == '\\')
-                                ++str;
-                            ++str;
-                            ++len;
-                        }
+                        len += Token::getStrLength(tok2);
                     }
                 }
                 if (len >= (int)size)

src/token.cpp

@@ -491,6 +491,32 @@ bool Token::Match(const Token *tok, const char pattern[], unsigned int varid)
     return true;
 }
 
+size_t Token::getStrLength(const Token *tok)
+{
+    assert(tok != NULL);
+
+    size_t len = 0;
+    const char *str = tok->strAt(0);
+
+    assert(str[0] == '"');
+    assert(str[strlen(str)-1] == '"');
+
+    while (*str)
+    {
+        if (*str == '\\')
+            ++str;
+        ++str;
+        ++len;
+    }
+
+    assert(len >= 2);
+
+    // don't count quotes
+    len -= 2;
+
+    return len;
+}
+
 bool Token::isStandardType() const
 {
     bool ret = false;

src/token.h

@@ -117,6 +117,15 @@ public:
      */
     static bool Match(const Token *tok, const char pattern[], unsigned int varid = 0);
 
+    /**
+     * Return length of C-string.
+     *
+     * Should be called for %str% tokens only.
+     *
+     * @param tok token with C-string
+     **/
+    static size_t getStrLength(const Token *tok);
+
     bool isName() const
     {
         return _isName;

test/testbufferoverrun.cpp

@@ -93,6 +93,7 @@ private:
         TEST_CASE(buffer_overrun_3);
         TEST_CASE(buffer_overrun_4);
         TEST_CASE(buffer_overrun_5);
+        TEST_CASE(buffer_overrun_6);
 
         TEST_CASE(sprintf1);
         TEST_CASE(sprintf2);
@@ -558,6 +559,16 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void buffer_overrun_6()
+    {
+        check("void f()\n"
+              "{\n"
+              "   char n[5];\n"
+              "   strcat(n, \"abc\");\n"
+              "   strcat(n, \"def\");\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:5]: (possible error) Buffer overrun\n", errout.str());
+    }
 
     void sprintf1()
     {

test/testtoken.cpp

@@ -34,6 +34,7 @@ private:
     {
         TEST_CASE(nextprevious);
         TEST_CASE(multiCompare);
+        TEST_CASE(getStrLength);
     }
 
     void nextprevious()
@@ -78,6 +79,23 @@ private:
         ASSERT_EQUALS(static_cast<unsigned int>(-1), static_cast<unsigned int>(Token::multiCompare("abc|def", "abcd")));
         ASSERT_EQUALS(static_cast<unsigned int>(-1), static_cast<unsigned int>(Token::multiCompare("abc|def", "default")));
     }
+
+    void getStrLength()
+    {
+        Token *tok = new Token();
+
+        tok->str("\"\"");
+        ASSERT_EQUALS(0, Token::getStrLength(tok));
+
+        tok->str("\"test\"");
+        ASSERT_EQUALS(4, Token::getStrLength(tok));
+
+        tok->str("\"test \\\\test\"");
+        ASSERT_EQUALS(10, Token::getStrLength(tok));
+
+        Tokenizer::deleteTokens(tok);
+    }
+
 };
 
 REGISTER_TEST(TestTOKEN)