diff --git a/cfg/posix.cfg b/cfg/posix.cfg index e0de433e1f1..70ba2582798 100644 --- a/cfg/posix.cfg +++ b/cfg/posix.cfg @@ -2123,11 +2123,13 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s + + diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index bdd18ee21d2..e8de2adf547 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -558,7 +558,7 @@ void CheckBufferOverrunImpl::pointerArithmeticError(const Token *tok, const Toke //--------------------------------------------------------------------------- -ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, const Settings& settings) const +ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, const Settings& settings, bool checkStringLiteral) const { if (!bufTok->valueType()) return ValueFlow::Value(-1); @@ -604,6 +604,14 @@ ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, cons return bufSizeVal; } } + } else if (checkStringLiteral && bufTok->tokType() == Token::Type::eString && bufTok->valueType()) { + ValueFlow::Value strSizeVal; + strSizeVal.valueType = ValueFlow::Value::ValueType::BUFFER_SIZE; + const size_t charSize = bufTok->valueType()->getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointee); + if (charSize > 0) { + strSizeVal.intvalue = Token::getStrArraySize(bufTok) * charSize; + return strSizeVal; + } } } @@ -715,7 +723,8 @@ void CheckBufferOverrunImpl::bufferOverflow() if (argtok->valueType() && argtok->valueType()->pointer == 0) continue; // TODO: strcpy(buf+10, "hello"); - ValueFlow::Value bufferSize = getBufferSize(argtok, mSettings); + const bool strz = mSettings.library.isargstrz(tok, argnr + 1); + ValueFlow::Value bufferSize = getBufferSize(argtok, mSettings, !strz); if (bufferSize.intvalue <= 0) continue; // buffer size == 1 => do not warn for dynamic memory diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index b9b82c897e0..afac19d796c 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -129,7 +129,7 @@ class CPPCHECKLIB CheckBufferOverrunImpl : public CheckImpl void objectIndex(); void objectIndexError(const Token *tok, const ValueFlow::Value *v, bool known); - ValueFlow::Value getBufferSize(const Token *bufTok, const Settings& settings) const; + ValueFlow::Value getBufferSize(const Token *bufTok, const Settings& settings, bool checkStringLiteral = false) const; // CTU static bool isCtuUnsafeBufferUsage(const Settings &settings, const Token *argtok, CTU::FileInfo::Value *offset, int type); diff --git a/test/cfg/posix.c b/test/cfg/posix.c index 82cfd950792..e630dedc975 100644 --- a/test/cfg/posix.c +++ b/test/cfg/posix.c @@ -719,6 +719,13 @@ size_t bufferAccessOutOfBounds_strnlen(const char *s, size_t maxlen) size_t len = strnlen(buf,2); // cppcheck-suppress bufferAccessOutOfBounds len+=strnlen(buf,3); + len+=strnlen("ab",3); + return len; +} + +size_t bufferAccessOutOfBounds_strncasecmp(const char *s) +{ + size_t len = strncasecmp(s, "abc", 5); return len; } @@ -1606,4 +1613,4 @@ void pthread_define_test() (void)rwlock; PTHREAD_SCOPE_PROCESS; PTHREAD_SCOPE_SYSTEM; -} \ No newline at end of file +} diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index f6a2e1a5ec1..e5026a71330 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -229,6 +229,7 @@ class TestBufferOverrun : public TestFixture { TEST_CASE(buffer_overrun_36); TEST_CASE(buffer_overrun_37); TEST_CASE(buffer_overrun_38); + TEST_CASE(buffer_overrun_39); TEST_CASE(buffer_overrun_errorpath); TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch TEST_CASE(buffer_overrun_function_array_argument); @@ -3638,6 +3639,44 @@ class TestBufferOverrun : public TestFixture { ASSERT_EQUALS("", errout_str()); } + void buffer_overrun_39() { // #11765 + check("void f() {\n" + " char buf[10];\n" + " memcpy(buf, \"abc\", 4);\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + + check("void f() {\n" + " char buf[128];\n" + " memcpy(buf, \"abc\", 5);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:17]: (warning) Buffer is accessed out of bounds: \"abc\" [bufferAccessOutOfBounds]\n", + errout_str()); + + check("void f() {\n" + " char buf[128];\n" + " memcpy(buf, L\"abc\", 4 * sizeof(wchar_t));\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + + check("void f() {\n" + " char buf[128];\n" + " memcpy(buf, L\"abc\", 4 * sizeof(wchar_t) + 1);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3:17]: (warning) Buffer is accessed out of bounds: L\"abc\" [bufferAccessOutOfBounds]\n", + errout_str()); + + check("int f(char* s) {\n" + " return memcmp(s, \"\\0\\0\\0\", 3);\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + + check("int f(const char16_t* buf) {\n" + " return memcmp(buf, u\"abc\", 4 * sizeof(char16_t));\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + } + void buffer_overrun_errorpath() { setMultiline(); Settings s = settings0; @@ -4618,9 +4657,8 @@ class TestBufferOverrun : public TestFixture { check("void f() {\n" " mymemset(\"abc\", 0, 20);\n" "}\n", settings); - TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Buffer is accessed out of bounds.\n", - "", - errout_str()); + ASSERT_EQUALS("[test.cpp:2:12]: (warning) Buffer is accessed out of bounds: \"abc\" [bufferAccessOutOfBounds]\n", + errout_str()); check("void f() {\n" " mymemset(temp, \"abc\", 4);\n"