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

CheckBufferOverrun: Reimplement CheckBufferOverrun::argumentSize check

This commit is contained in:
Daniel Marjamäki 2021-08-22 16:37:41 +02:00
parent 0662c94d83
commit db57efa486
3 changed files with 64 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
lib/checkbufferoverrun.cpp
View File

@ -52,6 +52,7 @@ namespace {
// CWE ids used:
static const CWE CWE131(131U); // Incorrect Calculation of Buffer Size
static const CWE CWE170(170U); // Improper Null Termination
static const CWE CWE_ARGUMENT_SIZE(398U); // Indicator of Poor Code Quality
static const CWE CWE_ARRAY_INDEX_THEN_CHECK(398U); // Indicator of Poor Code Quality
static const CWE CWE682(682U); // Incorrect Calculation
static const CWE CWE758(758U); // Reliance on Undefined, Unspecified, or Implementation-Defined Behavior
@ -761,8 +762,61 @@ void CheckBufferOverrun::terminateStrncpyError(const Token *tok, const std::stri
"zero at the end of the buffer. This causes bugs later in the code if the code "
"assumes buffer is null-terminated.", CWE170, Certainty::inconclusive);
}
//---------------------------------------------------------------------------
void CheckBufferOverrun::argumentSize()
{
// Check '%type% x[10]' arguments
if (!mSettings->severity.isEnabled(Severity::warning))
return;
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
for (const Scope * const scope : symbolDatabase->functionScopes) {
for (const Token *tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) {
if (!tok->function() || !Token::Match(tok, "%name% ("))
continue;
// If argument is '%type% a[num]' then check bounds against num
const Function *callfunc = tok->function();
const std::vector<const Token *> callargs = getArguments(tok);
for (nonneg int paramIndex = 0; paramIndex < callargs.size() && paramIndex < callfunc->argCount(); ++paramIndex) {
const Variable* const argument = callfunc->getArgumentVar(paramIndex);
if (!argument || !argument->nameToken() || !argument->isArray())
continue;
if (!argument->valueType() || !callargs[paramIndex]->valueType())
continue;
if (argument->valueType()->type != callargs[paramIndex]->valueType()->type)
continue;
const Token * calldata = callargs[paramIndex];
while (Token::Match(calldata, "::|."))
calldata = calldata->astOperand2();
if (!calldata->variable() || !calldata->variable()->isArray())
continue;
if (calldata->variable()->dimensions().size() != argument->dimensions().size())
continue;
bool err = false;
for (int d = 0; d < argument->dimensions().size(); ++d) {
const auto& dim1 = calldata->variable()->dimensions()[d];
const auto& dim2 = argument->dimensions()[d];
if (!dim1.known || !dim2.known)
break;
if (dim1.num < dim2.num)
err = true;
}
if (err)
argumentSizeError(tok, tok->str(), argument->name());
}
}
}
}
void CheckBufferOverrun::argumentSizeError(const Token *tok, const std::string &functionName, const std::string &varname)
{
reportError(tok, Severity::warning, "argumentSize",
"$symbol:" + functionName + '\n' +
"$symbol:" + varname + '\n' +
"The array '" + varname + "' is too small, the function '" + functionName + "' expects a bigger one.", CWE_ARGUMENT_SIZE, Certainty::normal);
}
//---------------------------------------------------------------------------
// CTU..

8
lib/checkbufferoverrun.h
View File

@ -74,6 +74,7 @@ public:
checkBufferOverrun.arrayIndexThenCheck();
checkBufferOverrun.stringNotZeroTerminated();
checkBufferOverrun.objectIndex();
checkBufferOverrun.argumentSize();
}
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE {
@ -84,6 +85,7 @@ public:
c.arrayIndexThenCheckError(nullptr, "i");
c.bufferOverflowError(nullptr, nullptr, Certainty::normal);
c.objectIndexError(nullptr, nullptr, true);
c.argumentSizeError(nullptr, "function", "buffer");
}
/** @brief Parse current TU and extract file info */
@ -114,6 +116,9 @@ private:
void stringNotZeroTerminated();
void terminateStrncpyError(const Token *tok, const std::string &varname);
void argumentSize();
void argumentSizeError(const Token *tok, const std::string &functionName, const std::string &varname);
void objectIndex();
void objectIndexError(const Token *tok, const ValueFlow::Value *v, bool known);
@ -153,7 +158,8 @@ private:
"- Buffer overflow\n"
"- Dangerous usage of strncat()\n"
"- Using array index before checking it\n"
"- Partial string write that leads to buffer that is not zero terminated.\n";
"- Partial string write that leads to buffer that is not zero terminated.\n"
"- Check for large enough arrays being passed to functions\n";
}
};
/// @}

13
test/testbufferoverrun.cpp
View File

@ -2855,28 +2855,21 @@ private:
" char a[2];\n"
" f(a);\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:4]: (warning) The array 'a' is too small, the function 'f' expects a bigger one.\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (warning) The array 'a' is too small, the function 'f' expects a bigger one.\n", errout.str());
check("void f(float a[10][3]);\n"
"void g() {\n"
" float a[2][3];\n"
" f(a);\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:4]: (warning) The array 'a' is too small, the function 'f' expects a bigger one.\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (warning) The array 'a' is too small, the function 'f' expects a bigger one.\n", errout.str());
check("void f(int a[20]);\n"
"void g() {\n"
" int a[2];\n"
" f(a);\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:4]: (warning) The array 'a' is too small, the function 'f' expects a bigger one.\n", "", errout.str());
check("void f(int a[20]);\n"
"void g() {\n"
" int a[5];\n"
" f(a);\n"
"}");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (warning) The array 'a' is too small, the function 'f' expects a bigger one.\n", errout.str());
check("void f(int a[]) {\n"
" switch (2) {\n"