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

refactor CheckBufferOverrun::checkScope strncpy check and change experimental to inconclusive

This commit is contained in:
Robert Reif 2011-08-27 21:18:39 -04:00
parent 15e965b530
commit 8c093d0f8a
3 changed files with 24 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
lib/checkbufferoverrun.cpp
View File

@ -155,9 +155,9 @@ void CheckBufferOverrun::sizeArgumentAsCharError(const Token *tok)
} }
void CheckBufferOverrun::terminateStrncpyError(const Token *tok) void CheckBufferOverrun::terminateStrncpyError(const Token *tok, const std::string &varname)
{ {
reportError(tok, Severity::warning, "terminateStrncpy", "After a strncpy() the buffer should be zero-terminated"); reportError(tok, Severity::warning, "terminateStrncpy", "After a strncpy() the buffer '" + varname + "' should be zero-terminated");
} }
void CheckBufferOverrun::cmdLineArgsError(const Token *tok) void CheckBufferOverrun::cmdLineArgsError(const Token *tok)
@ -1159,15 +1159,18 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
checkFunctionCall(tok, arrayInfo); checkFunctionCall(tok, arrayInfo);
} }
if (_settings->isEnabled("style")) if ((Token::Match(tok, "strncpy|strncat ( %varid% , %var% , %num% )", arrayInfo.varid())) ||
(Token::Match(tok, "strncpy|strncat ( %varid% , %var% [ %any% ] , %num% )", arrayInfo.varid())))
{ {
const int offset = tok->strAt(5) == "[" ? 3 : 0;
// check for strncpy which is not terminated // check for strncpy which is not terminated
if ((Token::Match(tok, "strncpy ( %varid% , %var% , %num% )", arrayInfo.varid())) || if (tok->str() == "strncpy")
(Token::Match(tok, "strncpy ( %varid% , %var% [ %any% ] , %num% )", arrayInfo.varid())))
{ {
// strncpy takes entire variable length as input size // strncpy takes entire variable length as input size
const int offset = tok->strAt(5) == "[" ? 9 : 6; unsigned int num = (unsigned int)MathLib::toLongNumber(tok->strAt(6 + offset));
if ((unsigned int)MathLib::toLongNumber(tok->strAt(offset)) >= total_size)
if (num >= total_size)
{ {
const Token *tok2 = tok->next()->link()->next(); const Token *tok2 = tok->next()->link()->next();
for (; tok2; tok2 = tok2->next()) for (; tok2; tok2 = tok2->next())
@ -1176,9 +1179,9 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
{ {
if (!Token::Match(tok2, "%varid% [ %any% ] = 0 ;", tok->tokAt(2)->varId())) if (!Token::Match(tok2, "%varid% [ %any% ] = 0 ;", tok->tokAt(2)->varId()))
{ {
// this is currently 'experimental'. See TestBufferOverrun::terminateStrncpy3 // this is currently 'inconclusive'. See TestBufferOverrun::terminateStrncpy3
if (_settings->experimental) if (_settings->isEnabled("style") && _settings->inconclusive)
terminateStrncpyError(tok); terminateStrncpyError(tok, tok->strAt(2));
} }
break; break;
@ -1186,24 +1189,21 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
} }
} }
} }
}
// Dangerous usage of strncat.. // Dangerous usage of strncat..
if (Token::Match(tok, "strncpy|strncat ( %varid% , %any% , %num% )", arrayInfo.varid()))
{
if (tok->str() == "strncat") if (tok->str() == "strncat")
{ {
const MathLib::bigint n = MathLib::toLongNumber(tok->strAt(6)); const MathLib::bigint n = MathLib::toLongNumber(tok->strAt(6 + offset));
if (n >= total_size) if (n >= total_size)
strncatUsageError(tok); strncatUsageError(tok);
} }
// Dangerous usage of strncpy + strncat.. // Dangerous usage of strncpy + strncat..
if (Token::Match(tok->tokAt(8), "; strncat ( %varid% , %any% , %num% )", arrayInfo.varid())) if (Token::Match(tok->tokAt(8 + offset), "; strncat ( %varid% , %any% , %num% )", arrayInfo.varid()))
{ {
const MathLib::bigint n = MathLib::toLongNumber(tok->strAt(6)) + MathLib::toLongNumber(tok->strAt(15)); const MathLib::bigint n = MathLib::toLongNumber(tok->strAt(6 + offset)) + MathLib::toLongNumber(tok->strAt(15 + offset));
if (n > total_size) if (n > total_size)
strncatUsageError(tok->tokAt(9)); strncatUsageError(tok->tokAt(9 + offset));
} }
} }

4
lib/checkbufferoverrun.h
View File

@ -216,7 +216,7 @@ public:
void strncatUsageError(const Token *tok); void strncatUsageError(const Token *tok);
void outOfBoundsError(const Token *tok, const std::string &what); void outOfBoundsError(const Token *tok, const std::string &what);
void sizeArgumentAsCharError(const Token *tok); void sizeArgumentAsCharError(const Token *tok);
void terminateStrncpyError(const Token *tok); void terminateStrncpyError(const Token *tok, const std::string &varname);
void negativeIndexError(const Token *tok, MathLib::bigint index); void negativeIndexError(const Token *tok, MathLib::bigint index);
void cmdLineArgsError(const Token *tok); void cmdLineArgsError(const Token *tok);
void pointerOutOfBoundsError(const Token *tok, const std::string &object); // UB when result of calculation is out of bounds void pointerOutOfBoundsError(const Token *tok, const std::string &object); // UB when result of calculation is out of bounds
@ -231,7 +231,7 @@ public:
c.strncatUsageError(0); c.strncatUsageError(0);
c.outOfBoundsError(0, "index"); c.outOfBoundsError(0, "index");
c.sizeArgumentAsCharError(0); c.sizeArgumentAsCharError(0);
c.terminateStrncpyError(0); c.terminateStrncpyError(0, "buffer");
c.negativeIndexError(0, -1); c.negativeIndexError(0, -1);
c.cmdLineArgsError(0); c.cmdLineArgsError(0);
c.pointerOutOfBoundsError(0, "array"); c.pointerOutOfBoundsError(0, "array");

8
test/testbufferoverrun.cpp
View File

@ -2794,7 +2794,7 @@ private:
" strncpy(baz, bar, sizeof(baz));\n" " strncpy(baz, bar, sizeof(baz));\n"
" bar[99] = 0;\n" " bar[99] = 0;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:4]: (warning) After a strncpy() the buffer should be zero-terminated\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (warning) After a strncpy() the buffer 'baz' should be zero-terminated\n", errout.str());
// Test with invalid code that there is no segfault // Test with invalid code that there is no segfault
check("char baz[100];\n" check("char baz[100];\n"
@ -2809,7 +2809,7 @@ private:
" foo(baz);\n" " foo(baz);\n"
" foo(baz);\n" " foo(baz);\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:4]: (warning) After a strncpy() the buffer should be zero-terminated\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (warning) After a strncpy() the buffer 'baz' should be zero-terminated\n", errout.str());
} }
void terminateStrncpy2() void terminateStrncpy2()
@ -2821,7 +2821,7 @@ private:
" bar[99] = 0;\n" " bar[99] = 0;\n"
" return baz;\n" " return baz;\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:4]: (warning) After a strncpy() the buffer should be zero-terminated\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (warning) After a strncpy() the buffer 'baz' should be zero-terminated\n", errout.str());
} }
void terminateStrncpy3() void terminateStrncpy3()
@ -2837,7 +2837,7 @@ private:
"void bar(char *p) {\n" "void bar(char *p) {\n"
" strncpy(p, str, 100);\n" " strncpy(p, str, 100);\n"
"}\n", false); "}\n", false);
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (warning) After a strncpy() the buffer 'str' should be zero-terminated\n", errout.str());
} }
void recursive_long_time() void recursive_long_time()