From d909ac856569e39cb2f53b158c6246ac82d461b7 Mon Sep 17 00:00:00 2001
From: Rikard Falkeborn <rikard.falkeborn@gmail.com>
Date: Sun, 16 Jun 2019 16:02:27 +0200
Subject: [PATCH] Bugfix buffer size for strdup like functions (#1893)

strdup() allocates the string length plus one for a terminating null
character. Add one to compensate for this.

Fixes false positive buffer out of bounds on code like this:

	void f() {
		const char *a = "abcd";
		char * b = strdup(a);
		printf("%c", b[4]); // prints the terminating null character
		free(b);
	}

Also, add a testcase for valueFlowDynamicBufferSize() and add tests for
strdup(), malloc() and calloc().
---
 lib/valueflow.cpp      |  2 +-
 test/testvalueflow.cpp | 46 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index 3be9faa60..32e31dc6c 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -5329,7 +5329,7 @@ static void valueFlowDynamicBufferSize(TokenList *tokenlist, SymbolDatabase *sym
                 if (arg1 && arg1->hasKnownValue()) {
                     const ValueFlow::Value &value = arg1->values().back();
                     if (value.isTokValue() && value.tokvalue->tokType() == Token::eString)
-                        sizeValue = Token::getStrLength(value.tokvalue);
+                        sizeValue = Token::getStrLength(value.tokvalue) + 1; // Add one for the null terminator
                 }
                 break;
             };
diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
index 9f30bc7c1..aae831172 100644
--- a/test/testvalueflow.cpp
+++ b/test/testvalueflow.cpp
@@ -117,6 +117,8 @@ private:
         TEST_CASE(valueFlowTerminatingCond);
 
         TEST_CASE(valueFlowContainerSize);
+
+        TEST_CASE(valueFlowDynamicBufferSize);
     }
 
     static bool isNotTokValue(const ValueFlow::Value &val) {
@@ -223,6 +225,24 @@ private:
         return false;
     }
 
+    bool testValueOfX(const char code[], unsigned int linenr, int value, ValueFlow::Value::ValueType type) {
+        // Tokenize..
+        Tokenizer tokenizer(&settings, this);
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+
+        for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) {
+            if (tok->str() == "x" && tok->linenr() == linenr) {
+                for (const ValueFlow::Value &v : tok->values()) {
+                    if (v.valueType == type && v.intvalue == value)
+                        return true;
+                }
+            }
+        }
+
+        return false;
+    }
+
     bool testValueOfX(const char code[], unsigned int linenr, ValueFlow::Value::MoveKind moveKind) {
         // Tokenize..
         Tokenizer tokenizer(&settings, this);
@@ -3819,6 +3839,32 @@ private:
                "}";
         ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "+"), 8));
     }
+
+    void valueFlowDynamicBufferSize() {
+        const char *code;
+
+        LOAD_LIB_2(settings.library, "std.cfg");
+        LOAD_LIB_2(settings.library, "posix.cfg");
+
+        code = "void* f() {\n"
+               "  void* x = malloc(10);\n"
+               "  return x;\n"
+               "}";
+        ASSERT_EQUALS(true, testValueOfX(code, 3U, 10,  ValueFlow::Value::BUFFER_SIZE));
+
+        code = "void* f() {\n"
+               "  void* x = calloc(4, 5);\n"
+               "  return x;\n"
+               "}";
+        ASSERT_EQUALS(true, testValueOfX(code, 3U, 20,  ValueFlow::Value::BUFFER_SIZE));
+
+        code = "void* f() {\n"
+               "  const char* y = \"abcd\";\n"
+               "  const char* x = strdup(y);\n"
+               "  return x;\n"
+               "}";
+        ASSERT_EQUALS(true, testValueOfX(code, 4U, 5,  ValueFlow::Value::BUFFER_SIZE));
+    }
 };
 
 REGISTER_TEST(TestValueFlow)