From f9f6927e63834cafca625d832297a500b4d0a79f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <danielm77@spray.se>
Date: Wed, 21 Apr 2010 20:02:58 +0200
Subject: [PATCH] CheckBufferOverrun: Don't give false positives when reading
 from array with strncpy/strncat

---
 lib/checkbufferoverrun.cpp | 39 +++++++++++++++++++++-----------------
 lib/checkbufferoverrun.h   |  9 +++++++--
 test/testbufferoverrun.cpp |  4 ++--
 3 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
index b2e642bc2..90f33853e 100644
--- a/lib/checkbufferoverrun.cpp
+++ b/lib/checkbufferoverrun.cpp
@@ -461,24 +461,28 @@ void CheckBufferOverrun::parse_for_body(const Token *tok2, const ArrayInfo &arra
 
 
 
-
-void CheckBufferOverrun::checkFunctionCall(const Token *tok, const ArrayInfo &arrayInfo)
+void CheckBufferOverrun::checkFunctionCall(const Token &tok, unsigned int par, const ArrayInfo &arrayInfo)
 {
     std::map<std::string, unsigned int> total_size;
     total_size["fgets"] = 2;	// The second argument for fgets can't exceed the total size of the array
     total_size["memcmp"] = 3;
     total_size["memcpy"] = 3;
     total_size["memmove"] = 3;
-    total_size["memset"] = 3;
-    total_size["strncat"] = 3;
-    total_size["strncmp"] = 3;
-    total_size["strncpy"] = 3;
 
-    std::map<std::string, unsigned int>::const_iterator it = total_size.find(tok->str());
+    if (par == 1)
+    {
+        // reading from array
+        // if it is zero terminated properly there won't be buffer overruns
+        total_size["strncat"] = 3;
+        total_size["strncpy"] = 3;
+        total_size["memset"] = 3;
+    }
+
+    std::map<std::string, unsigned int>::const_iterator it = total_size.find(tok.str());
     if (it != total_size.end())
     {
         unsigned int arg = it->second;
-        for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next())
+        for (const Token *tok2 = tok.tokAt(2); tok2; tok2 = tok2->next())
         {
             if (tok2->str() == "(")
             {
@@ -500,7 +504,7 @@ void CheckBufferOverrun::checkFunctionCall(const Token *tok, const ArrayInfo &ar
                             elements *= arrayInfo.num[i];
                         if (sz < 0 || sz > int(elements * arrayInfo.element_size))
                         {
-                            bufferOverrun(tok, arrayInfo.varname);
+                            bufferOverrun(&tok, arrayInfo.varname);
                         }
                     }
                     break;
@@ -597,12 +601,13 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
 
 
         // memset, memcmp, memcpy, strncpy, fgets..
-        if (varid == 0 &&
-            (Token::Match(tok, ("%var% ( " + varnames + " ,").c_str()) ||
-             Token::Match(tok, ("%var% ( %var% , " + varnames + " ,").c_str())))
+        if (varid == 0)
         {
             ArrayInfo arrayInfo(0, varnames, 1, total_size);
-            checkFunctionCall(tok, arrayInfo);
+            if (Token::Match(tok, ("%var% ( " + varnames + " ,").c_str()))
+                checkFunctionCall(*tok, 1, arrayInfo);
+            if (Token::Match(tok, ("%var% ( %var% , " + varnames + " ,").c_str()))
+                checkFunctionCall(*tok, 2, arrayInfo);
         }
 
         // Loop..
@@ -910,10 +915,10 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
 
 
         // Check function call..
-        if (Token::Match(tok, "%var% ( %varid% ,", arrayInfo.varid) ||
-            Token::Match(tok, "%var% ( %var% , %varid% ,", arrayInfo.varid))
-            checkFunctionCall(tok, arrayInfo);
-
+        if (Token::Match(tok, "%var% ( %varid% ,", arrayInfo.varid))
+            checkFunctionCall(*tok, 1, arrayInfo);
+        if (Token::Match(tok, "%var% ( %var% , %varid% ,", arrayInfo.varid))
+            checkFunctionCall(*tok, 2, arrayInfo);
 
 
         if (Token::Match(tok, "memset|memcpy|memmove|memcmp|strncpy|fgets ( %varid% , %any% , %any% )", arrayInfo.varid) ||
diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h
index 8f6cc204b..4e2201317 100644
--- a/lib/checkbufferoverrun.h
+++ b/lib/checkbufferoverrun.h
@@ -158,8 +158,13 @@ public:
     /** Helper function used when parsing for-loops */
     void parse_for_body(const Token *tok2, const ArrayInfo &arrayInfo, const std::string &strindex, bool condition_out_of_bounds, unsigned int counter_varid, const std::string &min_counter_value, const std::string &max_counter_value);
 
-    /** Helper function for checkScope - check a function call */
-    void checkFunctionCall(const Token *tok2, const ArrayInfo &arrayInfo);
+    /**
+     * Helper function for checkScope - check a function call
+     * \param tok token for the function name
+     * \param par on what parameter is the array used
+     * \param arrayInfo the array information
+     */
+    void checkFunctionCall(const Token &tok, const unsigned int par, const ArrayInfo &arrayInfo);
 
     /** callstack - used during intra-function checking */
     std::list<const Token *> _callStack;
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index a8dbb0ee4..a7305d494 100644
--- a/test/testbufferoverrun.cpp
+++ b/test/testbufferoverrun.cpp
@@ -1845,7 +1845,7 @@ private:
               "    strcpy(a, \"hello\");\n"
               "    strncpy(c, a, sizeof(c));\n"
               "}\n");
-        ASSERT_EQUALS("[test.cpp:6]: (error) Buffer access out-of-bounds: a\n", errout.str());
+        ASSERT_EQUALS("", errout.str());
 
         check("void f()\n"
               "{\n"
@@ -1884,7 +1884,7 @@ private:
               "{\n"
               "    strncpy(x, ab->a, 100);\n"
               "}\n");
-        ASSERT_EQUALS("[test.cpp:4]: (error) Buffer access out-of-bounds: ab.a\n", errout.str());
+        ASSERT_EQUALS("", errout.str());
     }
 
     void unknownType()