Fixed #1190 (array index out of bounds when index variable is assigned in a condition)

This commit is contained in:
Daniel Marjamäki 2010-04-10 21:12:00 +02:00
parent e17cce6ac4
commit 5fed938f56
4 changed files with 213 additions and 3 deletions

View File

@ -92,7 +92,7 @@ install: cppcheck
lib/checkautovariables.o: lib/checkautovariables.cpp lib/checkautovariables.h lib/check.h lib/token.h lib/tokenize.h lib/classinfo.h lib/settings.h lib/errorlogger.h
$(CXX) $(CXXFLAGS) -Ilib -c -o lib/checkautovariables.o lib/checkautovariables.cpp
lib/checkbufferoverrun.o: lib/checkbufferoverrun.cpp lib/checkbufferoverrun.h lib/check.h lib/token.h lib/tokenize.h lib/classinfo.h lib/settings.h lib/errorlogger.h lib/mathlib.h
lib/checkbufferoverrun.o: lib/checkbufferoverrun.cpp lib/checkbufferoverrun.h lib/check.h lib/token.h lib/tokenize.h lib/classinfo.h lib/settings.h lib/errorlogger.h lib/mathlib.h lib/executionpath.h
$(CXX) $(CXXFLAGS) -Ilib -c -o lib/checkbufferoverrun.o lib/checkbufferoverrun.cpp
lib/checkclass.o: lib/checkclass.cpp lib/checkclass.h lib/check.h lib/token.h lib/tokenize.h lib/classinfo.h lib/settings.h lib/errorlogger.h

View File

@ -1260,3 +1260,174 @@ void CheckBufferOverrun::checkSprintfCall(const Token *tok, int size)
}
}
#include "executionpath.h"
/// @addtogroup Checks
/// @{
class ArrayInfo
{
public:
/** type size in bytes */
unsigned int type_size;
/** number of elements of array */
unsigned int num;
};
/**
* @brief %Check for buffer overruns (using ExecutionPath)
*/
class ExecutionPathBufferOverrun : public ExecutionPath
{
public:
/** Startup constructor */
ExecutionPathBufferOverrun(Check *c, const std::map<unsigned int, ArrayInfo> &arrayinfo)
: ExecutionPath(c, 0), arrayInfo(arrayinfo)
{
}
private:
/** Copy this check */
ExecutionPath *copy()
{
return new ExecutionPathBufferOverrun(*this);
}
/** @brief buffer information */
const std::map<unsigned int, ArrayInfo> &arrayInfo;
/** no implementation => compiler error if used by accident */
void operator=(const ExecutionPathBufferOverrun &);
/** internal constructor for creating extra checks */
ExecutionPathBufferOverrun(Check *c, const std::map<unsigned int, ArrayInfo> &arrayinfo, unsigned int varid_)
: ExecutionPath(c, varid_),
arrayInfo(arrayinfo)
{
// Pretend that variables are initialized to 0
// This checking is not about uninitialized variables
value = 0;
}
unsigned int value;
/** @brief Assign value to a variable */
static void assign_value(std::list<ExecutionPath *> &checks, unsigned int varid, const std::string &value)
{
if (varid == 0)
return;
std::list<ExecutionPath *>::const_iterator it;
for (it = checks.begin(); it != checks.end(); ++it)
{
ExecutionPathBufferOverrun *c = dynamic_cast<ExecutionPathBufferOverrun *>(*it);
if (c && c->varId == varid)
c->value = MathLib::toLongNumber(value);
}
}
/** @brief Found array usage.. */
static void array_index(const Token *tok, std::list<ExecutionPath *> &checks, unsigned int varid1, unsigned int varid2)
{
if (varid1 == 0 || varid2 == 0)
return;
// Locate array info corresponding to varid1
ArrayInfo ai;
{
ExecutionPathBufferOverrun *c = dynamic_cast<ExecutionPathBufferOverrun *>(checks.front());
std::map<unsigned int, ArrayInfo>::const_iterator it;
it = c->arrayInfo.find(varid1);
if (it == c->arrayInfo.end())
return;
ai = it->second;
}
// Check if varid2 variable has a value that is out-of-bounds
std::list<ExecutionPath *>::const_iterator it;
for (it = checks.begin(); it != checks.end(); ++it)
{
ExecutionPathBufferOverrun *c = dynamic_cast<ExecutionPathBufferOverrun *>(*it);
if (c && c->varId == varid2 && c->value >= ai.num)
{
// variable value is out of bounds, report error
CheckBufferOverrun *checkBufferOverrun = dynamic_cast<CheckBufferOverrun *>(c->owner);
if (checkBufferOverrun)
{
checkBufferOverrun->arrayIndexOutOfBounds(tok, ai.num, c->value);
break;
}
}
}
}
const Token *parse(const Token &tok, std::list<ExecutionPath *> &checks) const
{
if (Token::Match(tok.previous(), "[;{}]"))
{
// Declaring variable..
if (Token::Match(&tok, "%type% %var% ;") && tok.isStandardType())
{
checks.push_back(new ExecutionPathBufferOverrun(owner, arrayInfo, tok.next()->varId()));
return tok.tokAt(2);
}
// Assign variable..
if (Token::Match(&tok, "%var% = %num% ;"))
{
assign_value(checks, tok.varId(), tok.strAt(2));
return tok.tokAt(3);
}
}
// Assign variable (unknown value = 0)..
if (Token::Match(&tok, "%var% ="))
{
assign_value(checks, tok.varId(), "0");
return &tok;
}
// Array index..
if (Token::Match(&tok, "%var% [ %var% ]"))
{
array_index(&tok, checks, tok.varId(), tok.tokAt(2)->varId());
return tok.tokAt(3);
}
return &tok;
}
};
/// @}
void CheckBufferOverrun::executionPaths()
{
// Parse all tokens and extract array info..
std::map<unsigned int, ArrayInfo> arrayInfo;
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
{
if (Token::Match(tok, "[;{}] %type% %var% [ %num% ] ;"))
{
const unsigned int varid(tok->tokAt(2)->varId());
ArrayInfo ai;
ai.type_size = _tokenizer->sizeOfType(tok->next());
ai.num = MathLib::toLongNumber(tok->strAt(4));
arrayInfo[varid] = ai;
}
}
// Perform checking - check how the arrayInfo arrays are used
ExecutionPathBufferOverrun c(this, arrayInfo);
checkExecutionPaths(_tokenizer->tokens(), &c);
}

View File

@ -61,11 +61,17 @@ public:
{
CheckBufferOverrun checkBufferOverrun(tokenizer, settings, errorLogger);
checkBufferOverrun.bufferOverrun();
/** ExecutionPath checking.. */
checkBufferOverrun.executionPaths();
}
/** @brief %Check for buffer overruns */
void bufferOverrun();
/** @brief %Check for buffer overruns by inspecting execution paths */
void executionPaths();
/**
* @brief Get minimum length of format string result
* @param input_string format string
@ -74,8 +80,6 @@ public:
*/
static int countSprintfLength(const std::string &input_string, const std::list<const Token*> &parameters);
private:
/**
* @brief %Check code that matches: "sprintf ( %varid% , %str% [,)]" when varid is not 0,
* and report found errors.

View File

@ -151,6 +151,8 @@ private:
TEST_CASE(terminateStrncpy1);
TEST_CASE(terminateStrncpy2);
TEST_CASE(recursive_long_time);
TEST_CASE(executionPaths1);
}
@ -1918,6 +1920,39 @@ private:
"}\n");
ASSERT_EQUALS("", errout.str());
}
void epcheck(const char code[])
{
// Tokenize..
Tokenizer tokenizer;
std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp");
tokenizer.simplifyTokenList();
// Clear the error buffer..
errout.str("");
// Check for buffer overruns..
Settings settings;
CheckBufferOverrun checkBufferOverrun(&tokenizer, &settings, this);
checkBufferOverrun.executionPaths();
}
void executionPaths1()
{
epcheck("void f(int a)\n"
"{\n"
" int buf[10];\n"
" int i = 5;\n"
" if (a == 1)\n"
" i = 1000;\n"
" buf[i] = 0;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:7]: (error) Array 'buf[10]' index 1000 out of bounds\n", errout.str());
}
};
REGISTER_TEST(TestBufferOverrun)