diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index a43d5b26ee8..01d8be4c617 100644 --- a/lib/checkbufferoverrun.cpp +++ b/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 ¶meters, 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()); } } } diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 16f0f4c3f72..7cda55bd1fb 100644 --- a/test/testbufferoverrun.cpp +++ b/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"