Skip to content

Commit 5e7baff

Browse files
Fix #14960 Inconsistent bufferAccessOutOfBounds (possible values) (#8779)
After #8763 --------- Co-authored-by: chrchr-github <noreply@github.com>
1 parent 8f7666d commit 5e7baff

2 files changed

Lines changed: 39 additions & 11 deletions

File tree

lib/checkbufferoverrun.cpp

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -629,28 +629,39 @@ ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, cons
629629
}
630630
//---------------------------------------------------------------------------
631631

632-
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)
632+
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)
633633
{
634634
const Token * const arg = (minsize.arg > 0 && minsize.arg - 1 < args.size()) ? args[minsize.arg - 1] : nullptr;
635635
const Token * const arg2 = (minsize.arg2 > 0 && minsize.arg2 - 1 < args.size()) ? args[minsize.arg2 - 1] : nullptr;
636636

637637
switch (minsize.type) {
638638
case Library::ArgumentChecks::MinSize::Type::STRLEN:
639639
if (settings.library.isargformatstr(ftok, minsize.arg)) {
640-
return getMinFormatStringOutputLength(args, minsize.arg, settings) < bufferSize;
640+
return getMinFormatStringOutputLength(args, minsize.arg, settings) < bufferSize.intvalue;
641641
} else if (arg) {
642642
const Token *strtoken = arg->getValueTokenMaxStrLength();
643643
if (strtoken)
644-
return Token::getStrLength(strtoken) < bufferSize;
644+
return Token::getStrLength(strtoken) < bufferSize.intvalue;
645645
}
646646
break;
647647
case Library::ArgumentChecks::MinSize::Type::ARGVALUE: {
648-
if (arg && arg->hasKnownIntValue()) {
649-
MathLib::bigint myMinsize = arg->getKnownIntValue();
648+
if (arg) {
649+
const ValueFlow::Value* argVal = arg->hasKnownIntValue() ?
650+
arg->getKnownValue(ValueFlow::Value::ValueType::INT) :
651+
arg->getMaxValue(/*condition*/ true);
652+
if (!argVal)
653+
break;
654+
MathLib::bigint myMinsize = argVal->intvalue;
650655
const int baseSize = tokenizer->sizeOfType(minsize.baseType);
651656
if (baseSize != 0)
652657
myMinsize *= baseSize;
653-
return myMinsize <= bufferSize;
658+
const bool ok = myMinsize <= bufferSize.intvalue;
659+
if (!ok) {
660+
bufferSize.errorPath.insert(bufferSize.errorPath.end(), argVal->errorPath.begin(), argVal->errorPath.end());
661+
if (!bufferSize.condition)
662+
bufferSize.condition = argVal->condition;
663+
}
664+
return ok;
654665
}
655666
break;
656667
}
@@ -659,14 +670,14 @@ static bool checkBufferSize(const Token *ftok, const Library::ArgumentChecks::Mi
659670
break;
660671
case Library::ArgumentChecks::MinSize::Type::MUL:
661672
if (arg && arg2 && arg->hasKnownIntValue() && arg2->hasKnownIntValue())
662-
return (arg->getKnownIntValue() * arg2->getKnownIntValue()) <= bufferSize;
673+
return (arg->getKnownIntValue() * arg2->getKnownIntValue()) <= bufferSize.intvalue;
663674
break;
664675
case Library::ArgumentChecks::MinSize::Type::VALUE: {
665676
MathLib::bigint myMinsize = minsize.value;
666677
const int baseSize = tokenizer->sizeOfType(minsize.baseType);
667678
if (baseSize != 0)
668679
myMinsize *= baseSize;
669-
return myMinsize <= bufferSize;
680+
return myMinsize <= bufferSize.intvalue;
670681
}
671682
case Library::ArgumentChecks::MinSize::Type::NONE:
672683
break;
@@ -704,7 +715,7 @@ void CheckBufferOverrunImpl::bufferOverflow()
704715
if (argtok->valueType() && argtok->valueType()->pointer == 0)
705716
continue;
706717
// TODO: strcpy(buf+10, "hello");
707-
const ValueFlow::Value bufferSize = getBufferSize(argtok, mSettings);
718+
ValueFlow::Value bufferSize = getBufferSize(argtok, mSettings);
708719
if (bufferSize.intvalue <= 0)
709720
continue;
710721
// buffer size == 1 => do not warn for dynamic memory
@@ -723,7 +734,7 @@ void CheckBufferOverrunImpl::bufferOverflow()
723734
}
724735
}
725736
const bool error = std::none_of(minsizes->begin(), minsizes->end(), [&](const Library::ArgumentChecks::MinSize &minsize) {
726-
return checkBufferSize(tok, minsize, args, bufferSize.intvalue, mSettings, mTokenizer);
737+
return checkBufferSize(tok, minsize, args, bufferSize, mSettings, mTokenizer);
727738
});
728739
if (error)
729740
bufferOverflowError(args[argnr], &bufferSize, Certainty::normal);
@@ -735,7 +746,7 @@ void CheckBufferOverrunImpl::bufferOverflow()
735746
void CheckBufferOverrunImpl::bufferOverflowError(const Token *tok, const ValueFlow::Value *value, Certainty certainty)
736747
{
737748
const auto errorPath = getErrorPath(tok, value, "Buffer overrun");
738-
const auto severity = !value || value->isKnown() ? Severity::error : Severity::warning;
749+
const auto severity = !value || (value->isKnown() && !value->condition) ? Severity::error : Severity::warning;
739750
const std::string msg = "Buffer is accessed out of bounds: " + (tok ? getRealBufferTok(tok)->expressionString() : "buf");
740751
reportError(errorPath, severity, "bufferAccessOutOfBounds", msg, CWE_BUFFER_OVERRUN, certainty);
741752
}

test/testbufferoverrun.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3659,6 +3659,23 @@ class TestBufferOverrun : public TestFixture {
36593659
ASSERT_EQUALS("[test.cpp:4:21]: warning: Buffer is accessed out of bounds: &s[0] [bufferAccessOutOfBounds]\n"
36603660
"[test.cpp:2:18]: note: Assuming that condition 's.size()==2' is not redundant\n"
36613661
"[test.cpp:4:21]: note: Buffer overrun\n", errout_str());
3662+
3663+
check("void f(int i) {\n" // #14960
3664+
" int a[1];\n"
3665+
" if (i != 2) return;\n"
3666+
" memset(a, 0, i * sizeof(int));\n"
3667+
"}"
3668+
"void g(int i) {\n"
3669+
" int a[1];\n"
3670+
" if (i != 2) {}\n"
3671+
" memset(a, 0, i * sizeof(int));\n"
3672+
"}", s);
3673+
ASSERT_EQUALS("[test.cpp:4:12]: warning: Buffer is accessed out of bounds: a [bufferAccessOutOfBounds]\n"
3674+
"[test.cpp:3:11]: note: Assuming that condition 'i!=2' is not redundant\n"
3675+
"[test.cpp:4:12]: note: Buffer overrun\n"
3676+
"[test.cpp:8:12]: warning: Buffer is accessed out of bounds: a [bufferAccessOutOfBounds]\n"
3677+
"[test.cpp:7:11]: note: Assuming that condition 'i!=2' is not redundant\n"
3678+
"[test.cpp:8:12]: note: Buffer overrun\n", errout_str());
36623679
}
36633680

36643681
void buffer_overrun_bailoutIfSwitch() {

0 commit comments

Comments
 (0)