Fixed #5257 (Check memcpy size for string literals)

lib/checkbufferoverrun.cpp

@@ -264,36 +264,33 @@ static bool bailoutIfSwitch(const Token *tok, const unsigned int varid)
 }
 //---------------------------------------------------------------------------
 
-void CheckBufferOverrun::checkFunctionParameter(const Token &ftok, unsigned int par, const ArrayInfo &arrayInfo, const std::list<const Token *>& callstack)
+static bool checkMinSizes(const std::list<Library::ArgumentChecks::MinSize> &minsizes, const Token * const ftok, const std::size_t arraySize, const Token **charSizeToken)
 {
-    const std::list<Library::ArgumentChecks::MinSize> * const minsizes = _settings->library.argminsizes(ftok.str(),par);
+    if (charSizeToken)
+        *charSizeToken = nullptr;
 
-    if (minsizes && !minsizes->empty() && (!(Token::simpleMatch(ftok.previous(), ".") || Token::Match(ftok.tokAt(-2), "!!std ::")))) {
-        if (arrayInfo.element_size() == 0)
-            return;
-
-        MathLib::bigint arraySize = arrayInfo.element_size();
-        for (unsigned int i = 0; i < arrayInfo.num().size(); ++i)
-            arraySize *= arrayInfo.num(i);
+    if (minsizes.empty())
+        return false;
 
+    // All conditions must be true
     bool error = true;
-        for (std::list<Library::ArgumentChecks::MinSize>::const_iterator minsize = minsizes->begin(); minsize != minsizes->end(); ++minsize) {
+    for (std::list<Library::ArgumentChecks::MinSize>::const_iterator minsize = minsizes.begin(); minsize != minsizes.end(); ++minsize) {
         if (!error)
-                break;
+            return false;
         error = false;
-            const Token *argtok = ftok.tokAt(2);
+        const Token *argtok = ftok->tokAt(2);
         for (int argnum = 1; argtok && argnum < minsize->arg; argnum++)
             argtok = argtok->nextArgument();
         if (!argtok)
-                break;
+            return false;
         switch (minsize->type) {
         case Library::ArgumentChecks::MinSize::ARGVALUE:
             if (Token::Match(argtok, "%num% ,|)")) {
                 const MathLib::bigint sz = MathLib::toLongNumber(argtok->str());
                 if (sz > arraySize)
                     error = true;
-                } else if (argtok->type() == Token::eChar && Token::Match(argtok->next(), ",|)"))
-                    sizeArgumentAsCharError(argtok);
+            } else if (argtok->type() == Token::eChar && Token::Match(argtok->next(), ",|)") && charSizeToken)
+                *charSizeToken = argtok; //sizeArgumentAsCharError(argtok);
             break;
         case Library::ArgumentChecks::MinSize::MUL:
             // TODO: handle arbitrary arg2
@@ -312,11 +309,29 @@ void CheckBufferOverrun::checkFunctionParameter(const Token &ftok, unsigned int
                 error = true;
             break;
         case Library::ArgumentChecks::MinSize::NONE:
-                break;
+            return false;
         };
     }
-        if (error)
+    return error;
+}
+
+void CheckBufferOverrun::checkFunctionParameter(const Token &ftok, unsigned int par, const ArrayInfo &arrayInfo, const std::list<const Token *>& callstack)
+{
+    const std::list<Library::ArgumentChecks::MinSize> * const minsizes = _settings->library.argminsizes(ftok.str(),par);
+
+    if (minsizes && (!(Token::simpleMatch(ftok.previous(), ".") || Token::Match(ftok.tokAt(-2), "!!std ::")))) {
+        if (arrayInfo.element_size() == 0)
+            return;
+
+        MathLib::bigint arraySize = arrayInfo.element_size();
+        for (unsigned int i = 0; i < arrayInfo.num().size(); ++i)
+            arraySize *= arrayInfo.num(i);
+
+        const Token *charSizeToken = nullptr;
+        if (checkMinSizes(*minsizes, &ftok, arraySize, &charSizeToken))
             bufferOverrunError(callstack, arrayInfo.varname());
+        if (charSizeToken)
+            sizeArgumentAsCharError(charSizeToken);
     }
 
     // Calling a user function?
@@ -1277,6 +1292,7 @@ void CheckBufferOverrun::bufferOverrun()
     checkGlobalAndLocalVariable();
     checkStructVariable();
     checkBufferAllocatedWithStrlen();
+    checkStringArgument();
     checkInsecureCmdLineArgs();
 }
 //---------------------------------------------------------------------------
@@ -1504,6 +1520,33 @@ void CheckBufferOverrun::checkBufferAllocatedWithStrlen()
     }
 }
 
+//---------------------------------------------------------------------------
+// memcpy(temp, "hello world", 50);
+//---------------------------------------------------------------------------
+void CheckBufferOverrun::checkStringArgument()
+{
+    const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
+    std::size_t functions = symbolDatabase->functionScopes.size();
+    for (std::size_t functionIndex = 0; functionIndex < functions; ++functionIndex) {
+        const Scope * const scope = symbolDatabase->functionScopes[functionIndex];
+        for (const Token *tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
+            if (!Token::Match(tok, "%var% (") || !_settings->library.hasminsize(tok->str()))
+                continue;
+
+            unsigned int argnr = 1;
+            for (const Token *argtok = tok->tokAt(2); argtok; argtok = argtok->nextArgument(), argnr++) {
+                if (!Token::Match(argtok, "%str% ,|)"))
+                    continue;
+                const std::list<Library::ArgumentChecks::MinSize> *minsizes = _settings->library.argminsizes(tok->str(), argnr);
+                if (!minsizes)
+                    continue;
+                if (checkMinSizes(*minsizes, tok, Token::getStrLength(argtok), nullptr))
+                    bufferOverrunError(argtok, argtok->str());
+            }
+        }
+    }
+}
+
 //---------------------------------------------------------------------------
 // Checking for buffer overflow caused by copying command line arguments
 // into fixed-sized buffers without checking to make sure that the command

lib/checkbufferoverrun.h

@@ -103,12 +103,12 @@ public:
     /** Check for buffer overruns due to allocating strlen(src) bytes instead of (strlen(src)+1) bytes before copying a string */
     void checkBufferAllocatedWithStrlen();
 
+    /** Check string argument buffer overruns */
+    void checkStringArgument();
+
     /** Check for buffer overruns due to copying command-line args to fixed-sized buffers without bounds checking */
     void checkInsecureCmdLineArgs();
 
-    /** Check for negative index */
-    void negativeIndex();
-
     /** Information about N-dimensional array */
     class CPPCHECKLIB ArrayInfo {
     private:

test/testbufferoverrun.cpp

@@ -2240,6 +2240,12 @@ private:
                  "    std::memset(str, 0, 100);\n"
                  "}");
         ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: str\n", errout.str());
+
+        // #5257 - check strings
+        checkstd("void f() {\n"
+                 "  memcpy(temp, \"hello world\", 20);\n"
+                 "}");
+        ASSERT_EQUALS("[test.cpp:2]: (error) Buffer is accessed out of bounds: \"helloworld\"\n", errout.str());
     }