partial fix for #2960 (false negative: buffer access out of bounds)
This commit is contained in:
Robert Reif
parent
40009d091d
commit
3f517b5f23
|
|
@ -1511,7 +1511,7 @@ void CheckBufferOverrun::checkStructVariable()
|
||||||
if (func_scope->type != Scope::eFunction)
|
if (func_scope->type != Scope::eFunction)
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
// is this a member function of this class/struct?
|
// check for member variables
|
||||||
if (func_scope->functionOf == &*scope)
|
if (func_scope->functionOf == &*scope)
|
||||||
{
|
{
|
||||||
// only check non-empty function
|
// only check non-empty function
|
||||||
|
|
@ -1523,129 +1523,125 @@ void CheckBufferOverrun::checkStructVariable()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// not a member function of this class/struct
|
// skip inner scopes..
|
||||||
else
|
/** @todo false negatives: handle inner scopes someday */
|
||||||
|
if (scope->nestedIn->isClassOrStruct())
|
||||||
|
continue;
|
||||||
|
|
||||||
|
std::vector<std::string> varname;
|
||||||
|
varname.push_back("");
|
||||||
|
varname.push_back(arrayInfo.varname());
|
||||||
|
|
||||||
|
// search the function and it's parameters
|
||||||
|
for (const Token *tok3 = func_scope->classDef; tok3 && tok3 != func_scope->classEnd; tok3 = tok3->next())
|
||||||
{
|
{
|
||||||
// skip inner scopes..
|
// search for the class/struct name
|
||||||
/** @todo false negatives: handle inner scopes someday */
|
if (tok3->str() != scope->className)
|
||||||
if (scope->nestedIn->isClassOrStruct())
|
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
std::vector<std::string> varname;
|
// Declare variable: Fred fred1;
|
||||||
varname.push_back("");
|
if (Token::Match(tok3->next(), "%var% ;"))
|
||||||
varname.push_back(arrayInfo.varname());
|
varname[0] = tok3->strAt(1);
|
||||||
|
|
||||||
// search the function and it's parameters
|
// Declare pointer or reference: Fred *fred1
|
||||||
for (const Token *tok3 = func_scope->classDef; tok3 && tok3 != func_scope->classEnd; tok3 = tok3->next())
|
else if (Token::Match(tok3->next(), "*|& %var% [,);=]"))
|
||||||
|
varname[0] = tok3->strAt(2);
|
||||||
|
|
||||||
|
else
|
||||||
|
continue;
|
||||||
|
|
||||||
|
// check for variable sized structure
|
||||||
|
if (scope->type == Scope::eStruct && var->isPublic())
|
||||||
{
|
{
|
||||||
// search for the class/struct name
|
// last member of a struct with array size of 0 or 1 could be a variable sized structure
|
||||||
if (tok3->str() != scope->className)
|
if (var->dimensions().size() == 1 && var->dimension(0) < 2 &&
|
||||||
continue;
|
var->index() == (scope->varlist.size() - 1))
|
||||||
|
|
||||||
// Declare variable: Fred fred1;
|
|
||||||
if (Token::Match(tok3->next(), "%var% ;"))
|
|
||||||
varname[0] = tok3->strAt(1);
|
|
||||||
|
|
||||||
// Declare pointer or reference: Fred *fred1
|
|
||||||
else if (Token::Match(tok3->next(), "*|& %var% [,);=]"))
|
|
||||||
varname[0] = tok3->strAt(2);
|
|
||||||
|
|
||||||
else
|
|
||||||
continue;
|
|
||||||
|
|
||||||
// check for variable sized structure
|
|
||||||
if (scope->type == Scope::eStruct && var->isPublic())
|
|
||||||
{
|
{
|
||||||
// last member of a struct with array size of 0 or 1 could be a variable sized structure
|
// dynamically allocated so could be variable sized structure
|
||||||
if (var->dimensions().size() == 1 && var->dimension(0) < 2 &&
|
if (tok3->next()->str() == "*")
|
||||||
var->index() == (scope->varlist.size() - 1))
|
|
||||||
{
|
{
|
||||||
// dynamically allocated so could be variable sized structure
|
// check for allocation
|
||||||
if (tok3->next()->str() == "*")
|
if ((Token::Match(tok3->tokAt(3), "; %var% = malloc ( %num% ) ;") ||
|
||||||
|
(Token::Match(tok3->tokAt(3), "; %var% = (") &&
|
||||||
|
Token::Match(tok3->tokAt(6)->link(), ") malloc ( %num% ) ;"))) &&
|
||||||
|
(tok3->strAt(4) == tok3->strAt(2)))
|
||||||
{
|
{
|
||||||
// check for allocation
|
MathLib::bigint size;
|
||||||
if ((Token::Match(tok3->tokAt(3), "; %var% = malloc ( %num% ) ;") ||
|
|
||||||
(Token::Match(tok3->tokAt(3), "; %var% = (") &&
|
|
||||||
Token::Match(tok3->tokAt(6)->link(), ") malloc ( %num% ) ;"))) &&
|
|
||||||
(tok3->strAt(4) == tok3->strAt(2)))
|
|
||||||
{
|
|
||||||
MathLib::bigint size;
|
|
||||||
|
|
||||||
// find size of allocation
|
// find size of allocation
|
||||||
if (tok3->strAt(3) == "(") // has cast
|
if (tok3->strAt(3) == "(") // has cast
|
||||||
size = MathLib::toLongNumber(tok3->tokAt(6)->link()->strAt(3));
|
size = MathLib::toLongNumber(tok3->tokAt(6)->link()->strAt(3));
|
||||||
else
|
|
||||||
size = MathLib::toLongNumber(tok3->strAt(8));
|
|
||||||
|
|
||||||
// We don't calculate the size of a structure even when we know
|
|
||||||
// the size of the members. We just assign a length of 100 for
|
|
||||||
// any struct. If the size is less than 100, we assume the
|
|
||||||
// programmer knew the size and specified it rather than using
|
|
||||||
// sizeof(struct). If the size is greater than 100, we assume
|
|
||||||
// the programmer specified the size as sizeof(struct) + number.
|
|
||||||
// Either way, this is just a guess and could be wrong. The
|
|
||||||
// information to make the right decision has been simplified
|
|
||||||
// away by the time we get here.
|
|
||||||
if (size != 100) // magic number for size of struct
|
|
||||||
{
|
|
||||||
// check if a real size was specified and give up
|
|
||||||
// malloc(10) rather than malloc(sizeof(struct))
|
|
||||||
if (size < 100)
|
|
||||||
continue;
|
|
||||||
|
|
||||||
// calculate real array size based on allocated size
|
|
||||||
MathLib::bigint elements = (size - 100) / arrayInfo.element_size();
|
|
||||||
arrayInfo.num(0, arrayInfo.num(0) + elements);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// size unknown so assume it is a variable sized structure
|
|
||||||
else
|
else
|
||||||
continue;
|
size = MathLib::toLongNumber(tok3->strAt(8));
|
||||||
|
|
||||||
|
// We don't calculate the size of a structure even when we know
|
||||||
|
// the size of the members. We just assign a length of 100 for
|
||||||
|
// any struct. If the size is less than 100, we assume the
|
||||||
|
// programmer knew the size and specified it rather than using
|
||||||
|
// sizeof(struct). If the size is greater than 100, we assume
|
||||||
|
// the programmer specified the size as sizeof(struct) + number.
|
||||||
|
// Either way, this is just a guess and could be wrong. The
|
||||||
|
// information to make the right decision has been simplified
|
||||||
|
// away by the time we get here.
|
||||||
|
if (size != 100) // magic number for size of struct
|
||||||
|
{
|
||||||
|
// check if a real size was specified and give up
|
||||||
|
// malloc(10) rather than malloc(sizeof(struct))
|
||||||
|
if (size < 100)
|
||||||
|
continue;
|
||||||
|
|
||||||
|
// calculate real array size based on allocated size
|
||||||
|
MathLib::bigint elements = (size - 100) / arrayInfo.element_size();
|
||||||
|
arrayInfo.num(0, arrayInfo.num(0) + elements);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// size unknown so assume it is a variable sized structure
|
||||||
|
else
|
||||||
|
continue;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Goto end of statement.
|
// Goto end of statement.
|
||||||
const Token *CheckTok = NULL;
|
const Token *CheckTok = NULL;
|
||||||
while (tok3 && tok3 != func_scope->classEnd)
|
while (tok3 && tok3 != func_scope->classEnd)
|
||||||
|
{
|
||||||
|
// End of statement.
|
||||||
|
if (tok3->str() == ";")
|
||||||
{
|
{
|
||||||
// End of statement.
|
CheckTok = tok3;
|
||||||
if (tok3->str() == ";")
|
break;
|
||||||
{
|
|
||||||
CheckTok = tok3;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
// End of function declaration..
|
|
||||||
if (Token::simpleMatch(tok3, ") ;"))
|
|
||||||
break;
|
|
||||||
|
|
||||||
// Function implementation..
|
|
||||||
if (Token::simpleMatch(tok3, ") {"))
|
|
||||||
{
|
|
||||||
CheckTok = tok3->tokAt(2);
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
|
|
||||||
tok3 = tok3->next();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!tok3)
|
// End of function declaration..
|
||||||
|
if (Token::simpleMatch(tok3, ") ;"))
|
||||||
break;
|
break;
|
||||||
|
|
||||||
if (!CheckTok)
|
// Function implementation..
|
||||||
continue;
|
if (Token::simpleMatch(tok3, ") {"))
|
||||||
|
{
|
||||||
|
CheckTok = tok3->tokAt(2);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
|
||||||
// Check variable usage..
|
tok3 = tok3->next();
|
||||||
ArrayInfo temp = arrayInfo;
|
|
||||||
temp.varid(0); // do variable lookup by variable and member names rather than varid
|
|
||||||
std::string varnames; // use class and member name for messages
|
|
||||||
for (unsigned int i = 0; i < varname.size(); ++i)
|
|
||||||
varnames += (i == 0 ? "" : ".") + varname[i];
|
|
||||||
temp.varname(varnames);
|
|
||||||
checkScope(CheckTok, varname, temp);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!tok3)
|
||||||
|
break;
|
||||||
|
|
||||||
|
if (!CheckTok)
|
||||||
|
continue;
|
||||||
|
|
||||||
|
// Check variable usage..
|
||||||
|
ArrayInfo temp = arrayInfo;
|
||||||
|
temp.varid(0); // do variable lookup by variable and member names rather than varid
|
||||||
|
std::string varnames; // use class and member name for messages
|
||||||
|
for (unsigned int i = 0; i < varname.size(); ++i)
|
||||||
|
varnames += (i == 0 ? "" : ".") + varname[i];
|
||||||
|
temp.varname(varnames);
|
||||||
|
checkScope(CheckTok, varname, temp);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -112,6 +112,7 @@ private:
|
||||||
TEST_CASE(array_index_33); // ticket #3044
|
TEST_CASE(array_index_33); // ticket #3044
|
||||||
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_35); // ticket #2889
|
||||||
|
TEST_CASE(array_index_36); // ticket #2960
|
||||||
TEST_CASE(array_index_multidim);
|
TEST_CASE(array_index_multidim);
|
||||||
TEST_CASE(array_index_switch_in_for);
|
TEST_CASE(array_index_switch_in_for);
|
||||||
TEST_CASE(array_index_for_in_for); // FP: #2634
|
TEST_CASE(array_index_for_in_for); // FP: #2634
|
||||||
|
|
@ -1281,6 +1282,20 @@ private:
|
||||||
ASSERT_EQUALS("", errout.str());
|
ASSERT_EQUALS("", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void array_index_36() // ticket #2960
|
||||||
|
{
|
||||||
|
check("class Fred {\n"
|
||||||
|
" Fred(const Fred &);\n"
|
||||||
|
"private:\n"
|
||||||
|
" bool m_b[2];\n"
|
||||||
|
"};\n"
|
||||||
|
"Fred::Fred(const Fred & rhs) {\n"
|
||||||
|
" m_b[2] = rhs.m_b[2];\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("[test.cpp:7]: (error) Array 'm_b[2]' index 2 out of bounds\n"
|
||||||
|
"[test.cpp:7]: (error) Array 'rhs.m_b[2]' index 2 out of bounds\n", errout.str());
|
||||||
|
}
|
||||||
|
|
||||||
void array_index_multidim()
|
void array_index_multidim()
|
||||||
{
|
{
|
||||||
check("void f()\n"
|
check("void f()\n"
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue