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

cfg/gtk.cfg

@@ -5386,11 +5386,13 @@
     <arg nr="1" direction="in">
       <not-null/>
       <not-uninit/>
+      <strz/>
       <minsize type="argvalue" arg="3"/>
     </arg>
     <arg nr="2" direction="in">
       <not-null/>
       <not-uninit/>
+      <strz/>
       <minsize type="argvalue" arg="3"/>
     </arg>
     <arg nr="3" direction="in">

cfg/posix.cfg

@@ -2123,11 +2123,13 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
     <arg nr="1" direction="in">
       <not-null/>
       <not-uninit/>
+      <strz/>
       <minsize type="argvalue" arg="3"/>
     </arg>
     <arg nr="2" direction="in">
       <not-null/>
       <not-uninit/>
+      <strz/>
       <minsize type="argvalue" arg="3"/>
     </arg>
     <arg nr="3" direction="in">
@@ -2146,11 +2148,13 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
     <arg nr="1" direction="in">
       <not-null/>
       <not-uninit/>
+      <strz/>
       <minsize type="argvalue" arg="3"/>
     </arg>
     <arg nr="2" direction="in">
       <not-null/>
       <not-uninit/>
+      <strz/>
       <minsize type="argvalue" arg="3"/>
     </arg>
     <arg nr="3" direction="in">
@@ -5606,6 +5610,7 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
     <arg nr="1" direction="in">
       <not-null/>
       <not-uninit/>
+      <strz/>
       <minsize type="argvalue" arg="2"/>
     </arg>
     <arg nr="2" direction="in">
@@ -5782,6 +5787,7 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
     <arg nr="2" direction="in">
       <not-null/>
       <not-uninit/>
+      <strz/>
       <minsize type="argvalue" arg="3"/>
     </arg>
     <arg nr="3" direction="in">

cfg/windows.cfg

@@ -4293,11 +4293,13 @@ HFONT CreateFont(
     <arg nr="1" direction="in">
       <not-null/>
       <not-uninit/>
+      <strz/>
       <minsize type="argvalue" arg="3"/>
     </arg>
     <arg nr="2" direction="in">
       <not-null/>
       <not-uninit/>
+      <strz/>
       <minsize type="argvalue" arg="3"/>
     </arg>
     <arg nr="3" direction="in">
@@ -4317,11 +4319,13 @@ HFONT CreateFont(
     <arg nr="1" direction="in">
       <not-null/>
       <not-uninit/>
+      <strz/>
       <minsize type="argvalue" arg="3"/>
     </arg>
     <arg nr="2" direction="in">
       <not-null/>
       <not-uninit/>
+      <strz/>
       <minsize type="argvalue" arg="3"/>
     </arg>
     <arg nr="3" direction="in">

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,15 @@ ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, cons
                     return bufSizeVal;
                 }
             }
+        } else if (checkStringLiteral && bufTok->tokType() == Token::Type::eString && bufTok->valueType()) {
+            ValueFlow::Value strSizeVal;
+            strSizeVal.setKnown();
+            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) - index) * charSize;
+                return strSizeVal;
+            }
         }
     }
 
@@ -715,7 +724,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);

lib/checkfunctions.cpp

@@ -102,6 +102,18 @@ void CheckFunctionsImpl::functionCalledError(const Token* tok, Severity severity
     reportError(tok, severity, prefix + "Called", msg, CWE477, Certainty::normal);
 }
 
+static bool hasArgMinReadSize(const Token *ftok, int argnr, const Library &library)
+{
+    if (library.getArgDirection(ftok, argnr) == Library::ArgumentChecks::Direction::DIR_IN) {
+        if (const std::vector<Library::ArgumentChecks::MinSize> *minsizes = library.argminsizes(ftok, argnr)) {
+            return std::any_of(minsizes->begin(), minsizes->end(), [](const Library::ArgumentChecks::MinSize &ms) {
+                return ms.type == Library::ArgumentChecks::MinSize::Type::ARGVALUE;
+            });
+        }
+    }
+    return false;
+}
+
 //---------------------------------------------------------------------------
 // Check <valid>, <strz> and <not-bool>
 //---------------------------------------------------------------------------
@@ -137,7 +149,7 @@ void CheckFunctionsImpl::invalidFunctionUsage()
                         invalidFunctionArgError(argtok, functionToken->str(), argnr, nullptr, mSettings.library.validarg(functionToken, argnr));
                 }
                 // check <strz>
-                if (mSettings.library.isargstrz(functionToken, argnr)) {
+                if (mSettings.library.isargstrz(functionToken, argnr) && !hasArgMinReadSize(functionToken, argnr, mSettings.library)) {
                     if (Token::Match(argtok, "& %var% !![") && argtok->next() && argtok->next()->valueType()) {
                         const ValueType * valueType = argtok->next()->valueType();
                         const Variable * variable = argtok->next()->variable();

test/cfg/posix.c

@@ -194,6 +194,12 @@ void invalidFunctionArgStr_mbsnrtowcs(void)
     if (mbsnrtowcs (wenough, &cp, 1, 10, &s) != 1 || wcscmp (wenough, L"AEF") != 0) {}
 }
 
+int invalidFunctionArgStr_strncasecmp(const char* s)
+{
+    const char buf[3] = {'a', 'b', 'c'};
+    return strncasecmp(buf, s, 3);
+}
+
 struct tm * ignoredReturnValue_localtime(const time_t *tp)
 {
     // cppcheck-suppress [ignoredReturnValue,localtimeCalled]
@@ -719,6 +725,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",5);
+    return len;
+}
+
+size_t bufferAccessOutOfBounds_strncasecmp(const char *s)
+{
+    size_t len = strncasecmp(s, "abc", 5);
     return len;
 }
 
@@ -1606,4 +1619,4 @@ void pthread_define_test()
     (void)rwlock;
     PTHREAD_SCOPE_PROCESS;
     PTHREAD_SCOPE_SYSTEM;
-}
+}

test/cfg/std.c

@@ -78,6 +78,9 @@ void invalidFunctionArgStr_strncat(void)
     // No warning is expected for
     strncat (str1, &src, 1);
     puts (str1);
+    char d[] = {'a','b','c'};
+    // cppcheck-suppress invalidFunctionArgStr
+    strncat(d, "def", 1);
 }
 
 char * invalidFunctionArgStr_strpbrk( const char *p )
@@ -4000,11 +4003,10 @@ void bufferAccessOutOfBounds_strxfrm(void)
 {
     const char src[3] = "abc";
     char dest[1] = "a";
-    // cppcheck-suppress invalidFunctionArgStr
     (void)strxfrm(dest,src,1);
-    // cppcheck-suppress [bufferAccessOutOfBounds,invalidFunctionArgStr]
+    // cppcheck-suppress [bufferAccessOutOfBounds]
     (void)strxfrm(dest,src,2);
-    // cppcheck-suppress [bufferAccessOutOfBounds,invalidFunctionArgStr]
+    // cppcheck-suppress [bufferAccessOutOfBounds]
     (void)strxfrm(dest,src,3);
 }
 
@@ -4913,6 +4915,10 @@ void invalidFunctionArgString(char c)
     char d='d';
     /* cppcheck-suppress invalidFunctionArgStr */
     (void)strcat(str,&d);
+
+    char a[] = { 'a', 'b', 'c' };
+    /* cppcheck-suppress [bufferAccessOutOfBounds,invalidFunctionArgStr] */
+    (void)strcat(a, "def");
 }
 
 void ignoredReturnValue_abs(int i)

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,50 @@ 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]: (error) 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]: (error) 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());
+
+        check("int f(const char* buf) {\n"
+              "    return memcmp(buf, &\"abc\"[2], 4 * sizeof(char));\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2:24]: (error) Buffer is accessed out of bounds: &\"abc\"[2] [bufferAccessOutOfBounds]\n",
+                      errout_str());
+    }
+
     void buffer_overrun_errorpath() {
         setMultiline();
         Settings s = settings0;
@@ -4618,9 +4663,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]: (error) Buffer is accessed out of bounds: \"abc\" [bufferAccessOutOfBounds]\n",
+                      errout_str());
 
         check("void f() {\n"
               "  mymemset(temp, \"abc\", 4);\n"