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

Warn about buffer overruns when allocating memory with new (#3879) 

* Warn about buffer overruns when allocating memory with new

* Format

* Avoid FP
This commit is contained in:
chrchr-github 2022-03-07 19:43:40 +01:00 committed by GitHub
parent 25360d5e4c
commit fbdfb60809
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 150 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

107
lib/valueflow.cpp
View File

@ -7842,6 +7842,78 @@ struct ContainerConditionHandler : ConditionHandler {
static void valueFlowDynamicBufferSize(TokenList* tokenlist, SymbolDatabase* symboldatabase, const Settings* settings)
{
auto getBufferSizeFromAllocFunc = [&](const Token* funcTok) -> MathLib::bigint {
MathLib::bigint sizeValue = -1;
const Library::AllocFunc* allocFunc = settings->library.getAllocFuncInfo(funcTok);
if (!allocFunc)
allocFunc = settings->library.getReallocFuncInfo(funcTok);
if (!allocFunc || allocFunc->bufferSize == Library::AllocFunc::BufferSize::none)
return sizeValue;
const std::vector<const Token*> args = getArguments(funcTok);
const Token* const arg1 = (args.size() >= allocFunc->bufferSizeArg1) ? args[allocFunc->bufferSizeArg1 - 1] : nullptr;
const Token* const arg2 = (args.size() >= allocFunc->bufferSizeArg2) ? args[allocFunc->bufferSizeArg2 - 1] : nullptr;
switch (allocFunc->bufferSize) {
case Library::AllocFunc::BufferSize::none:
break;
case Library::AllocFunc::BufferSize::malloc:
if (arg1 && arg1->hasKnownIntValue())
sizeValue = arg1->getKnownIntValue();
break;
case Library::AllocFunc::BufferSize::calloc:
if (arg1 && arg2 && arg1->hasKnownIntValue() && arg2->hasKnownIntValue())
sizeValue = arg1->getKnownIntValue() * arg2->getKnownIntValue();
break;
case Library::AllocFunc::BufferSize::strdup:
if (arg1 && arg1->hasKnownValue()) {
const ValueFlow::Value& value = arg1->values().back();
if (value.isTokValue() && value.tokvalue->tokType() == Token::eString)
sizeValue = Token::getStrLength(value.tokvalue) + 1; // Add one for the null terminator
}
break;
}
return sizeValue;
};
auto getBufferSizeFromNew = [&](const Token* newTok) -> MathLib::bigint {
MathLib::bigint sizeValue = -1, numElem = -1;
if (newTok && newTok->astOperand1()) { // number of elements
const Token* bracTok = nullptr, *typeTok = nullptr;
if (newTok->astOperand1()->str() == "[")
bracTok = newTok->astOperand1();
else if (newTok->astOperand1()->str() == "(") {
if (newTok->astOperand1()->astOperand1() && newTok->astOperand1()->astOperand1()->str() == "[")
bracTok = newTok->astOperand1()->astOperand1();
else
typeTok = newTok->astOperand1()->astOperand1();
}
else {
typeTok = newTok->astOperand1();
if (typeTok && typeTok->str() == "{")
typeTok = typeTok->astOperand2();
}
if (bracTok && bracTok->astOperand2() && bracTok->astOperand2()->hasKnownIntValue())
numElem = bracTok->astOperand2()->getKnownIntValue();
else if (Token::Match(typeTok, "%type%"))
numElem = 1;
}
if (numElem >= 0 && newTok->astParent() && newTok->astParent()->isAssignmentOp()) { // size of the allocated type
const Token* typeTok = newTok->astParent()->astOperand1(); // TODO: implement fallback for e.g. "auto p = new Type;"
if (!typeTok || !typeTok->varId())
typeTok = newTok->astParent()->previous(); // hack for "int** z = ..."
if (typeTok && typeTok->valueType()) {
const MathLib::bigint typeSize = typeTok->valueType()->typeSize(*settings, typeTok->valueType()->pointer > 1);
if (typeSize >= 0)
sizeValue = numElem * typeSize;
}
}
return sizeValue;
};
for (const Scope *functionScope : symboldatabase->functionScopes) {
for (const Token *tok = functionScope->bodyStart; tok != functionScope->bodyEnd; tok = tok->next()) {
if (!Token::Match(tok, "[;{}] %var% ="))
@ -7856,40 +7928,11 @@ static void valueFlowDynamicBufferSize(TokenList* tokenlist, SymbolDatabase* sym
if (!rhs)
continue;
if (!Token::Match(rhs->previous(), "%name% ("))
const bool isNew = symboldatabase->isCPP() && rhs->str() == "new";
if (!isNew && !Token::Match(rhs->previous(), "%name% ("))
continue;
const Library::AllocFunc *allocFunc = settings->library.getAllocFuncInfo(rhs->previous());
if (!allocFunc)
allocFunc = settings->library.getReallocFuncInfo(rhs->previous());
if (!allocFunc || allocFunc->bufferSize == Library::AllocFunc::BufferSize::none)
continue;
const std::vector<const Token *> args = getArguments(rhs->previous());
const Token * const arg1 = (args.size() >= allocFunc->bufferSizeArg1) ? args[allocFunc->bufferSizeArg1 - 1] : nullptr;
const Token * const arg2 = (args.size() >= allocFunc->bufferSizeArg2) ? args[allocFunc->bufferSizeArg2 - 1] : nullptr;
MathLib::bigint sizeValue = -1;
switch (allocFunc->bufferSize) {
case Library::AllocFunc::BufferSize::none:
break;
case Library::AllocFunc::BufferSize::malloc:
if (arg1 && arg1->hasKnownIntValue())
sizeValue = arg1->getKnownIntValue();
break;
case Library::AllocFunc::BufferSize::calloc:
if (arg1 && arg2 && arg1->hasKnownIntValue() && arg2->hasKnownIntValue())
sizeValue = arg1->getKnownIntValue() * arg2->getKnownIntValue();
break;
case Library::AllocFunc::BufferSize::strdup:
if (arg1 && arg1->hasKnownValue()) {
const ValueFlow::Value &value = arg1->values().back();
if (value.isTokValue() && value.tokvalue->tokType() == Token::eString)
sizeValue = Token::getStrLength(value.tokvalue) + 1; // Add one for the null terminator
}
break;
}
const MathLib::bigint sizeValue = isNew ? getBufferSizeFromNew(rhs) : getBufferSizeFromAllocFunc(rhs->previous());
if (sizeValue < 0)
continue;

83
test/testbufferoverrun.cpp
View File

@ -214,6 +214,7 @@ private:
TEST_CASE(array_index_enum_array); // #8439
TEST_CASE(array_index_container); // #9386
TEST_CASE(array_index_two_for_loops);
TEST_CASE(array_index_new); // #7690
TEST_CASE(buffer_overrun_2_struct);
TEST_CASE(buffer_overrun_3);
@ -441,7 +442,7 @@ private:
" str[i] = 0;\n"
"}\n"
"void b() { a(16); }");
TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Array 'str[16]' accessed at index 16, which is out of bounds.\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'str[16]' accessed at index 16, which is out of bounds.\n", errout.str());
}
void array_index_4() {
@ -2560,6 +2561,72 @@ private:
ASSERT_EQUALS("", errout.str());
}
void array_index_new() { // #7690
check("void f() {\n"
" int* z = new int;\n"
" for (int n = 0; n < 8; ++n)\n"
" z[n] = 0;\n"
" delete[] z;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'z[1]' accessed at index 7, which is out of bounds.\n", errout.str());
check("void f() {\n"
" int* z = new int(1);\n"
" for (int n = 0; n < 8; ++n)\n"
" z[n] = 0;\n"
" delete[] z;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'z[1]' accessed at index 7, which is out of bounds.\n", errout.str());
check("void f() {\n"
" int* z = new int{};\n"
" for (int n = 0; n < 8; ++n)\n"
" z[n] = 0;\n"
" delete[] z;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'z[1]' accessed at index 7, which is out of bounds.\n", errout.str());
check("void f() {\n"
" int* z = new int[5];\n"
" for (int n = 0; n < 8; ++n)\n"
" z[n] = 0;\n"
" delete[] z;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'z[5]' accessed at index 7, which is out of bounds.\n", errout.str());
check("void g() {\n"
" int* z = new int[5]();\n"
" for (int n = 0; n < 8; ++n)\n"
" z[n] = 1;\n"
" delete[] z;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'z[5]' accessed at index 7, which is out of bounds.\n", errout.str());
check("void h() {\n"
" int** z = new int* [5];\n"
" for (int n = 0; n < 8; ++n)\n"
" z[n] = nullptr;\n"
" delete[] z;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'z[5]' accessed at index 7, which is out of bounds.\n", errout.str());
check("void h() {\n"
" int** z = new int* [5]();\n"
" for (int n = 0; n < 8; ++n)\n"
" z[n] = nullptr;\n"
" delete[] z;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'z[5]' accessed at index 7, which is out of bounds.\n", errout.str());
check("void h() {\n"
" int** z = new int* [5]{};\n"
" for (int n = 0; n < 8; ++n)\n"
" z[n] = nullptr;\n"
" delete[] z;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'z[5]' accessed at index 7, which is out of bounds.\n", errout.str());
}
void buffer_overrun_2_struct() {
check("struct ABC\n"
"{\n"
@ -3405,7 +3472,7 @@ private:
" char *s; s = new char[10];\n"
" s[10] = 0;\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Array 's[10]' accessed at index 10, which is out of bounds.\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (error) Array 's[10]' accessed at index 10, which is out of bounds.\n", errout.str());
// ticket #1670 - false negative when using return
check("char f()\n"
@ -3413,7 +3480,7 @@ private:
" int *s; s = new int[10];\n"
" return s[10];\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Array 's[10]' accessed at index 10, which is out of bounds.\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (error) Array 's[10]' accessed at index 10, which is out of bounds.\n", errout.str());
check("struct Fred { char c[10]; };\n"
"char f()\n"
@ -3446,7 +3513,7 @@ private:
" buf[9] = 0;\n"
" delete [] buf;\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:6]: (error) Array 'buf[9]' accessed at index 9, which is out of bounds.\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:6]: (error) Array 'buf[9]' accessed at index 9, which is out of bounds.\n", errout.str());
check("void foo()\n"
"{\n"
@ -3454,7 +3521,7 @@ private:
" char *s; s = new char[Size];\n"
" s[Size] = 0;\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:5]: (error) Array 's[10]' accessed at index 10, which is out of bounds.\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:5]: (error) Array 's[10]' accessed at index 10, which is out of bounds.\n", errout.str());
check("void foo()\n"
"{\n"
@ -3462,7 +3529,7 @@ private:
" E *e; e = new E[10];\n"
" e[10] = ZERO;\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:5]: (error) Array 'e[10]' accessed at index 10, which is out of bounds.\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:5]: (error) Array 'e[10]' accessed at index 10, which is out of bounds.\n", errout.str());
}
// data is allocated with malloc
@ -3557,7 +3624,7 @@ private:
" for (int i = 0; i < 3; i++)\n"
" a[i] = NULL;\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Array 'a[2]' accessed at index 2, which is out of bounds.\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Array 'a[2]' accessed at index 2, which is out of bounds.\n", errout.str());
}
// statically allocated buffer
@ -3930,7 +3997,7 @@ private:
" char *str = new char[5];\n"
" mysprintf(str, \"abcde\");\n"
"}", settings);
TODO_ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds.\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: str\n", errout.str());
check("void f(int condition) {\n"
" char str[5];\n"