sprintf: check for dangerous usage with sprintf|snprintf with

overlapping data

src/checkother.cpp

@@ -296,6 +296,7 @@ void CheckOther::WarningIf()
 
 void CheckOther::InvalidFunctionUsage()
 {
+    // strtol and strtoul..
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
     {
         if ((tok->str() != "strtol") && (tok->str() != "strtoul"))
@@ -330,6 +331,48 @@ void CheckOther::InvalidFunctionUsage()
             }
         }
     }
+    
+    // sprintf|snprintf overlapping data
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+    {
+        // Get variable id of target buffer..
+        unsigned int varid = 0;
+    
+        if ( Token::Match(tok, "sprintf|snprintf ( %var% ,") )
+            varid = tok->tokAt(2)->varId();
+
+        else if ( Token::Match(tok, "sprintf|snprintf ( %var% . %var% ,") )
+            varid = tok->tokAt(4)->varId();
+            
+        if ( varid == 0 )
+            continue;
+
+        // goto ","
+        const Token *tok2 = tok->tokAt(3);
+        while ( tok2 && tok2->str() != "," )
+            tok2 = tok2->next();
+
+        // is any source buffer overlapping the target buffer?
+        unsigned int parlevel = 0;
+        while ( (tok2 = tok2->next()) != NULL )
+        {
+            if ( tok2->str() == "(" )
+                ++parlevel;
+            else if ( tok2->str() == ")" )
+            {
+                --parlevel;
+                if ( parlevel < 0 )
+                    break;
+            }
+            else if ( tok2->varId() == varid )
+            {
+                std::ostringstream ostr;
+                ostr << _tokenizer->fileLine(tok2) << ": Overlapping data buffer " << tok2->str();
+                _errorLogger->reportErr(ostr.str());
+                break;
+            }
+        }
+    }
 }
 //---------------------------------------------------------------------------
 

test/testother.cpp

@@ -35,8 +35,9 @@ private:
     void run()
     {
         TEST_CASE(delete1);
-
         TEST_CASE(delete2);
+        
+        TEST_CASE(sprintf1);    // Dangerous usage of sprintf
     }
 
     void check(const char code[])
@@ -85,6 +86,34 @@ private:
               "}\n");
         ASSERT_EQUALS(std::string("[test.cpp:3]: Redundant condition. It is safe to deallocate a NULL pointer\n"), errout.str());
     }
+    
+    
+    
+    void sprintfUsage(const char code[])
+    {
+        // Tokenize..
+        Tokenizer tokenizer;
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+        tokenizer.setVarId();
+
+        // Clear the error buffer..
+        errout.str("");
+
+        // Check for redundant code..
+        CheckOther checkOther(&tokenizer, this);
+        checkOther.InvalidFunctionUsage();        
+    }
+    
+    void sprintf1()
+    {
+        sprintfUsage( "void foo()\n"
+                      "{\n"
+                      "    char buf[100];\n"
+                      "    sprintf(buf,\"%s\",buf);\n"
+                      "}\n");
+        ASSERT_EQUALS(std::string("[test.cpp:4]: Overlapping data buffer buf\n"), errout.str());
+    }
 };
 
 REGISTER_TEST(TestOther)