diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index f0a12efccec..4843f5faa9e 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -629,7 +629,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; @@ -637,20 +637,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; } @@ -659,14 +668,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; @@ -704,7 +713,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 @@ -723,7 +732,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); @@ -735,7 +744,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); } diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 3914f0ee828..229b2883b9e 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -3659,6 +3659,23 @@ class TestBufferOverrun : public TestFixture { 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() {