Fix #11765 FN: minsize not checked for string literal, buffer access out of bounds not found

Author: web-flow

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

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);

test/cfg/posix.c

@@ -719,6 +719,7 @@ 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;
 }
 
@@ -1606,4 +1607,4 @@ void pthread_define_test()
     (void)rwlock;
     PTHREAD_SCOPE_PROCESS;
     PTHREAD_SCOPE_SYSTEM;
-}
+}

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"