Fix #7570: Support address-of operator on variables in getBufferSize()

lib/checkbufferoverrun.cpp

@@ -75,6 +75,14 @@ static const ValueFlow::Value *getBufferSizeValue(const Token *tok)
     return it == tokenValues.cend() ? nullptr : &*it;
 }
 
+static const Token* getRealBufferTok(const Token* tok) {
+    if (!tok->isUnaryOp("&"))
+        return tok;
+
+    const Token* op = tok->astOperand1();
+    return (op->valueType() && op->valueType()->pointer) ? op : tok;
+}
+
 static int getMinFormatStringOutputLength(const std::vector<const Token*> &parameters, nonneg int formatStringArgNr)
 {
     if (formatStringArgNr <= 0 || formatStringArgNr > parameters.size())
@@ -553,6 +561,8 @@ ValueFlow::Value CheckBufferOverrun::getBufferSize(const Token *bufTok) const
 {
     if (!bufTok->valueType())
         return ValueFlow::Value(-1);
+    if (bufTok->isUnaryOp("&"))
+        bufTok = bufTok->astOperand1();
     const Variable *var = bufTok->variable();
 
     if (!var || var->dimensions().empty()) {
@@ -653,7 +663,7 @@ void CheckBufferOverrun::bufferOverflow()
                     argtok = argtok->astOperand2() ? argtok->astOperand2() : argtok->astOperand1();
                 while (Token::Match(argtok, ".|::"))
                     argtok = argtok->astOperand2();
-                if (!argtok || !argtok->variable())
+                if (!argtok)
                     continue;
                 if (argtok->valueType() && argtok->valueType()->pointer == 0)
                     continue;
@@ -688,7 +698,7 @@ void CheckBufferOverrun::bufferOverflow()
 
 void CheckBufferOverrun::bufferOverflowError(const Token *tok, const ValueFlow::Value *value, Certainty certainty)
 {
-    reportError(getErrorPath(tok, value, "Buffer overrun"), Severity::error, "bufferAccessOutOfBounds", "Buffer is accessed out of bounds: " + (tok ? tok->expressionString() : "buf"), CWE_BUFFER_OVERRUN, certainty);
+    reportError(getErrorPath(tok, value, "Buffer overrun"), Severity::error, "bufferAccessOutOfBounds", "Buffer is accessed out of bounds: " + (tok ? getRealBufferTok(tok)->expressionString() : "buf"), CWE_BUFFER_OVERRUN, certainty);
 }
 
 //---------------------------------------------------------------------------
@@ -798,7 +808,7 @@ void CheckBufferOverrun::stringNotZeroTerminated()
             if (isZeroTerminated)
                 continue;
             // TODO: Locate unsafe string usage..
-            terminateStrncpyError(tok, args[0]->expressionString());
+            terminateStrncpyError(tok, getRealBufferTok(args[0])->expressionString());
         }
     }
 }

test/testbufferoverrun.cpp

@@ -231,6 +231,7 @@ class TestBufferOverrun : public TestFixture {
         TEST_CASE(buffer_overrun_function_array_argument);
         TEST_CASE(possible_buffer_overrun_1); // #3035
         TEST_CASE(buffer_overrun_readSizeFromCfg);
+        TEST_CASE(buffer_overrun_handle_addr_of_var); // ticket #7570 - correctly handle &var
 
         TEST_CASE(valueflow_string); // using ValueFlow string values in checking
 
@@ -270,6 +271,7 @@ class TestBufferOverrun : public TestFixture {
         TEST_CASE(terminateStrncpy3);
         TEST_CASE(terminateStrncpy4);
         TEST_CASE(terminateStrncpy5); // #9944
+        TEST_CASE(terminateStrncpy_handle_addr_of_var); // #7570
         TEST_CASE(recursive_long_time);
 
         TEST_CASE(crash1);  // Ticket #1587 - crash
@@ -3723,6 +3725,38 @@ class TestBufferOverrun : public TestFixture {
         ASSERT_EQUALS("[test.cpp:3:16]: (error) Buffer is accessed out of bounds: ms.str [bufferAccessOutOfBounds]\n", errout_str());
     }
 
+    void buffer_overrun_handle_addr_of_var() { // #7570
+        check("void f() {\n"
+              "  int i;\n"
+              "  memset(i, 0, 1000);\n"
+              "}");
+        ASSERT_EQUALS("", errout_str());
+
+        check("void f() {\n"
+              "  int i;\n"
+              "  memset(&i, 0, 1000);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3:10]: (error) Buffer is accessed out of bounds: &i [bufferAccessOutOfBounds]\n", errout_str());
+
+        check("void f() {\n"
+              "  int i[2];\n"
+              "  memset(&i, 0, 1000);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3:10]: (error) Buffer is accessed out of bounds: i [bufferAccessOutOfBounds]\n", errout_str());
+
+        check("void f() {\n"
+              "  int i;\n"
+              "  memset(&i, 0, sizeof(i));\n"
+              "}");
+        ASSERT_EQUALS("", errout_str());
+
+        check("void f() {\n"
+              "  int i[10];\n"
+              "  memset(&i[1], 0, 1000);\n"
+              "}");
+        TODO_ASSERT_EQUALS("[test.cpp:3:10]: (error) Buffer is accessed out of bounds: &i[1] [bufferAccessOutOfBounds]\n", "", errout_str());
+    }
+
     void valueflow_string() { // using ValueFlow string values in checking
         check("char f() {\n"
               "  const char *x = s;\n"
@@ -4320,7 +4354,7 @@ class TestBufferOverrun : public TestFixture {
               "  char c;\n"
               "  mymemset(&c, 0, 4);\n"
               "}", dinit(CheckOptions, $.s = &settings));
-        TODO_ASSERT_EQUALS("[test.cpp:3:14]: (error) Buffer is accessed out of bounds: c [bufferAccessOutOfBounds]\n", "", errout_str());
+        ASSERT_EQUALS("[test.cpp:3:12]: (error) Buffer is accessed out of bounds: &c [bufferAccessOutOfBounds]\n", errout_str());
 
         // ticket #2121 - buffer access out of bounds when using uint32_t
         check("void f(void) {\n"
@@ -4684,6 +4718,20 @@ class TestBufferOverrun : public TestFixture {
     }
     // extracttests.enable
 
+    void terminateStrncpy_handle_addr_of_var() { // #7570
+        check("void foo() {\n"
+              "  char c[6];\n"
+              "  strncpy(&c, \"hello!\", 6);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3:3]: (warning, inconclusive) The buffer 'c' may not be null-terminated after the call to strncpy(). [terminateStrncpy]\n", errout_str());
+
+        check("void foo() {\n"
+              "  char c[6];\n"
+              "  strncpy(&c, \"hello\\0\", 6);\n"
+              "}");
+        ASSERT_EQUALS("", errout_str());
+    }
+
     void recursive_long_time() {
         // Just test that recursive check doesn't take long time
         check("char *f2 ( char *b )\n"