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

partial fix for #3035 (false negative: strcpy(dst, src) where src is bigger than dst)

This commit is contained in:
Robert Reif 2011-08-21 14:44:55 -04:00
parent 7afec3cf6d
commit 67e8731a96
3 changed files with 59 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
lib/checkbufferoverrun.cpp
View File

@ -113,6 +113,13 @@ void CheckBufferOverrun::bufferOverrun(const Token *tok, const std::string &varn
reportError(tok, Severity::error, "bufferAccessOutOfBounds", errmsg);
}
void CheckBufferOverrun::possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst)
{
reportError(tok, Severity::warning, "possibleBufferAccessOutOfBounds",
"Possible buffer overflow if strlen(" + src + ") is larger than sizeof(" + dst + ")-strlen(" + dst +").\n"
"The source buffer is larger than the destination buffer so there is the potential for overflowing the destination buffer.");
}
void CheckBufferOverrun::strncatUsage(const Token *tok)
{
if (_settings && !_settings->isEnabled("style"))
@ -970,7 +977,21 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
continue;
}
}
else if ((varid > 0 && Token::Match(tok, "strcpy|strcat ( %varid% , %var% )", varid)) ||
(varid == 0 && Token::Match(tok, ("strcpy|strcat ( " + varnames + " , %var% )").c_str())))
{
const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(tok->tokAt(4)->varId());
if (var && var->isArray() && var->dimensions().size() == 1)
{
const std::size_t len = var->dimension(0);
if (total_size > 0 && len > (unsigned int)total_size)
{
if (_settings->inconclusive)
possibleBufferOverrunError(tok, tok->strAt(4), tok->strAt(2));
continue;
}
}
}
// Detect few strcat() calls
const std::string strcatPattern = varid > 0 ? std::string("strcat ( %varid% , %str% ) ;") : ("strcat ( " + varnames + " , %str% ) ;");

2
lib/checkbufferoverrun.h
View File

@ -221,6 +221,7 @@ public:
void cmdLineArgsError(const Token *tok);
void pointerOutOfBounds(const Token *tok, const std::string &object); // UB when result of calculation is out of bounds
void arrayIndexThenCheckError(const Token *tok, const std::string &indexName);
void possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst);
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings)
{
@ -235,6 +236,7 @@ public:
c.cmdLineArgsError(0);
c.pointerOutOfBounds(0, "array");
c.arrayIndexThenCheckError(0, "index");
c.possibleBufferOverrunError(0, "source", "destination");
}
std::string myName() const

35
test/testbufferoverrun.cpp
View File

@ -42,6 +42,7 @@ private:
errout.str("");
Settings settings;
settings.inconclusive = true;
settings.experimental = experimental;
settings.addEnabled("style");
@ -142,6 +143,7 @@ private:
TEST_CASE(buffer_overrun_19); // #2597 - class member with unknown type
TEST_CASE(buffer_overrun_20); // #2986 (segmentation fault)
TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch
TEST_CASE(possible_buffer_overrun_1); // #3035
// It is undefined behaviour to point out of bounds of an array
// the address beyond the last element is in bounds
@ -2014,6 +2016,39 @@ private:
ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:3]: (error) Array 'a[10]' index 100 out of bounds\n", errout.str());
}
void possible_buffer_overrun_1() // #3035
{
check("void foo() {\n"
" char * data = (char *)alloca(50);\n"
" char src[100];\n"
" memset(src, 'C', 100-1);\n"
" src[100-1] = '\\0';\n"
" strcat(data, src);\n"
"}");
ASSERT_EQUALS("[test.cpp:6]: (warning) Possible buffer overflow if strlen(src) is larger than sizeof(data)-strlen(data).\n", errout.str());
check("void foo() {\n"
" char * data = (char *)alloca(100);\n"
" char src[100];\n"
" memset(src, 'C', 100-1);\n"
" src[100-1] = '\\0';\n"
" strcat(data, src);\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void foo(char src[100]) {\n"
" char * data = (char *)alloca(50);\n"
" strcat(data, src);\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Possible buffer overflow if strlen(src) is larger than sizeof(data)-strlen(data).\n", errout.str());
check("void foo(char src[100]) {\n"
" char * data = (char *)alloca(100);\n"
" strcat(data, src);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void pointer_out_of_bounds_1()
{
check("void f() {\n"