From e004731f1ca0b36d2b2ff926083c8654ea0a960f Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Tue, 5 Jan 2021 09:56:38 -0600
Subject: [PATCH] Fix issue 8650: ValueFlow: Track if pointer is created by '&'
 operator (#3011)

---
 lib/checkbufferoverrun.cpp | 37 ++++++++++++++++++++++---------------
 lib/valueflow.cpp          | 24 ++++++++++++++----------
 lib/valueflow.h            |  2 ++
 test/testbufferoverrun.cpp | 12 ++++++++++++
 4 files changed, 50 insertions(+), 25 deletions(-)

diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
index 2c216540f..54598ed1d 100644
--- a/lib/checkbufferoverrun.cpp
+++ b/lib/checkbufferoverrun.cpp
@@ -889,21 +889,28 @@ void CheckBufferOverrun::objectIndex()
             if (idx->hasKnownIntValue() && idx->getKnownIntValue() == 0)
                 continue;
 
-            ValueFlow::Value v = getLifetimeObjValue(obj);
-            if (!v.isLocalLifetimeValue())
-                continue;
-            if (v.lifetimeKind != ValueFlow::Value::LifetimeKind::Address)
-                continue;
-            const Variable *var = v.tokvalue->variable();
-            if (var->isReference())
-                continue;
-            if (var->isRValueReference())
-                continue;
-            if (var->isArray())
-                continue;
-            if (var->isPointer())
-                continue;
-            objectIndexError(tok, &v, idx->hasKnownIntValue());
+            std::vector<ValueFlow::Value> values = getLifetimeObjValues(obj, false, true);
+            for(const ValueFlow::Value& v:values) {
+                if (v.lifetimeKind != ValueFlow::Value::LifetimeKind::Address)
+                    continue;
+                const Variable *var = v.tokvalue->variable();
+                if (var->isReference())
+                    continue;
+                if (var->isRValueReference())
+                    continue;
+                if (var->isArray())
+                    continue;
+                if (var->isPointer()) {
+                    if (!var->valueType())
+                        continue;
+                    if (!obj->valueType())
+                        continue;
+                    if (var->valueType()->pointer > obj->valueType()->pointer)
+                        continue;
+                }
+                objectIndexError(tok, &v, idx->hasKnownIntValue());
+                break;
+            }
         }
     }
 }
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index 049ed2104..02f88b4a4 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -98,6 +98,7 @@
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
+#include <cstdint>
 #include <cstring>
 #include <functional>
 #include <iterator>
@@ -2703,11 +2704,11 @@ std::string lifetimeMessage(const Token *tok, const ValueFlow::Value *val, Error
     return msg;
 }
 
-ValueFlow::Value getLifetimeObjValue(const Token *tok, bool inconclusive)
+std::vector<ValueFlow::Value> getLifetimeObjValues(const Token *tok, bool inconclusive, bool subfunction)
 {
-    ValueFlow::Value result;
+    std::vector<ValueFlow::Value> result;
     auto pred = [&](const ValueFlow::Value &v) {
-        if (!v.isLocalLifetimeValue())
+        if (!v.isLocalLifetimeValue() && !(subfunction && v.isSubFunctionLifetimeValue()))
             return false;
         if (!inconclusive && v.isInconclusive())
             return false;
@@ -2715,16 +2716,19 @@ ValueFlow::Value getLifetimeObjValue(const Token *tok, bool inconclusive)
             return false;
         return true;
     };
-    auto it = std::find_if(tok->values().begin(), tok->values().end(), pred);
-    if (it == tok->values().end())
-        return result;
-    result = *it;
-    // There should only be one lifetime
-    if (std::find_if(std::next(it), tok->values().end(), pred) != tok->values().end())
-        return ValueFlow::Value{};
+    std::copy_if(tok->values().begin(), tok->values().end(), std::back_inserter(result), pred);
     return result;
 }
 
+ValueFlow::Value getLifetimeObjValue(const Token *tok, bool inconclusive)
+{
+    std::vector<ValueFlow::Value> values = getLifetimeObjValues(tok, inconclusive, false);
+    // There should only be one lifetime
+    if (values.size() != 1)
+        return ValueFlow::Value{};
+    return values.front();
+}
+
 std::vector<LifetimeToken> getLifetimeTokens(const Token* tok, bool escape, ValueFlow::Value::ErrorPath errorPath, int depth)
 {
     if (!tok)
diff --git a/lib/valueflow.h b/lib/valueflow.h
index d3eb2634f..c1e060d18 100644
--- a/lib/valueflow.h
+++ b/lib/valueflow.h
@@ -401,4 +401,6 @@ std::string lifetimeMessage(const Token *tok, const ValueFlow::Value *val, Value
 
 CPPCHECKLIB ValueFlow::Value getLifetimeObjValue(const Token *tok, bool inconclusive = false);
 
+CPPCHECKLIB std::vector<ValueFlow::Value> getLifetimeObjValues(const Token *tok, bool inconclusive = false, bool subfunction = false);
+
 #endif // valueflowH
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index f443ef9d7..c76330e4a 100644
--- a/test/testbufferoverrun.cpp
+++ b/test/testbufferoverrun.cpp
@@ -4484,6 +4484,18 @@ private:
               "  return m[1][1];\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        check("void print(char** test);\n"
+              "int main(){\n"
+              "    char* test = \"abcdef\";\n"
+              "    print(&test);\n"
+              "    return 0;\n"
+              "}\n"
+              "void print(char** test){\n"
+              "    for(int i=0;i<strlen(*test);i++)\n"
+              "        printf(\"%c\",*test[i]);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:4] -> [test.cpp:9]: (warning) The address of local variable 'test' might be accessed at non-zero index.\n", errout.str());
     }
 };