Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions lib/checkbufferoverrun.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -629,28 +629,37 @@ 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: {
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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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);
}
Expand Down
17 changes: 17 additions & 0 deletions test/testbufferoverrun.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Loading