Buffer overruns: Use ValueFlow to detect negative index

This commit is contained in:
Daniel Marjamäki 2014-03-25 18:22:22 +01:00
parent 533a62a4c7
commit c8004a8d31
4 changed files with 56 additions and 18 deletions

View File

@ -1183,6 +1183,23 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
else if (Token::Match(tok, "%varid% [", arrayInfo.declarationId())) {
// Look for errors first
for (int warn = 0; warn == 0 || warn == 1; ++warn) {
// Negative index..
for (const Token *tok2 = tok->next(); tok2 && tok2->str() == "["; tok2 = tok2->link()->next()) {
const Token *index = tok2->astOperand2();
std::list<ValueFlow::Value>::const_iterator it;
const ValueFlow::Value *val = nullptr;
for (it = index->values.begin(); it != index->values.end(); ++it) {
if (it->intvalue < 0) {
val = &*it;
if (val->condition == nullptr)
break;
}
}
if (val && !val->condition)
negativeIndexError(index, val->intvalue);
}
// Index out of bounds..
std::vector<ValueFlow::Value> indexes;
unsigned int valuevarid = 0;
for (const Token *tok2 = tok->next(); indexes.size() < arrayInfo.num().size() && Token::Match(tok2, "["); tok2 = tok2->link()->next()) {

View File

@ -663,6 +663,11 @@ static void execute(const Token *expr,
if (var == programMemory->end())
*error = true;
else {
if (var->second == 0 &&
expr->str() == "--" &&
expr->astOperand1()->variable() &&
expr->astOperand1()->variable()->typeStartToken()->isUnsigned())
*error = true; // overflow
*result = var->second + (expr->str() == "++" ? 1 : -1);
var->second = *result;
}
@ -800,8 +805,13 @@ static void valueFlowForLoopSimplify(Token * const bodyStart, const unsigned int
while (parent) {
const Token * const p = parent;
parent = parent->astParent();
if (parent && parent->str() == "?" && parent->astOperand2() == p)
if (parent && parent->str() == ":")
break;
if (parent && parent->str() == "?") {
if (parent->astOperand2() != p)
parent = NULL;
break;
}
}
if (parent) {
if (settings->debugwarnings)

View File

@ -969,48 +969,48 @@ private:
" a[-1] = 0;\n" // negative index
" a[" + charMaxPlusOne.str() + "] = 0;\n" // 128/256 > CHAR_MAX
"}\n").c_str());
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a["+charMaxPlusOne.str()+"]' accessed at index "+charMaxPlusOne.str()+", which is out of bounds.\n"
"[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n"
"[test.cpp:4]: (error) Array 'a["+charMaxPlusOne.str()+"]' accessed at index "+charMaxPlusOne.str()+", which is out of bounds.\n", errout.str());
check("void f(signed char n) {\n"
" int a[n];\n" // n <= SCHAR_MAX
" a[-1] = 0;\n" // negative index
" a[128] = 0;\n" // 128 > SCHAR_MAX
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[128]' accessed at index 128, which is out of bounds.\n"
"[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n"
"[test.cpp:4]: (error) Array 'a[128]' accessed at index 128, which is out of bounds.\n", errout.str());
check("void f(unsigned char n) {\n"
" int a[n];\n" // n <= UCHAR_MAX
" a[-1] = 0;\n" // negative index
" a[256] = 0;\n" // 256 > UCHAR_MAX
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[256]' accessed at index 256, which is out of bounds.\n"
"[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n"
"[test.cpp:4]: (error) Array 'a[256]' accessed at index 256, which is out of bounds.\n", errout.str());
check("void f(short n) {\n"
" int a[n];\n" // n <= SHRT_MAX
" a[-1] = 0;\n" // negative index
" a[32768] = 0;\n" // 32768 > SHRT_MAX
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[32768]' accessed at index 32768, which is out of bounds.\n"
"[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n"
"[test.cpp:4]: (error) Array 'a[32768]' accessed at index 32768, which is out of bounds.\n", errout.str());
check("void f(unsigned short n) {\n"
" int a[n];\n" // n <= USHRT_MAX
" a[-1] = 0;\n" // negative index
" a[65536] = 0;\n" // 65536 > USHRT_MAX
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[65536]' accessed at index 65536, which is out of bounds.\n"
"[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n"
"[test.cpp:4]: (error) Array 'a[65536]' accessed at index 65536, which is out of bounds.\n", errout.str());
check("void f(signed short n) {\n"
" int a[n];\n" // n <= SHRT_MAX
" a[-1] = 0;\n" // negative index
" a[32768] = 0;\n" // 32768 > SHRT_MAX
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[32768]' accessed at index 32768, which is out of bounds.\n"
"[test.cpp:3]: (error) Array index -1 is out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds.\n"
"[test.cpp:4]: (error) Array 'a[32768]' accessed at index 32768, which is out of bounds.\n", errout.str());
check("void f(int n) {\n"
" int a[n];\n" // n <= INT_MAX
@ -1066,7 +1066,8 @@ private:
" for (int i = 0; i < 10; i++)\n"
" a[i-1] = a[i];\n"
"}");
ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n"
"[test.cpp:5]: (error) Array index -1 is out of bounds.\n", errout.str());
}
void array_index_28() {
@ -1789,7 +1790,8 @@ private:
" val[i+1] = val[i];\n"
" }\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:5]: (error) Array 'val[5]' index -1 out of bounds.\n", "", errout.str());
ASSERT_EQUALS("[test.cpp:5]: (error) Array index -9994 is out of bounds.\n"
"[test.cpp:5]: (error) Array index -9995 is out of bounds.\n", errout.str());
}
@ -1872,7 +1874,7 @@ private:
" a[i - 1] = 0;\n"
" }\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:7]: (error) Array 'a[2]' accessed at index -1, which is out of bounds", "", errout.str());
ASSERT_EQUALS("[test.cpp:7]: (error) Array index -1 is out of bounds.\n", errout.str());
}
void array_index_for() {
@ -1939,7 +1941,8 @@ private:
" some_condition ? 0 : a[i-1];\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n"
"[test.cpp:4]: (error) Array index -1 is out of bounds.\n", errout.str());
check("void f() {\n"
" int a[10];\n"
@ -1948,7 +1951,8 @@ private:
" a[i-1] = 0;\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n", errout.str());
ASSERT_EQUALS("[test.cpp:5]: (error) Array 'a[10]' accessed at index -1, which is out of bounds.\n"
"[test.cpp:5]: (error) Array index -1 is out of bounds.\n", errout.str());
}
void array_index_for_varid0() { // #4228: No varid for counter variable

View File

@ -655,6 +655,13 @@ private:
ASSERT_EQUALS(true, testValueOfX(code, 3U, 0));
ASSERT_EQUALS(true, testValueOfX(code, 3U, 9));
ASSERT_EQUALS(false, testValueOfX(code, 4U, 9));
code = "void f() {\n"
" for (int x = 0; x < 10; x++)\n"
" x==0 ?\n"
" 0 : a[x];\n"
"}";
ASSERT_EQUALS(false, testValueOfX(code, 4U, 0));
}
void valueFlowSubFunction() {