Partial fix for #14960

Author: web-flow

lib/checkbufferoverrun.cpp

@@ -620,19 +620,19 @@ ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, cons
 }
 //---------------------------------------------------------------------------
 
-static bool checkBufferSize(const Token *ftok, const Library::ArgumentChecks::MinSize &minsize, const std::vector<const Token *> &args, const MathLib::bigint bufferSize, const Settings &settings, const Tokenizer* tokenizer)
+static bool checkBufferSize(const Token *ftok, const Library::ArgumentChecks::MinSize &minsize, const std::vector<const Token *> &args, ValueFlow::Value& bufferSize, const Settings &settings, const Tokenizer* tokenizer)
 {
     const Token * const arg = (minsize.arg > 0 && minsize.arg - 1 < args.size()) ? args[minsize.arg - 1] : nullptr;
     const Token * const arg2 = (minsize.arg2 > 0 && minsize.arg2 - 1 < args.size()) ? args[minsize.arg2 - 1] : nullptr;
 
     switch (minsize.type) {
     case Library::ArgumentChecks::MinSize::Type::STRLEN:
         if (settings.library.isargformatstr(ftok, minsize.arg)) {
-            return getMinFormatStringOutputLength(args, minsize.arg, settings) < bufferSize;
+            return getMinFormatStringOutputLength(args, minsize.arg, settings) < bufferSize.intvalue;
         } else if (arg) {
             const Token *strtoken = arg->getValueTokenMaxStrLength();
             if (strtoken)
-                return Token::getStrLength(strtoken) < bufferSize;
+                return Token::getStrLength(strtoken) < bufferSize.intvalue;
         }
         break;
     case Library::ArgumentChecks::MinSize::Type::ARGVALUE: {
@@ -641,7 +641,14 @@ static bool checkBufferSize(const Token *ftok, const Library::ArgumentChecks::Mi
             const int baseSize = tokenizer->sizeOfType(minsize.baseType);
             if (baseSize != 0)
                 myMinsize *= baseSize;
-            return myMinsize <= bufferSize;
+            const bool ok = myMinsize <= bufferSize.intvalue;
+            if (!ok) {
+                if (bufferSize.errorPath.empty())
+                    bufferSize.errorPath = arg->values().front().errorPath;
+                if (!bufferSize.condition)
+                    bufferSize.condition = arg->values().front().condition;
+            }
+            return ok;
         }
         break;
     }
@@ -650,14 +657,14 @@ static bool checkBufferSize(const Token *ftok, const Library::ArgumentChecks::Mi
         break;
     case Library::ArgumentChecks::MinSize::Type::MUL:
         if (arg && arg2 && arg->hasKnownIntValue() && arg2->hasKnownIntValue())
-            return (arg->getKnownIntValue() * arg2->getKnownIntValue()) <= bufferSize;
+            return (arg->getKnownIntValue() * arg2->getKnownIntValue()) <= bufferSize.intvalue;
         break;
     case Library::ArgumentChecks::MinSize::Type::VALUE: {
         MathLib::bigint myMinsize = minsize.value;
         const int baseSize = tokenizer->sizeOfType(minsize.baseType);
         if (baseSize != 0)
             myMinsize *= baseSize;
-        return myMinsize <= bufferSize;
+        return myMinsize <= bufferSize.intvalue;
     }
     case Library::ArgumentChecks::MinSize::Type::NONE:
         break;
@@ -695,7 +702,7 @@ void CheckBufferOverrunImpl::bufferOverflow()
                 if (argtok->valueType() && argtok->valueType()->pointer == 0)
                     continue;
                 // TODO: strcpy(buf+10, "hello");
-                const ValueFlow::Value bufferSize = getBufferSize(argtok, mSettings);
+                ValueFlow::Value bufferSize = getBufferSize(argtok, mSettings);
                 if (bufferSize.intvalue <= 0)
                     continue;
                 // buffer size == 1 => do not warn for dynamic memory
@@ -714,7 +721,7 @@ void CheckBufferOverrunImpl::bufferOverflow()
                     }
                 }
                 const bool error = std::none_of(minsizes->begin(), minsizes->end(), [&](const Library::ArgumentChecks::MinSize &minsize) {
-                    return checkBufferSize(tok, minsize, args, bufferSize.intvalue, mSettings, mTokenizer);
+                    return checkBufferSize(tok, minsize, args, bufferSize, mSettings, mTokenizer);
                 });
                 if (error)
                     bufferOverflowError(args[argnr], &bufferSize, Certainty::normal);
@@ -726,7 +733,7 @@ void CheckBufferOverrunImpl::bufferOverflow()
 void CheckBufferOverrunImpl::bufferOverflowError(const Token *tok, const ValueFlow::Value *value, Certainty certainty)
 {
     const auto errorPath = getErrorPath(tok, value, "Buffer overrun");
-    const auto severity = !value || value->isKnown() ? Severity::error : Severity::warning;
+    const auto severity = !value || (value->isKnown() && !value->condition) ? Severity::error : Severity::warning;
     const std::string msg = "Buffer is accessed out of bounds: " + (tok ? getRealBufferTok(tok)->expressionString() : "buf");
     reportError(errorPath, severity, "bufferAccessOutOfBounds", msg, CWE_BUFFER_OVERRUN, certainty);
 }

test/testbufferoverrun.cpp

@@ -3572,13 +3572,6 @@ class TestBufferOverrun : public TestFixture {
               "    std::memcpy(&u, &s[0], sizeof(u));\n"
               "}\n");
         ASSERT_EQUALS("", errout_str());
-
-        check("void f(const std::vector<uint8_t>& s) {\n"
-              "    if (s.size() == 2) {}\n"
-              "    uint32_t u = 0;\n"
-              "    std::memcpy(&u, &s[0], sizeof(u));\n"
-              "}\n");
-        ASSERT_EQUALS("[test.cpp:4:21]: (warning) Buffer is accessed out of bounds: &s[0] [bufferAccessOutOfBounds]\n", errout_str());
     }
 
     void buffer_overrun_errorpath() {
@@ -3593,6 +3586,24 @@ class TestBufferOverrun : public TestFixture {
         ASSERT_EQUALS("[test.cpp:3:12]: error: Buffer is accessed out of bounds: p [bufferAccessOutOfBounds]\n"
                       "[test.cpp:2:13]: note: Assign p, buffer with size 10\n"
                       "[test.cpp:3:12]: note: Buffer overrun\n", errout_str());
+
+        check("void f(const std::vector<uint8_t>& s) {\n"
+              "    if (s.size() == 2) {}\n"
+              "    uint32_t u = 0;\n"
+              "    std::memcpy(&u, &s[0], sizeof(u));\n"
+              "}\n", s);
+        ASSERT_EQUALS("[test.cpp:4:21]: warning: Buffer is accessed out of bounds: &s[0] [bufferAccessOutOfBounds]\n"
+                      "[test.cpp:2:18]: note: Assuming that condition 's.size()==2' is not redundant\n"
+                      "[test.cpp:4:21]: note: Buffer overrun\n", errout_str());
+
+        check("void f(int i) {\n" // #14960
+              "    int a[1];\n"
+              "    if (i != 2) return;\n"
+              "    memset(a, 0, i * sizeof(int));\n"
+              "}", s);
+        ASSERT_EQUALS("[test.cpp:4:12]: warning: Buffer is accessed out of bounds: a [bufferAccessOutOfBounds]\n"
+                      "[test.cpp:3:11]: note: Assuming that condition 'i!=2' is not redundant\n"
+                      "[test.cpp:4:12]: note: Buffer overrun\n", errout_str());
     }
 
     void buffer_overrun_bailoutIfSwitch() {