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

CheckBufferOverrun: Handle multidimensional arrays

This commit is contained in:
Daniel Marjamäki 2019-03-19 09:29:32 +01:00
parent 774464eabb
commit e98a4a6f14
3 changed files with 137 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

137
lib/checkbufferoverrun.cpp
View File

@ -201,8 +201,15 @@ void CheckBufferOverrun::arrayIndex()
continue;
}
const Token *indexToken = tok->astOperand2();
if (!indexToken)
std::vector<const Token *> indexTokens;
for (const Token *tok2 = tok; tok2 && tok2->str() == "["; tok2 = tok2->link()->next()) {
if (!tok2->astOperand2()) {
indexTokens.clear();
break;
}
indexTokens.emplace_back(tok2->astOperand2());
}
if (indexTokens.empty())
continue;
std::vector<Dimension> dimensions;
@ -212,7 +219,17 @@ void CheckBufferOverrun::arrayIndex()
if (array->variable()->isArray() && !array->variable()->dimensions().empty()) {
dimensions = array->variable()->dimensions();
mightBeLarger = (dimensions.size() >= 1 && (dimensions[0].num <= 1 || !dimensions[0].tok));
if (dimensions.size() >= 1 && (dimensions[0].num <= 1 || !dimensions[0].tok)) {
mightBeLarger = false;
visitAstNodes(tok->astOperand1(),
[&](const Token *child) {
if (child->originalName() == "->") {
mightBeLarger = true;
return ChildrenToVisit::none;
}
return ChildrenToVisit::op1_and_op2;
});
}
} else if (const Token *stringLiteral = array->getValueTokenMinStrSize()) {
Dimension dim;
dim.tok = nullptr;
@ -236,53 +253,94 @@ void CheckBufferOverrun::arrayIndex()
if (dimensions.empty())
continue;
const MathLib::bigint dim = dimensions[0].num;
// Positive index
if (!mightBeLarger) { // TODO check arrays with dim 1 also
for (int cond = 0; cond < 2; cond++) {
const ValueFlow::Value *value = indexToken->getMaxValue(cond == 1);
if (!value)
continue;
const MathLib::bigint index = value->intvalue;
if (index < dim)
continue;
if (index == dim) {
bool equal = false;
bool overflow = false;
bool allKnown = true;
std::vector<const ValueFlow::Value *> indexes;
for (size_t i = 0; i < dimensions.size() && i < indexTokens.size(); ++i) {
const ValueFlow::Value *value = indexTokens[i]->getMaxValue(cond == 1);
indexes.push_back(value);
if (!value)
continue;
if (!value->isKnown()) {
if (!allKnown)
continue;
allKnown = false;
}
if (array->variable()->isArray() && dimensions[i].num == 0)
continue;
if (value->intvalue == dimensions[i].num)
equal = true;
else if (value->intvalue > dimensions[i].num)
overflow = true;
}
if (!overflow && equal) {
const Token *parent = tok;
while (Token::simpleMatch(parent, "["))
parent = parent->astParent();
if (parent->isUnaryOp("&"))
continue;
}
arrayIndexError(tok, dimensions, value);
if (overflow || equal) {
arrayIndexError(tok, dimensions, indexes);
break;
}
}
}
// Negative index
const ValueFlow::Value *negativeValue = indexToken->getValueLE(-1, mSettings);
if (negativeValue) {
negativeIndexError(tok, dimensions, negativeValue);
bool neg = false;
std::vector<const ValueFlow::Value *> negativeIndexes;
for (size_t i = 0; i < indexTokens.size(); ++i) {
const ValueFlow::Value *negativeValue = indexTokens[i]->getValueLE(-1, mSettings);
negativeIndexes.emplace_back(negativeValue);
if (negativeValue)
neg = true;
}
if (neg) {
negativeIndexError(tok, dimensions, negativeIndexes);
}
}
}
static std::string arrayIndexMessage(const Token *tok, const std::vector<Dimension> &dimensions, const ValueFlow::Value *index)
static std::string stringifyIndexes(const std::string &array, const std::vector<const ValueFlow::Value *> &indexValues)
{
if (indexValues.size() == 1)
return MathLib::toString(indexValues[0]->intvalue);
std::ostringstream ret;
ret << array;
for (const ValueFlow::Value *index : indexValues) {
ret << "[";
if (index)
ret << index->intvalue;
else
ret << "*";
ret << "]";
}
return ret.str();
}
static std::string arrayIndexMessage(const Token *tok, const std::vector<Dimension> &dimensions, const std::vector<const ValueFlow::Value *> &indexValues, const Token *condition)
{
std::string array = tok->astOperand1()->expressionString();
for (const Dimension &dim : dimensions)
array += "[" + MathLib::toString(dim.num) + "]";
std::ostringstream errmsg;
if (index->condition)
errmsg << ValueFlow::eitherTheConditionIsRedundant(index->condition)
<< " or the array '" + array + "' is accessed at index " << index->intvalue << ", which is out of bounds.";
if (condition)
errmsg << ValueFlow::eitherTheConditionIsRedundant(condition)
<< " or the array '" + array + "' is accessed at index " << stringifyIndexes(tok->astOperand1()->expressionString(), indexValues) << ", which is out of bounds.";
else
errmsg << "Array '" << array << "' accessed at index " << index->intvalue << ", which is out of bounds.";
errmsg << "Array '" << array << "' accessed at index " << stringifyIndexes(tok->astOperand1()->expressionString(), indexValues) << ", which is out of bounds.";
return errmsg.str();
}
void CheckBufferOverrun::arrayIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const ValueFlow::Value *index)
void CheckBufferOverrun::arrayIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const std::vector<const ValueFlow::Value *> &indexes)
{
if (!tok) {
reportError(tok, Severity::error, "arrayIndexOutOfBounds", "Array 'arr[16]' accessed at index 16, which is out of bounds.", CWE788, false);
@ -290,28 +348,51 @@ void CheckBufferOverrun::arrayIndexError(const Token *tok, const std::vector<Dim
return;
}
const Token *condition = nullptr;
const ValueFlow::Value *index = nullptr;
for (const ValueFlow::Value *indexValue: indexes) {
if (!indexValue)
continue;
if (!indexValue->errorSeverity() && !mSettings->isEnabled(Settings::WARNING))
return;
if (indexValue->condition)
condition = indexValue->condition;
if (!index || !indexValue->errorPath.empty())
index = indexValue;
}
reportError(getErrorPath(tok, index, "Array index out of bounds"),
index->errorSeverity() ? Severity::error : Severity::warning,
index->condition ? "arrayIndexOutOfBoundsCond" : "arrayIndexOutOfBounds",
arrayIndexMessage(tok, dimensions, index),
arrayIndexMessage(tok, dimensions, indexes, condition),
CWE788,
index->isInconclusive());
}
void CheckBufferOverrun::negativeIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const ValueFlow::Value *negativeValue)
void CheckBufferOverrun::negativeIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const std::vector<const ValueFlow::Value *> &indexes)
{
if (!negativeValue) {
if (!tok) {
reportError(tok, Severity::error, "negativeIndex", "Negative array index", CWE786, false);
return;
}
if (!negativeValue->errorSeverity() && !mSettings->isEnabled(Settings::WARNING))
return;
const Token *condition = nullptr;
const ValueFlow::Value *negativeValue = nullptr;
for (const ValueFlow::Value *indexValue: indexes) {
if (!indexValue)
continue;
if (!indexValue->errorSeverity() && !mSettings->isEnabled(Settings::WARNING))
return;
if (indexValue->condition)
condition = indexValue->condition;
if (!negativeValue || !indexValue->errorPath.empty())
negativeValue = indexValue;
}
reportError(getErrorPath(tok, negativeValue, "Negative array index"),
negativeValue->errorSeverity() ? Severity::error : Severity::warning,
"negativeIndex",
arrayIndexMessage(tok, dimensions, negativeValue),
arrayIndexMessage(tok, dimensions, indexes, condition),
CWE786,
negativeValue->isInconclusive());
}

8
lib/checkbufferoverrun.h
View File

@ -70,8 +70,8 @@ public:
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE {
CheckBufferOverrun c(nullptr, settings, errorLogger);
c.arrayIndexError(nullptr, std::vector<Dimension>(), nullptr);
c.negativeIndexError(nullptr, std::vector<Dimension>(), nullptr);
c.arrayIndexError(nullptr, std::vector<Dimension>(), std::vector<const ValueFlow::Value *>());
c.negativeIndexError(nullptr, std::vector<Dimension>(), std::vector<const ValueFlow::Value *>());
c.arrayIndexThenCheckError(nullptr, "i");
c.bufferOverflowError(nullptr, nullptr);
}
@ -79,8 +79,8 @@ public:
private:
void arrayIndex();
void arrayIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const ValueFlow::Value *index);
void negativeIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const ValueFlow::Value *negativeValue);
void arrayIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const std::vector<const ValueFlow::Value *> &indexes);
void negativeIndexError(const Token *tok, const std::vector<Dimension> &dimensions, const std::vector<const ValueFlow::Value *> &indexes);
void bufferOverflow();
void bufferOverflowError(const Token *tok, const ValueFlow::Value *value);

53
test/testbufferoverrun.cpp
View File

@ -113,7 +113,7 @@ private:
TEST_CASE(array_index_31); // ticket #2120 - out of bounds in subfunction when type is unknown
TEST_CASE(array_index_32);
TEST_CASE(array_index_33); // ticket #3044
// TODO multidim TEST_CASE(array_index_34); // ticket #3063
TEST_CASE(array_index_34); // ticket #3063
TEST_CASE(array_index_35); // ticket #2889
TEST_CASE(array_index_36); // ticket #2960
TEST_CASE(array_index_37);
@ -127,7 +127,7 @@ private:
TEST_CASE(array_index_45); // #4207 - calling function with variable number of parameters (...)
TEST_CASE(array_index_46); // #4840 - two-statement for loop
TEST_CASE(array_index_47); // #5849
// TODO multidim TEST_CASE(array_index_multidim);
TEST_CASE(array_index_multidim);
TEST_CASE(array_index_switch_in_for);
TEST_CASE(array_index_for_in_for); // FP: #2634
TEST_CASE(array_index_calculation);
@ -571,7 +571,7 @@ private:
" struct ABC x;\n"
" x.str[1] = 0;"
"}");
TODO_ASSERT_EQUALS("[test.cpp:9]: (error) Array 'x.str[1]' accessed at index 1, which is out of bounds.\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:9]: (error) Array 'x.str[1]' accessed at index 1, which is out of bounds.\n", errout.str());
check("struct foo\n"
"{\n"
@ -596,7 +596,7 @@ private:
"void f() {\n"
" ab.a[2] = 0;\n"
"}");
ASSERT_EQUALS("", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'ab.a[1]' accessed at index 2, which is out of bounds.\n", errout.str());
}
@ -1134,8 +1134,8 @@ private:
" y[0][2][0] = 0;\n"
" y[0][0][2] = 0;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'y[2][2][2]' index y[0][2][0] out of bounds.\n"
"[test.cpp:4]: (error) Array 'y[2][2][2]' index y[0][0][2] out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'y[2][2][2]' accessed at index y[0][2][0], which is out of bounds.\n"
"[test.cpp:4]: (error) Array 'y[2][2][2]' accessed at index y[0][0][2], which is out of bounds.\n", errout.str());
check("struct TEST\n"
"{\n"
@ -1155,14 +1155,11 @@ private:
" ptest->b[0][19] = 4;\n"
"}");
ASSERT_EQUALS("[test.cpp:9]: (error) Array 'test.a[10]' accessed at index 10, which is out of bounds.\n"
"[test.cpp:10]: (error) Array 'test.b[10][5]' index test.b[10][2] out of bounds.\n"
"[test.cpp:11]: (error) Array 'test.b[10][5]' index test.b[0][19] out of bounds.\n"
"[test.cpp:10]: (error) Array 'test.b[10][5]' accessed at index test.b[10][2], which is out of bounds.\n"
"[test.cpp:11]: (error) Array 'test.b[10][5]' accessed at index test.b[0][19], which is out of bounds.\n"
"[test.cpp:14]: (error) Array 'ptest->a[10]' accessed at index 10, which is out of bounds.\n"
"[test.cpp:15]: (error) Array 'ptest->b[10][5]' index ptest->b[10][2] out of bounds.\n"
"[test.cpp:16]: (error) Array 'ptest->b[10][5]' index ptest->b[0][19] out of bounds.\n"
"[test.cpp:14]: (error) Array 'ptest.a[10]' accessed at index 10, which is out of bounds.\n"
"[test.cpp:15]: (error) Array 'ptest.b[10][5]' index ptest.b[10][2] out of bounds.\n"
"[test.cpp:16]: (error) Array 'ptest.b[10][5]' index ptest.b[0][19] out of bounds.\n", errout.str());
"[test.cpp:15]: (error) Array 'ptest->b[10][5]' accessed at index ptest->b[10][2], which is out of bounds.\n"
"[test.cpp:16]: (error) Array 'ptest->b[10][5]' accessed at index ptest->b[0][19], which is out of bounds.\n", errout.str());
check("struct TEST\n"
"{\n"
@ -1178,12 +1175,10 @@ private:
" ptest->a[9][5] = 4;\n"
" ptest->a[0][50] = 4;\n"
"}");
ASSERT_EQUALS("[test.cpp:8]: (error) Array 'test.a[10][5]' index test.a[9][5] out of bounds.\n"
"[test.cpp:9]: (error) Array 'test.a[10][5]' index test.a[0][50] out of bounds.\n"
"[test.cpp:12]: (error) Array 'ptest->a[10][5]' index ptest->a[9][5] out of bounds.\n"
"[test.cpp:13]: (error) Array 'ptest->a[10][5]' index ptest->a[0][50] out of bounds.\n"
"[test.cpp:12]: (error) Array 'ptest.a[10][5]' index ptest.a[9][5] out of bounds.\n"
"[test.cpp:13]: (error) Array 'ptest.a[10][5]' index ptest.a[0][50] out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:8]: (error) Array 'test.a[10][5]' accessed at index test.a[9][5], which is out of bounds.\n"
"[test.cpp:9]: (error) Array 'test.a[10][5]' accessed at index test.a[0][50], which is out of bounds.\n"
"[test.cpp:12]: (error) Array 'ptest->a[10][5]' accessed at index ptest->a[9][5], which is out of bounds.\n"
"[test.cpp:13]: (error) Array 'ptest->a[10][5]' accessed at index ptest->a[0][50], which is out of bounds.\n", errout.str());
}
void array_index_35() { // ticket #2889
@ -1550,67 +1545,67 @@ private:
" char a[2][2];\n"
" a[2][1] = 'a';\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2]' index a[2][1] out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2]' accessed at index a[2][1], which is out of bounds.\n", errout.str());
check("void f()\n"
"{\n"
" char a[2][2];\n"
" a[1][2] = 'a';\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2]' index a[1][2] out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2]' accessed at index a[1][2], which is out of bounds.\n", errout.str());
check("void f()\n"
"{\n"
" char a[2][2][2];\n"
" a[2][1][1] = 'a';\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' index a[2][1][1] out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' accessed at index a[2][1][1], which is out of bounds.\n", errout.str());
check("void f()\n"
"{\n"
" char a[2][2][2];\n"
" a[1][2][1] = 'a';\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' index a[1][2][1] out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' accessed at index a[1][2][1], which is out of bounds.\n", errout.str());
check("void f()\n"
"{\n"
" char a[2][2][2][2];\n"
" a[1][2][1][1] = 'a';\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2][2]' index a[1][2][1][1] out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2][2]' accessed at index a[1][2][1][1], which is out of bounds.\n", errout.str());
check("void f()\n"
"{\n"
" char a[2][2][2];\n"
" a[1][1][2] = 'a';\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' index a[1][1][2] out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' accessed at index a[1][1][2], which is out of bounds.\n", errout.str());
check("void f()\n"
"{\n"
" char a[10][10][10];\n"
" a[2*3][4*3][2] = 'a';\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10][10][10]' index a[6][12][2] out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10][10][10]' accessed at index a[6][12][2], which is out of bounds.\n", errout.str());
check("void f() {\n"
" char a[10][10][10];\n"
" a[6][40][10] = 'a';\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[10][10][10]' index a[6][40][10] out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[10][10][10]' accessed at index a[6][40][10], which is out of bounds.\n", errout.str());
check("void f() {\n"
" char a[1][1][1];\n"
" a[2][2][2] = 'a';\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[1][1][1]' index a[2][2][2] out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[1][1][1]' accessed at index a[2][2][2], which is out of bounds.\n", errout.str());
check("void f() {\n"
" char a[6][6][6];\n"
" a[6][6][2] = 'a';\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[6][6][6]' index a[6][6][2] out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[6][6][6]' accessed at index a[6][6][2], which is out of bounds.\n", errout.str());
check("void f() {\n"
" int a[2][2];\n"