diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 46d8e1c7c6c..19d2a6cc1ab 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -68,7 +68,9 @@ static const ValueFlow::Value *getBufferSizeValue(const Token *tok) auto it = std::find_if(tokenValues.cbegin(), tokenValues.cend(), std::mem_fn(&ValueFlow::Value::isBufferSizeValue)); if (it != tokenValues.cend()) return &*it; - it = std::find_if(tokenValues.cbegin(), tokenValues.cend(), std::mem_fn(&ValueFlow::Value::isContainerSizeValue)); + it = std::find_if(tokenValues.cbegin(), tokenValues.cend(), [](const ValueFlow::Value& v) { + return v.isContainerSizeValue() && !v.isImpossible(); + }); return it == tokenValues.cend() ? nullptr : &*it; } @@ -588,6 +590,8 @@ ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, cons ValueFlow::Value bufSizeVal; bufSizeVal.valueType = ValueFlow::Value::ValueType::BUFFER_SIZE; bufSizeVal.intvalue = value->intvalue * elementSize; + bufSizeVal.valueKind = value->valueKind; + bufSizeVal.errorPath = value->errorPath; return bufSizeVal; } } @@ -616,7 +620,7 @@ ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, cons } //--------------------------------------------------------------------------- -static bool checkBufferSize(const Token *ftok, const Library::ArgumentChecks::MinSize &minsize, const std::vector &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 &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; @@ -624,20 +628,29 @@ static bool checkBufferSize(const Token *ftok, const Library::ArgumentChecks::Mi 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: { - if (arg && arg->hasKnownIntValue()) { - MathLib::bigint myMinsize = arg->getKnownIntValue(); + if (arg) { + const ValueFlow::Value* argVal = arg->hasKnownIntValue() ? &arg->values().front() : arg->getMaxValue(true); + if (!argVal) + break; + MathLib::bigint myMinsize = argVal->intvalue; const int baseSize = tokenizer->sizeOfType(minsize.baseType); if (baseSize != 0) myMinsize *= baseSize; - return myMinsize <= bufferSize; + const bool ok = myMinsize <= bufferSize.intvalue; + if (!ok) { + bufferSize.errorPath.insert(bufferSize.errorPath.end(), argVal->errorPath.begin(), argVal->errorPath.end()); + if (!bufferSize.condition) + bufferSize.condition = argVal->condition; + } + return ok; } break; } @@ -646,14 +659,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; @@ -691,7 +704,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 @@ -710,7 +723,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); @@ -721,7 +734,10 @@ void CheckBufferOverrunImpl::bufferOverflow() void CheckBufferOverrunImpl::bufferOverflowError(const Token *tok, const ValueFlow::Value *value, Certainty certainty) { - reportError(getErrorPath(tok, value, "Buffer overrun"), Severity::error, "bufferAccessOutOfBounds", "Buffer is accessed out of bounds: " + (tok ? getRealBufferTok(tok)->expressionString() : "buf"), CWE_BUFFER_OVERRUN, certainty); + const auto errorPath = getErrorPath(tok, value, "Buffer overrun"); + 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); } //--------------------------------------------------------------------------- diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 37ab7083902..d3db1f2ad3f 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -3564,6 +3564,14 @@ class TestBufferOverrun : public TestFixture { " memset(&a[i], 0, sizeof(a));\n" "}\n"); ASSERT_EQUALS("[test.cpp:4:16]: (error) Buffer is accessed out of bounds: &a[i] [bufferAccessOutOfBounds]\n", errout_str()); + + check("void f(const std::vector& s) {\n" // #14948 + " if (s.size() < 4)\n" + " return;\n" + " uint32_t u = 0;\n" + " std::memcpy(&u, &s[0], sizeof(u));\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); } void buffer_overrun_errorpath() { @@ -3578,6 +3586,32 @@ 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& 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" + "}" + "void g(int i) {\n" + " int a[1];\n" + " if (i != 2) {}\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" + "[test.cpp:8:12]: warning: Buffer is accessed out of bounds: a [bufferAccessOutOfBounds]\n" + "[test.cpp:7:11]: note: Assuming that condition 'i!=2' is not redundant\n" + "[test.cpp:8:12]: note: Buffer overrun\n", errout_str()); } void buffer_overrun_bailoutIfSwitch() {