walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Better multiline warning when there is buffer overflow

This commit is contained in:
Daniel Marjamäki 2019-03-17 20:12:02 +01:00
parent 3c85d8a8ac
commit 03f8535c71
3 changed files with 39 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
lib/checkbufferoverrun.cpp
View File

@ -318,30 +318,37 @@ void CheckBufferOverrun::negativeIndexError(const Token *tok, const std::vector<
//---------------------------------------------------------------------------
size_t CheckBufferOverrun::getBufferSize(const Token *bufTok) const
ValueFlow::Value CheckBufferOverrun::getBufferSize(const Token *bufTok) const
{
if (!bufTok->valueType())
return 0;
return ValueFlow::Value(-1);
const Variable *var = bufTok->variable();
if (!var || var->dimensions().empty()) {
const ValueFlow::Value *value = getBufferSizeValue(bufTok);
if (value)
return value->intvalue;
return *value;
}
if (!var)
return 0;
return ValueFlow::Value(-1);
MathLib::bigint dim = 1;
for (const Dimension &d : var->dimensions())
dim *= d.num;
if (var->isPointerArray())
return dim * mSettings->sizeof_pointer;
ValueFlow::Value v;
v.setKnown();
v.valueType = ValueFlow::Value::ValueType::BUFFER_SIZE;
const MathLib::bigint typeSize = bufTok->valueType()->typeSize(*mSettings);
return dim * typeSize;
if (var->isPointerArray())
v.intvalue = dim * mSettings->sizeof_pointer;
else {
const MathLib::bigint typeSize = bufTok->valueType()->typeSize(*mSettings);
v.intvalue = dim * typeSize;
}
return v;
}
//---------------------------------------------------------------------------
@ -403,26 +410,26 @@ void CheckBufferOverrun::bufferOverflow()
if (!argtok || !argtok->variable())
continue;
// TODO: strcpy(buf+10, "hello");
const size_t bufferSize = getBufferSize(argtok);
if (bufferSize <= 1)
const ValueFlow::Value bufferSize = getBufferSize(argtok);
if (bufferSize.intvalue <= 1)
continue;
bool error = true;
for (const Library::ArgumentChecks::MinSize &minsize : *minsizes) {
if (checkBufferSize(tok, minsize, args, bufferSize, mSettings)) {
if (checkBufferSize(tok, minsize, args, bufferSize.intvalue, mSettings)) {
error = false;
break;
}
}
if (error)
bufferOverflowError(args[argnr]);
bufferOverflowError(args[argnr], &bufferSize);
}
}
}
}
void CheckBufferOverrun::bufferOverflowError(const Token *tok)
void CheckBufferOverrun::bufferOverflowError(const Token *tok, const ValueFlow::Value *value)
{
reportError(tok, Severity::error, "bufferAccessOutOfBounds", "Buffer is accessed out of bounds: " + (tok ? tok->expressionString() : "buf"), CWE788, false);
reportError(getErrorPath(tok, value, "Buffer overrun"), Severity::error, "bufferAccessOutOfBounds", "Buffer is accessed out of bounds: " + (tok ? tok->expressionString() : "buf"), CWE788, false);
}
//---------------------------------------------------------------------------
@ -498,8 +505,8 @@ void CheckBufferOverrun::stringNotZeroTerminated()
const Token *sizeToken = args[2];
if (!sizeToken->hasKnownIntValue())
continue;
const size_t bufferSize = getBufferSize(args[0]);
if (bufferSize == 0 || sizeToken->getKnownIntValue() < bufferSize)
const ValueFlow::Value &bufferSize = getBufferSize(args[0]);
if (bufferSize.intvalue < 0 || sizeToken->getKnownIntValue() < bufferSize.intvalue)
continue;
const Token *srcValue = args[1]->getValueTokenMaxStrLength();
if (srcValue && Token::getStrLength(srcValue) < sizeToken->getKnownIntValue())

5
lib/checkbufferoverrun.h
View File

@ -73,6 +73,7 @@ public:
c.arrayIndexError(nullptr, std::vector<Dimension>(), nullptr);
c.negativeIndexError(nullptr, std::vector<Dimension>(), nullptr);
c.arrayIndexThenCheckError(nullptr, "i");
c.bufferOverflowError(nullptr, nullptr);
}
private:
@ -82,7 +83,7 @@ private:
void negativeIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const ValueFlow::Value *negativeValue);
void bufferOverflow();
void bufferOverflowError(const Token *tok);
void bufferOverflowError(const Token *tok, const ValueFlow::Value *value);
void arrayIndexThenCheck();
void arrayIndexThenCheckError(const Token *tok, const std::string &indexName);
@ -91,7 +92,7 @@ private:
void terminateStrncpyError(const Token *tok, const std::string &varname);
void bufferNotZeroTerminatedError(const Token *tok, const std::string &varname, const std::string &function);
size_t getBufferSize(const Token *bufTok) const;
ValueFlow::Value getBufferSize(const Token *bufTok) const;
static std::string myName() {
return "Bounds checking";

19
test/testbufferoverrun.cpp
View File

@ -172,6 +172,7 @@ private:
TEST_CASE(buffer_overrun_27); // #4444 (segmentation fault)
TEST_CASE(buffer_overrun_29); // #7083: false positive: typedef and initialization with strings
TEST_CASE(buffer_overrun_30); // #6367
TEST_CASE(buffer_overrun_errorpath);
// TODO CTU TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch
// TODO TEST_CASE(buffer_overrun_function_array_argument);
// TODO alloca TEST_CASE(possible_buffer_overrun_1); // #3035
@ -2571,6 +2572,18 @@ private:
ASSERT_EQUALS("[test.cpp:3]: (error) Array 's->m[9]' accessed at index 36, which is out of bounds.\n", errout.str());
}
void buffer_overrun_errorpath() {
setMultiline();
settings0.templateLocation = "{file}:{line}:note:{info}";
check("void f() {\n"
" char *p = malloc(10);\n"
" memset(p, 0, 20);\n"
"}");
ASSERT_EQUALS("test.cpp:3:error:Buffer is accessed out of bounds: p\n"
"test.cpp:2:note:Assign p, buffer with size 10\n"
"test.cpp:3:note:Buffer overrun\n", errout.str());
}
void buffer_overrun_bailoutIfSwitch() {
// No false positive
@ -3032,12 +3045,6 @@ private:
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 's[10]' accessed at index 10, which is out of bounds.\n", errout.str());
check("void foo() {\n"
" char *p = malloc(10);\n"
" memset(p, 0, 100);\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: p\n", errout.str());
// ticket #842
check("void f() {\n"
" int *tab4 = malloc(20 * sizeof(int));\n"