From 8c1efe9bb697baaa8d21cb63017f20574902860e Mon Sep 17 00:00:00 2001
From: Robert Reif <reif@earthlink.net>
Date: Sun, 21 Aug 2011 15:18:41 -0400
Subject: [PATCH] improve message for #3035 (false negative: strcpy(dst, src)
 where src is bigger than dst)

---
 lib/checkbufferoverrun.cpp | 15 ++++++++++-----
 lib/checkbufferoverrun.h   |  4 ++--
 test/testbufferoverrun.cpp | 30 ++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
index c0d9d0f48..5ff7b8f68 100644
--- a/lib/checkbufferoverrun.cpp
+++ b/lib/checkbufferoverrun.cpp
@@ -113,11 +113,16 @@ void CheckBufferOverrun::bufferOverrun(const Token *tok, const std::string &varn
     reportError(tok, Severity::error, "bufferAccessOutOfBounds",  errmsg);
 }
 
-void CheckBufferOverrun::possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst)
+void CheckBufferOverrun::possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst, bool cat)
 {
-    reportError(tok, Severity::warning, "possibleBufferAccessOutOfBounds",
-                "Possible buffer overflow if strlen(" + src + ") is larger than sizeof(" + dst + ")-strlen(" + dst +").\n"
-                "The source buffer is larger than the destination buffer so there is the potential for overflowing the destination buffer.");
+    if (cat)
+        reportError(tok, Severity::warning, "possibleBufferAccessOutOfBounds",
+                    "Possible buffer overflow if strlen(" + src + ") is larger than sizeof(" + dst + ")-strlen(" + dst +").\n"
+                    "The source buffer is larger than the destination buffer so there is the potential for overflowing the destination buffer.");
+    else
+        reportError(tok, Severity::warning, "possibleBufferAccessOutOfBounds",
+                    "Possible buffer overflow if strlen(" + src + ") is larger than or equal to sizeof(" + dst + ").\n"
+                    "The source buffer is larger than the destination buffer so there is the potential for overflowing the destination buffer.");
 }
 
 void CheckBufferOverrun::strncatUsage(const Token *tok)
@@ -987,7 +992,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
                 if (total_size > 0 && len > (unsigned int)total_size)
                 {
                     if (_settings->inconclusive)
-                        possibleBufferOverrunError(tok, tok->strAt(4), tok->strAt(2));
+                        possibleBufferOverrunError(tok, tok->strAt(4), tok->strAt(2), tok->str() == "strcat");
                     continue;
                 }
             }
diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h
index 2cb32c95b..81f8c66d2 100644
--- a/lib/checkbufferoverrun.h
+++ b/lib/checkbufferoverrun.h
@@ -221,7 +221,7 @@ public:
     void cmdLineArgsError(const Token *tok);
     void pointerOutOfBounds(const Token *tok, const std::string &object);	// UB when result of calculation is out of bounds
     void arrayIndexThenCheckError(const Token *tok, const std::string &indexName);
-    void possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst);
+    void possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst, bool cat);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings)
     {
@@ -236,7 +236,7 @@ public:
         c.cmdLineArgsError(0);
         c.pointerOutOfBounds(0, "array");
         c.arrayIndexThenCheckError(0, "index");
-        c.possibleBufferOverrunError(0, "source", "destination");
+        c.possibleBufferOverrunError(0, "source", "destination", false);
     }
 
     std::string myName() const
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index d892a4a24..16a178a32 100644
--- a/test/testbufferoverrun.cpp
+++ b/test/testbufferoverrun.cpp
@@ -2047,6 +2047,36 @@ private:
               "    strcat(data, src);\n"
               "}");
         ASSERT_EQUALS("", errout.str());
+
+        check("void foo() {\n"
+              "    char * data = (char *)alloca(50);\n"
+              "    char src[100];\n"
+              "    memset(src, 'C', 100-1);\n"
+              "    src[100-1] = '\\0';\n"
+              "    strcpy(data, src);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:6]: (warning) Possible buffer overflow if strlen(src) is larger than or equal to sizeof(data).\n", errout.str());
+
+        check("void foo() {\n"
+              "    char * data = (char *)alloca(100);\n"
+              "    char src[100];\n"
+              "    memset(src, 'C', 100-1);\n"
+              "    src[100-1] = '\\0';\n"
+              "    strcpy(data, src);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void foo(char src[100]) {\n"
+              "    char * data = (char *)alloca(50);\n"
+              "    strcpy(data, src);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Possible buffer overflow if strlen(src) is larger than or equal to sizeof(data).\n", errout.str());
+
+        check("void foo(char src[100]) {\n"
+              "    char * data = (char *)alloca(100);\n"
+              "    strcpy(data, src);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void pointer_out_of_bounds_1()