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

Refactoring: Moved some code into a new function. 

Renamed count->countSprintfLength.
Added code to collect sprintf parameters.
Added a few TODO test cases.
This commit is contained in:
Reijo Tomperi 2009-10-07 15:37:20 +03:00
parent 8fc21dda27
commit fcd269dbf7
3 changed files with 180 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

211
src/checkbufferoverrun.cpp
View File

@ -465,78 +465,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con
// sprintf.. // sprintf..
if (varid > 0 && Token::Match(tok, "sprintf ( %varid% , %str% [,)]", varid)) if (varid > 0 && Token::Match(tok, "sprintf ( %varid% , %str% [,)]", varid))
{ {
int len = -2; checkSprintfCall(tok, size);
// check format string
const char *fmt = tok->strAt(4);
while (*fmt)
{
if (*fmt == '\\')
{
++fmt;
}
else if (*fmt == '%')
{
++fmt;
// skip field width
while (std::isdigit(*fmt))
{
++fmt;
}
// FIXME: better handling for format specifiers
++fmt;
continue;
}
++fmt;
++len;
}
if (len >= (int)size)
{
bufferOverrun(tok);
}
// check arguments (if they exists)
if (tok->tokAt(5)->str() == ",")
{
len = 0;
const Token *end = tok->next()->link();
bool argumentAlreadyChecked = false;
int lastCheckedArgumentMaxSize = 0;
for (const Token *tok2 = tok->tokAt(6); tok2 && tok2 != end; tok2 = tok2->next())
{
if (tok2->str() == ",")
{
argumentAlreadyChecked = false;
lastCheckedArgumentMaxSize = 0;
}
else if (Token::Match(tok2, "%str%"))
{
int argumentSize = static_cast<int>(Token::getStrLength(tok2));
if (argumentAlreadyChecked == false)
{
lastCheckedArgumentMaxSize = argumentSize;
len += argumentSize;
argumentAlreadyChecked = true;
}
else if (argumentSize > lastCheckedArgumentMaxSize)
{
// when sprintf() argument is ternary
// operation we may have two and more
// strings as argument. In this case we
// use length of longest string.
len += (argumentSize - lastCheckedArgumentMaxSize);
lastCheckedArgumentMaxSize = argumentSize;
}
}
}
if (len >= (int)size)
{
bufferOverrun(tok);
}
}
} }
// snprintf.. // snprintf..
@ -842,7 +771,7 @@ void CheckBufferOverrun::bufferOverrun()
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
int CheckBufferOverrun::count(const std::string &input_string) int CheckBufferOverrun::countSprintfLength(const std::string &input_string, const std::list<const Token*> &/*parameters*/)
{ {
@ -926,3 +855,139 @@ int CheckBufferOverrun::count(const std::string &input_string)
} }
void CheckBufferOverrun::checkSprintfCall(const Token *tok, int size)
{
if (0)
{
std::list<const Token*> parameters;
if (tok->tokAt(5)->str() == ",")
{
const Token *end = tok->next()->link();
for (const Token *tok2 = tok->tokAt(6); tok2 && tok2 != end; tok2 = tok2->next())
{
if (Token::Match(tok2, ", %any% [,)]"))
{
if (Token::Match(tok2->next(), "%str%"))
parameters.push_back(tok2->next());
else if (Token::Match(tok2->next(), "%num%"))
parameters.push_back(tok2->next());
// TODO, get value of the variable if possible and use that instead of 0
else
parameters.push_back(0);
}
else
{
// Parameter is more complex, than just a value or variable. Ignore it for now
// and skip to next token.
parameters.push_back(0);
int ind = 0;
for (const Token *tok3 = tok2->next(); tok3; tok3 = tok3->next())
{
if (tok3->str() == "(")
++ind;
else if (tok3->str() == ")")
{
--ind;
if (ind < 0)
break;
}
else if (ind == 0 && tok3->str() == ",")
{
tok2 = tok3->previous();
break;
}
}
if (ind < 0)
break;
}
}
}
int len = countSprintfLength(tok->tokAt(4)->strValue(), parameters);
if (len > size)
{
bufferOverrun(tok);
}
}
else
{
int len = -2;
// check format string
const char *fmt = tok->strAt(4);
while (*fmt)
{
if (*fmt == '\\')
{
++fmt;
}
else if (*fmt == '%')
{
++fmt;
// skip field width
while (std::isdigit(*fmt))
{
++fmt;
}
// FIXME: better handling for format specifiers
++fmt;
continue;
}
++fmt;
++len;
}
if (len >= (int)size)
{
bufferOverrun(tok);
}
// check arguments (if they exists)
if (tok->tokAt(5)->str() == ",")
{
len = 0;
const Token *end = tok->next()->link();
bool argumentAlreadyChecked = false;
int lastCheckedArgumentMaxSize = 0;
for (const Token *tok2 = tok->tokAt(6); tok2 && tok2 != end; tok2 = tok2->next())
{
if (tok2->str() == ",")
{
argumentAlreadyChecked = false;
lastCheckedArgumentMaxSize = 0;
}
else if (Token::Match(tok2, "%str%"))
{
int argumentSize = static_cast<int>(Token::getStrLength(tok2));
if (argumentAlreadyChecked == false)
{
lastCheckedArgumentMaxSize = argumentSize;
len += argumentSize;
argumentAlreadyChecked = true;
}
else if (argumentSize > lastCheckedArgumentMaxSize)
{
// when sprintf() argument is ternary
// operation we may have two and more
// strings as argument. In this case we
// use length of longest string.
len += (argumentSize - lastCheckedArgumentMaxSize);
lastCheckedArgumentMaxSize = argumentSize;
}
}
}
if (len >= (int)size)
{
bufferOverrun(tok);
}
}
}
}

10
src/checkbufferoverrun.h
View File

@ -56,10 +56,18 @@ public:
/** Check for buffer overruns */ /** Check for buffer overruns */
void bufferOverrun(); void bufferOverrun();
static int count(const std::string &input_string); static int countSprintfLength(const std::string &input_string, const std::list<const Token*> &parameters);
private: private:
/**
* Check code that matches: "sprintf ( %varid% , %str% [,)]" when varid is not 0,
* and report found errors.
* @param tok The "sprintf" token.
* @param size The size of the buffer where sprintf is writing.
*/
void checkSprintfCall(const Token *tok, int size);
/** Check for buffer overruns - locate struct variables and check them with the .._CheckScope function */ /** Check for buffer overruns - locate struct variables and check them with the .._CheckScope function */
void checkStructVariable(); void checkStructVariable();

45
test/testbufferoverrun.cpp
View File

@ -826,7 +826,7 @@ private:
check("void f()\n" check("void f()\n"
"{\n" "{\n"
" char str[5];\n" " char str[5];\n"
" sprintf(str, \"test%s\", "");\n" " sprintf(str, \"test%s\", \"\");\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
@ -1013,17 +1013,38 @@ private:
void counter_test() void counter_test()
{ {
ASSERT_EQUALS(6, CheckBufferOverrun::count("Hello")); std::list<const Token*> unknownParameter;
ASSERT_EQUALS(2, CheckBufferOverrun::count("s")); unknownParameter.push_back(0);
ASSERT_EQUALS(2, CheckBufferOverrun::count("i"));
ASSERT_EQUALS(2, CheckBufferOverrun::count("%d")); ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("Hello", unknownParameter));
ASSERT_EQUALS(2, CheckBufferOverrun::count("%1d")); ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("s", unknownParameter));
ASSERT_EQUALS(3, CheckBufferOverrun::count("%2.2d")); ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("i", unknownParameter));
ASSERT_EQUALS(1, CheckBufferOverrun::count("%s")); ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("%d", unknownParameter));
ASSERT_EQUALS(6, CheckBufferOverrun::count("%-5s")); ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("%1d", unknownParameter));
ASSERT_EQUALS(2, CheckBufferOverrun::count("\\\"")); ASSERT_EQUALS(3, CheckBufferOverrun::countSprintfLength("%2.2d", unknownParameter));
TODO_ASSERT_EQUALS(6, CheckBufferOverrun::count("Hello\\0Text")); ASSERT_EQUALS(1, CheckBufferOverrun::countSprintfLength("%s", unknownParameter));
TODO_ASSERT_EQUALS(2, CheckBufferOverrun::count("%%")); ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%-5s", unknownParameter));
ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("\\\"", unknownParameter));
TODO_ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("Hello\\0Text", unknownParameter));
TODO_ASSERT_EQUALS(2, CheckBufferOverrun::countSprintfLength("%%", unknownParameter));
std::list<const Token*> stringAsParameter;
{
Token tok;
tok.str("\"12345\"");
stringAsParameter.push_back(&tok);
}
TODO_ASSERT_EQUALS(9, CheckBufferOverrun::countSprintfLength("str%s", stringAsParameter));
std::list<const Token*> intAsParameter;
{
Token tok;
tok.str("12345");
stringAsParameter.push_back(&tok);
}
TODO_ASSERT_EQUALS(6, CheckBufferOverrun::countSprintfLength("%02ld", intAsParameter));
} }
void strncpy1() void strncpy1()