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"