From 14528bcf2575bb273fca2ae055d6f303698b7a23 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Wed, 20 Mar 2019 06:46:55 +0100
Subject: [PATCH] Library: allowed values for the buffer-size attribute:
 malloc/calloc/strdup

---
 cfg/cppcheck-cfg.rng |  8 +++++---
 cfg/gnu.cfg          |  4 ++--
 cfg/posix.cfg        |  2 +-
 cfg/std.cfg          |  6 +++---
 lib/library.cpp      | 12 +++++++++---
 lib/library.h        |  3 ++-
 lib/valueflow.cpp    | 33 +++++++++++++++++++++++++--------
 7 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/cfg/cppcheck-cfg.rng b/cfg/cppcheck-cfg.rng
index b5e8b5196..b87c07152 100644
--- a/cfg/cppcheck-cfg.rng
+++ b/cfg/cppcheck-cfg.rng
@@ -474,9 +474,11 @@
   </define>
 
   <define name="DATA-BUFFER-SIZE">
-    <data type="string">
-      <param name="pattern">arg-value:[1-3]</param>
-    </data>
+    <choice>
+      <value>malloc</value>
+      <value>calloc</value>
+      <value>strdup</value>
+    </choice>
   </define>
 
   <define name="CONTAINER-ACTION">
diff --git a/cfg/gnu.cfg b/cfg/gnu.cfg
index 77e3f8331..6af4d572b 100644
--- a/cfg/gnu.cfg
+++ b/cfg/gnu.cfg
@@ -9,7 +9,7 @@
     <dealloc>free</dealloc>
   </memory>
   <memory>
-    <alloc init="false" buffer-size="arg-value:1">xmalloc</alloc>
+    <alloc init="false" buffer-size="malloc">xmalloc</alloc>
     <alloc init="true">xcalloc</alloc>
     <dealloc>free</dealloc>
   </memory>
@@ -19,7 +19,7 @@
     <dealloc>free</dealloc>
   </memory>
   <memory>
-    <alloc init="false" buffer-size="arg-value:1">pvalloc</alloc>
+    <alloc init="false" buffer-size="malloc">pvalloc</alloc>
     <dealloc>free</dealloc>
   </memory>
   <!-- https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html -->
diff --git a/cfg/posix.cfg b/cfg/posix.cfg
index 2a1f5afe2..6a15ab04d 100644
--- a/cfg/posix.cfg
+++ b/cfg/posix.cfg
@@ -4364,7 +4364,7 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
     <dealloc>free</dealloc>
   </memory>
   <memory>
-    <alloc init="true">strdup</alloc>
+    <alloc init="true" buffer-size="strdup">strdup</alloc>
     <alloc init="true">strndup</alloc>
     <alloc init="true">wcsdup</alloc>
     <dealloc>free</dealloc>
diff --git a/cfg/std.cfg b/cfg/std.cfg
index f57921694..ad7c559d4 100644
--- a/cfg/std.cfg
+++ b/cfg/std.cfg
@@ -7320,10 +7320,10 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
     <arg nr="2"/>
   </function>
   <memory>
-    <alloc init="false" buffer-size="arg-value:1">malloc</alloc>
-    <alloc init="true">calloc</alloc>
+    <alloc init="false" buffer-size="malloc">malloc</alloc>
+    <alloc init="true" buffer-size="calloc">calloc</alloc>
     <alloc init="false">aligned_alloc</alloc>
-    <alloc init="false">valloc</alloc>
+    <alloc init="false" buffer-size="malloc">valloc</alloc>
     <dealloc>free</dealloc>
   </memory>
   <resource>
diff --git a/lib/library.cpp b/lib/library.cpp
index 37691d7c3..181d00294 100644
--- a/lib/library.cpp
+++ b/lib/library.cpp
@@ -202,10 +202,16 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc)
                         temp.arg = -1;
 
                     const char *bufferSize = memorynode->Attribute("buffer-size");
-                    if (bufferSize && std::strncmp(bufferSize, "arg-value:", 10) == 0)
-                        temp.bufferSizeArgValue = bufferSize[10] - '0';
+                    if (!bufferSize)
+                        temp.bufferSize = AllocFunc::BufferSize::none;
+                    else if (std::strcmp(bufferSize, "malloc") == 0)
+                        temp.bufferSize = AllocFunc::BufferSize::malloc;
+                    else if (std::strcmp(bufferSize, "calloc") == 0)
+                        temp.bufferSize = AllocFunc::BufferSize::calloc;
+                    else if (std::strcmp(bufferSize, "strdup") == 0)
+                        temp.bufferSize = AllocFunc::BufferSize::strdup;
                     else
-                        temp.bufferSizeArgValue = -1;
+                        return Error(BAD_ATTRIBUTE_VALUE, bufferSize);
 
                     mAlloc[memorynode->GetText()] = temp;
                 } else if (memorynodename == "dealloc") {
diff --git a/lib/library.h b/lib/library.h
index 9198072b8..204bebb7e 100644
--- a/lib/library.h
+++ b/lib/library.h
@@ -73,7 +73,8 @@ public:
     struct AllocFunc {
         int groupId;
         int arg;
-        int bufferSizeArgValue;
+        enum class BufferSize {none,malloc,calloc,strdup};
+        BufferSize bufferSize;
     };
 
     /** get allocation info for function */
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index e845e497f..6d35dce33 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -5125,19 +5125,36 @@ static void valueFlowDynamicBufferSize(TokenList *tokenlist, SymbolDatabase *sym
                 continue;
 
             const Library::AllocFunc *allocFunc = settings->library.alloc(rhs->previous());
-            if (!allocFunc || allocFunc->bufferSizeArgValue < 1)
+            if (!allocFunc || allocFunc->bufferSize == Library::AllocFunc::BufferSize::none)
                 continue;
 
             const std::vector<const Token *> args = getArguments(rhs->previous());
-            if (args.size() < allocFunc->bufferSizeArgValue)
+
+            MathLib::bigint sizeValue = -1;
+            switch (allocFunc->bufferSize) {
+            case Library::AllocFunc::BufferSize::none:
+                break;
+            case Library::AllocFunc::BufferSize::malloc:
+                if (args.size() == 1 && args[0]->hasKnownIntValue())
+                    sizeValue = args[0]->getKnownIntValue();
+                break;
+            case Library::AllocFunc::BufferSize::calloc:
+                if (args.size() == 2 && args[0]->hasKnownIntValue() && args[1]->hasKnownIntValue())
+                    sizeValue = args[0]->getKnownIntValue() * args[1]->getKnownIntValue();
+                break;
+            case Library::AllocFunc::BufferSize::strdup:
+                if (args.size() == 1 && args[0]->hasKnownValue()) {
+                    const ValueFlow::Value &value = args[0]->values().back();
+                    if (value.isTokValue() && value.tokvalue->tokType() == Token::eString)
+                        sizeValue = Token::getStrLength(value.tokvalue);
+                }
+                break;
+            };
+            if (sizeValue < 0)
                 continue;
 
-            const Token *sizeArg = args[allocFunc->bufferSizeArgValue - 1];
-            if (!sizeArg || !sizeArg->hasKnownIntValue())
-                continue;
-
-            ValueFlow::Value value(sizeArg->getKnownIntValue());
-            value.errorPath.emplace_back(tok->tokAt(2), "Assign " + tok->strAt(1) + ", buffer with size " + MathLib::toString(value.intvalue));
+            ValueFlow::Value value(sizeValue);
+            value.errorPath.emplace_back(tok->tokAt(2), "Assign " + tok->strAt(1) + ", buffer with size " + MathLib::toString(sizeValue));
             value.valueType = ValueFlow::Value::ValueType::BUFFER_SIZE;
             value.setKnown();
             const std::list<ValueFlow::Value> values{value};