diff --git a/cfg/std.cfg b/cfg/std.cfg index 59eac426a..9577a6fc3 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -5011,6 +5011,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun + @@ -5096,6 +5097,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun + @@ -5211,6 +5213,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun + diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 7afad1de9..d0dbe95da 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -554,25 +554,20 @@ ValueFlow::Value CheckBufferOverrun::getBufferSize(const Token *bufTok) const return *value; } - MathLib::bigint dim = -1; - if (var) { - dim = std::accumulate(var->dimensions().cbegin(), var->dimensions().cend(), 1LL, [](MathLib::bigint i1, const Dimension &dim) { - return i1 * dim.num; - }); - } - else if (bufTok->tokType() == Token::Type::eString) { - dim = Token::getStrLength(bufTok) + 1; - } - else + if (!var) return ValueFlow::Value(-1); + const MathLib::bigint dim = std::accumulate(var->dimensions().cbegin(), var->dimensions().cend(), 1LL, [](MathLib::bigint i1, const Dimension &dim) { + return i1 * dim.num; + }); + ValueFlow::Value v; v.setKnown(); v.valueType = ValueFlow::Value::ValueType::BUFFER_SIZE; - if (var && var->isPointerArray()) + if (var->isPointerArray()) v.intvalue = dim * mSettings->platform.sizeof_pointer; - else if (var && var->isPointer()) + else if (var->isPointer()) return ValueFlow::Value(-1); else { const MathLib::bigint typeSize = bufTok->valueType()->typeSize(mSettings->platform); @@ -651,7 +646,7 @@ void CheckBufferOverrun::bufferOverflow() argtok = argtok->astOperand2() ? argtok->astOperand2() : argtok->astOperand1(); while (Token::Match(argtok, ".|::")) argtok = argtok->astOperand2(); - if (!argtok || (!argtok->variable() && argtok->tokType() != Token::Type::eString)) + if (!argtok || !argtok->variable()) continue; if (argtok->valueType() && argtok->valueType()->pointer == 0) continue; diff --git a/test/cfg/std.c b/test/cfg/std.c index 5c35cba12..1cc16fefc 100644 --- a/test/cfg/std.c +++ b/test/cfg/std.c @@ -219,12 +219,14 @@ void bufferAccessOutOfBounds(void) strncpy_s(a,5,"abcd",5); // string will be truncated, error is returned, but no buffer overflow strncpy_s(a,5,"abcde",6); + // TODO cppcheck-suppress bufferAccessOutOfBounds strncpy_s(a,5,"a",6); strncpy_s(a,5,"abcdefgh",4); // valid call strncat_s(a,5,"1",2); // cppcheck-suppress bufferAccessOutOfBounds strncat_s(a,10,"1",2); + // TODO cppcheck-suppress bufferAccessOutOfBounds strncat_s(a,5,"1",5); fread(a,1,5,stdin); // cppcheck-suppress bufferAccessOutOfBounds @@ -516,6 +518,7 @@ void nullpointer(int value) wcstok(NULL,L"xyz",&pWcsUninit); strxfrm(0,"foo",0); + // TODO: error message (#6306 and http://trac.cppcheck.net/changeset/d11eb4931aea51cf2cb74faccdcd2a3289b818d6/) strxfrm(0,"foo",42); wcsxfrm(0,L"foo",0); // TODO: error message when arg1==NULL and arg3!=0 #6306: https://trac.cppcheck.net/ticket/6306#comment:2 diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 0eb59bf5a..c12bb4467 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -238,7 +238,6 @@ private: TEST_CASE(buffer_overrun_34); //#11035 TEST_CASE(buffer_overrun_35); //#2304 TEST_CASE(buffer_overrun_36); - TEST_CASE(buffer_overrun_37); TEST_CASE(buffer_overrun_errorpath); TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch TEST_CASE(buffer_overrun_function_array_argument); @@ -3325,6 +3324,23 @@ private: " (void)strxfrm(dest,src,3);\n" // << "}"); ASSERT_EQUALS("[test.cpp:6]: (error) Buffer is accessed out of bounds: dest\n", errout.str()); + // source size is too small + check("void f(void) {\n" + " const char src[2] = \"ab\";\n" + " char dest[3] = \"abc\";\n" + " (void)strxfrm(dest,src,1);\n" + " (void)strxfrm(dest,src,2);\n" + " (void)strxfrm(dest,src,3);\n" // << + "}"); + ASSERT_EQUALS("[test.cpp:6]: (error) Buffer is accessed out of bounds: src\n", errout.str()); + // source size is too small + check("void f(void) {\n" + " const char src[1] = \"a\";\n" + " char dest[3] = \"abc\";\n" + " (void)strxfrm(dest,src,1);\n" + " (void)strxfrm(dest,src,2);\n" // << + "}"); + ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: src\n", errout.str()); } void buffer_overrun_33() { // #2019 @@ -3387,32 +3403,6 @@ private: ASSERT_EQUALS("", errout.str()); } - void buffer_overrun_37() { // #11765 - check("void f() {\n" - " char buf[128];\n" - " memcpy(buf, \"Error\", 6);\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " char buf[128];\n" - " memcpy(buf, \"Error\", 16);\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: \"Error\"\n", errout.str()); - - check("void f() {\n" - " char buf[128];\n" - " memcpy(buf, L\"Error\", 10);\n" // at least 12 bytes on any platform - "}\n"); - ASSERT_EQUALS("", errout.str()); - - check("void f() {\n" - " char buf[128];\n" - " memcpy(buf, L\"Error\", 26);\n" // at most 24 bytes - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: L\"Error\"\n", errout.str()); - } - void buffer_overrun_errorpath() { setMultiline(); const Settings settingsOld = settings0; @@ -4275,7 +4265,9 @@ private: check("void f() {\n" " mymemset(\"abc\", 0, 20);\n" "}", settings); - ASSERT_EQUALS("[test.cpp:2]: (error) Buffer is accessed out of bounds: \"abc\"\n", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Buffer is accessed out of bounds.\n", + "", + errout.str()); check("void f() {\n" " mymemset(temp, \"abc\", 4);\n"