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

TestBufferOverrun: clean up

This commit is contained in:
Daniel Marjamäki 2015-02-10 17:29:36 +01:00
parent efd2f51cba
commit d9deabe2ce
4 changed files with 43 additions and 378 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

99
lib/checkbufferoverrun.cpp
View File

@ -142,15 +142,6 @@ void CheckBufferOverrun::possibleBufferOverrunError(const Token *tok, const std:
"The source buffer is larger than the destination buffer so there is the potential for overflowing the destination buffer.");
}
void CheckBufferOverrun::possibleReadlinkBufferOverrunError(const Token* tok, const std::string &funcname, const std::string &varname)
{
const std::string errmsg = funcname + "() might return the full size of '" + varname + "'. Lower the supplied size by one.\n" +
funcname + "() might return the full size of '" + varname + "'. "
"If a " + varname + "[len] = '\\0'; statement follows, it will overrun the buffer. Lower the supplied size by one.";
reportError(tok, Severity::warning, "possibleReadlinkBufferOverrun", errmsg, true);
}
void CheckBufferOverrun::strncatUsageError(const Token *tok)
{
if (_settings && !_settings->isEnabled("warning"))
@ -989,11 +980,6 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
if (n > total_size)
outOfBoundsError(tok->tokAt(4), "snprintf size", true, n, total_size);
}
// readlink() / readlinkat() buffer usage
if (_settings->standards.posix && Token::Match(tok, "readlink|readlinkat ("))
checkReadlinkBufferUsage(tok, scope_begin, declarationId, total_size);
}
}
}
@ -1021,44 +1007,6 @@ bool CheckBufferOverrun::isArrayOfStruct(const Token* tok, int &position)
return false;
}
void CheckBufferOverrun::checkReadlinkBufferUsage(const Token* ftok, const Token *scope_begin, const unsigned int varid, const MathLib::bigint total_size)
{
const std::string& funcname = ftok->str();
const Token* bufParam = ftok->tokAt(2)->nextArgument();
if (funcname == "readlinkat")
bufParam = bufParam ? bufParam->nextArgument() : nullptr;
if (!Token::Match(bufParam, "%varid% , %num% )", varid))
return;
const MathLib::bigint n = MathLib::toLongNumber(bufParam->strAt(2));
if (total_size > 0 && n > total_size)
outOfBoundsError(bufParam, funcname + "() buf size", true, n, total_size);
if (!_settings->inconclusive)
return;
// only writing a part of the buffer
if (n < total_size)
return;
// readlink()/readlinkat() never terminates the buffer, check the end of the scope for buffer termination.
bool found_termination = false;
const Token *scope_end = scope_begin->link();
for (const Token *tok2 = bufParam->tokAt(4); tok2 && tok2 != scope_end; tok2 = tok2->next()) {
if (Token::Match(tok2, "%varid% [ %any% ] = 0 ;", bufParam->varId())) {
found_termination = true;
break;
}
}
if (!found_termination) {
bufferNotZeroTerminatedError(ftok, bufParam->str(), funcname);
} else if (n == total_size) {
possibleReadlinkBufferOverrunError(ftok, funcname, bufParam->str());
}
}
//---------------------------------------------------------------------------
// Checking local variables in a scope
//---------------------------------------------------------------------------
@ -1826,53 +1774,6 @@ void CheckBufferOverrun::arrayIndexThenCheckError(const Token *tok, const std::s
"not be accessed if the index is out of limits.");
}
// -------------------------------------------------------------------------------------
// Check the second and the third parameter of the POSIX function write and validate
// their values.
// The parameters have the following meaning:
// - 1.parameter: file descripter (not required for this check)
// - 2.parameter: is a null terminated character string of the content to write.
// - 3.parameter: the number of bytes to write.
//
// This check is triggered if the size of the string ( 2. parameter) is lower than
// the number of bytes provided at the 3. parameter.
//
// References:
// - http://pubs.opengroup.org/onlinepubs/9699919799/functions/write.html
// - http://gd.tuwien.ac.at/languages/c/programming-bbrown/c_075.htm
// - http://codewiki.wikidot.com/c:system-calls:write
// -------------------------------------------------------------------------------------
void CheckBufferOverrun::writeOutsideBufferSize()
{
if (!_settings->standards.posix)
return;
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
const std::size_t functions = symbolDatabase->functionScopes.size();
for (std::size_t i = 0; i < functions; ++i) {
const Scope * const scope = symbolDatabase->functionScopes[i];
for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
if (Token::Match(tok, "pwrite|write (") && Token::Match(tok->tokAt(2)->nextArgument(), "%str% , %num%")) {
const std::string & functionName(tok->str());
tok = tok->tokAt(2)->nextArgument(); // set tokenptr to %str% parameter
const std::size_t stringLength = Token::getStrLength(tok)+1; // zero-terminated string!
tok = tok->tokAt(2); // set tokenptr to %num% parameter
const MathLib::bigint writeLength = MathLib::toLongNumber(tok->str());
if (static_cast<std::size_t>(writeLength) > stringLength)
writeOutsideBufferSizeError(tok, stringLength, writeLength, functionName);
}
}
}
}
void CheckBufferOverrun::writeOutsideBufferSizeError(const Token *tok, const std::size_t stringLength, const MathLib::bigint writeLength, const std::string &strFunctionName)
{
reportError(tok, Severity::error, "writeOutsideBufferSize",
"Writing " + MathLib::toString(writeLength-stringLength) + " bytes outside buffer size.\n"
"The number of bytes to write (" + MathLib::toString(writeLength) + " bytes) are bigger than the source buffer (" +MathLib::toString(stringLength)+ " bytes)."
" Please check the second and the third parameter of the function '"+strFunctionName+"'.");
}
Check::FileInfo* CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const
{
(void)settings;

12
lib/checkbufferoverrun.h
View File

@ -60,7 +60,6 @@ public:
checkBufferOverrun.bufferOverrun();
checkBufferOverrun.bufferOverrun2();
checkBufferOverrun.arrayIndexThenCheck();
checkBufferOverrun.writeOutsideBufferSize();
}
/** @brief %Check for buffer overruns */
@ -75,9 +74,6 @@ public:
/** @brief %Check for buffer overruns by inspecting execution paths */
void executionPaths();
/** @brief %Check using POSIX write function and writing outside buffer size */
void writeOutsideBufferSize();
/**
* @brief Get minimum length of format string result
* @param input_string format string
@ -180,9 +176,6 @@ public:
/** Check for buffer overruns */
void checkScope(const Token *tok, const std::vector<std::string> &varname, const ArrayInfo &arrayInfo);
/** Check readlink or readlinkat() buffer usage */
void checkReadlinkBufferUsage(const Token *ftok, const Token *scope_begin, const unsigned int varid, const MathLib::bigint total_size);
/**
* Helper function for checkFunctionCall - check a function parameter
* \param tok token for the function name
@ -243,9 +236,7 @@ private:
void pointerOutOfBoundsError(const Token *tok, const Token *index=nullptr, const MathLib::bigint indexvalue=0);
void arrayIndexThenCheckError(const Token *tok, const std::string &indexName);
void possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst, bool cat);
void possibleReadlinkBufferOverrunError(const Token *tok, const std::string &funcname, const std::string &varname);
void argumentSizeError(const Token *tok, const std::string &functionName, const std::string &varname);
void writeOutsideBufferSizeError(const Token *tok, const std::size_t stringLength, const MathLib::bigint writeLength, const std::string& functionName);
void valueFlowCheckArrayIndex(const Token * const tok, const ArrayInfo &arrayInfo);
@ -266,9 +257,7 @@ public:
c.pointerOutOfBoundsError(nullptr, nullptr, 0);
c.arrayIndexThenCheckError(0, "index");
c.possibleBufferOverrunError(0, "source", "destination", false);
c.possibleReadlinkBufferOverrunError(0, "readlink", "buffer");
c.argumentSizeError(0, "function", "array");
c.writeOutsideBufferSizeError(0,2,3,"write");
c.negativeMemoryAllocationSizeError(0);
c.reportError(nullptr, Severity::warning, "arrayIndexOutOfBoundsCond", "Array 'x[10]' accessed at index 20, which is out of bounds. Otherwise condition 'y==20' is redundant.");
}
@ -288,7 +277,6 @@ private:
"- Unsafe usage of main(argv, argc) arguments\n"
"- Accessing array with index variable before checking its value\n"
"- Check for large enough arrays being passed to functions\n"
"- Writing beyond bounds of a buffer\n"
"- Allocating memory with a negative size\n";
}
};

23
test/cfg/posix.c
View File

@ -9,6 +9,29 @@
#include <unistd.h>
void bufferAccessOutOf(int fd) {
char a[5];
read(fd,a,5);
// cppcheck-suppress bufferAccessOutOfBounds
read(fd,a,6);
write(fd,a,5);
// cppcheck-suppress bufferAccessOutOfBounds
write(fd,a,6);
recv(fd,a,5,0);
// cppcheck-suppress bufferAccessOutOfBounds
recv(fd,a,6,0);
recvfrom(fd,a,5,0,0x0,0x0);
// cppcheck-suppress bufferAccessOutOfBounds
recvfrom(fd,a,6,0,0x0,0x0);
send(fd,a,5,0);
// cppcheck-suppress bufferAccessOutOfBounds
send(fd,a,6,0);
sendto(fd,a,5,0,0x0,0x0);
// cppcheck-suppress bufferAccessOutOfBounds
sendto(fd,a,6,0,0x0,0x0);
0;
}
void f(char *p) {
isatty (0);
mkdir (p, 0);

287
test/testbufferoverrun.cpp
View File

@ -40,7 +40,6 @@ private:
Settings settings;
settings.inconclusive = true;
settings.standards.posix = true;
settings.experimental = experimental;
settings.addEnabled("warning");
settings.addEnabled("style");
@ -68,7 +67,6 @@ private:
checkBufferOverrun.bufferOverrun();
checkBufferOverrun.bufferOverrun2();
checkBufferOverrun.arrayIndexThenCheck();
checkBufferOverrun.writeOutsideBufferSize();
}
void check(const char code[], const Settings &settings, const char filename[] = "test.cpp") {
@ -85,7 +83,6 @@ private:
checkBufferOverrun.bufferOverrun();
checkBufferOverrun.bufferOverrun2();
checkBufferOverrun.arrayIndexThenCheck();
checkBufferOverrun.writeOutsideBufferSize();
}
void checkstd(const char code[], const char filename[] = "test.cpp") {
@ -99,31 +96,6 @@ private:
check(code, settings, filename);
}
void checkposix(const char code[], const char filename[] = "test.cpp") {
static bool init;
static Settings settings;
if (!init) {
init = true;
LOAD_LIB_2(settings.library, "posix.cfg");
settings.addEnabled("warning");
}
Tokenizer tokenizer(&settings, this);
std::istringstream istr(code);
tokenizer.tokenize(istr, filename);
// Clear the error buffer..
errout.str("");
// Check for buffer overruns..
CheckBufferOverrun checkBufferOverrun(&tokenizer, &settings, this);
checkBufferOverrun.bufferOverrun();
checkBufferOverrun.bufferOverrun2();
checkBufferOverrun.arrayIndexThenCheck();
checkBufferOverrun.writeOutsideBufferSize();
}
void run() {
TEST_CASE(noerr1);
TEST_CASE(noerr2);
@ -197,8 +169,6 @@ private:
TEST_CASE(array_index_valueflow);
TEST_CASE(array_index_function_parameter);
TEST_CASE(buffer_overrun_1_standard_functions);
TEST_CASE(buffer_overrun_1_posix_functions);
TEST_CASE(buffer_overrun_2_struct);
TEST_CASE(buffer_overrun_3);
TEST_CASE(buffer_overrun_4);
@ -318,10 +288,6 @@ private:
TEST_CASE(arrayIndexThenCheck);
TEST_CASE(bufferNotZeroTerminated);
TEST_CASE(readlink);
TEST_CASE(readlinkat);
TEST_CASE(writeOutsideBufferSize)
TEST_CASE(negativeMemoryAllocationSizeError) // #389
@ -2159,95 +2125,6 @@ private:
ASSERT_EQUALS("", errout.str());
}
void buffer_overrun_1_posix_functions() {
checkposix("void f(int fd)\n"
"{\n"
" char str[3];\n"
" read(fd, str, 3);\n"
"}");
ASSERT_EQUALS("", errout.str());
checkposix("void f(int fd)\n"
"{\n"
" char str[3];\n"
" read(fd, str, 4);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str());
checkposix("void f(int fd)\n"
"{\n"
" char str[3];\n"
" write(fd, str, 3);\n"
"}");
ASSERT_EQUALS("", errout.str());
checkposix("void f(int fd)\n"
"{\n"
" char str[3];\n"
" write(fd, str, 4);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str());
checkposix("void f()\n"
"{\n"
" long bb[2];\n"
" write(stdin, bb, sizeof(bb));\n"
"}");
ASSERT_EQUALS("", errout.str());
checkposix("void f()\n"
"{\n"
"char str[3];\n"
"recv(s, str, 4, 0);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str());
checkposix("void f()\n"
"{\n"
"char str[3];\n"
"recvfrom(s, str, 4, 0, 0x0, 0x0);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str());
checkposix("void f()\n"
"{\n"
"char str[3];\n"
"send(s, str, 4, 0);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str());
checkposix("void f()\n"
"{\n"
"char str[3];\n"
"sendto(s, str, 4, 0, 0x0, 0x0);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: str\n", errout.str());
}
void buffer_overrun_1_standard_functions() {
// #4968 - not standard function
checkstd("void f() {\n"
" char str[3];\n"
" foo.memset(str, 0, 100);\n"
" foo::memset(str, 0, 100);\n"
" std::memset(str, 0, 100);\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: str\n", "", errout.str());
// #5257 - check strings
checkstd("void f() {\n"
" memcpy(temp, \"hello world\", 20);\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (error) Buffer is accessed out of bounds.\n", errout.str());
checkstd("void f() {\n"
" memcpy(temp, \"abc\", 4);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void buffer_overrun_2_struct() {
check("struct ABC\n"
"{\n"
@ -3713,6 +3590,26 @@ private:
" memset(a+5, 0, 10);\n"
"}", settings);
ASSERT_EQUALS("[test.cpp:3]: (error) Buffer is accessed out of bounds: a\n", errout.str());
// #4968 - not standard function
check("void f() {\n"
" char str[3];\n"
" foo.memset(str, 0, 100);\n"
" foo::memset(str, 0, 100);\n"
" std::memset(str, 0, 100);\n"
"}", settings);
TODO_ASSERT_EQUALS("[test.cpp:5]: (error) Buffer is accessed out of bounds: str\n", "", errout.str());
// #5257 - check strings
check("void f() {\n"
" memset(\"abc\", 0, 20);\n"
"}", settings);
ASSERT_EQUALS("[test.cpp:2]: (error) Buffer is accessed out of bounds.\n", errout.str());
check("void f() {\n"
" memset(temp, \"abc\", 4);\n"
"}", settings);
ASSERT_EQUALS("", errout.str());
}
void minsize_sizeof() {
@ -4233,150 +4130,6 @@ private:
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) The buffer 'c' is not null-terminated after the call to memmove().\n", errout.str());
}
void readlink() {
check("void f() {\n"
" char buf[255];\n"
" ssize_t len = readlink(path, buf, 254);\n"
" printf(\"%s\n\", buf);\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) The buffer 'buf' is not null-terminated after the call to readlink().\n", "", errout.str());
// C only: Primitive pointer simplification
check("void f() {\n"
" char buf[255];\n"
" ssize_t len = readlink(path, &buf[0], 254);\n"
" printf(\"%s\n\", buf);\n"
"}\n", true, "test.c");
TODO_ASSERT_EQUALS("[test.c:3]: (warning, inconclusive) The buffer 'buf' is not null-terminated after the call to readlink().\n", "", errout.str());
check("void f() {\n"
" char buf[255];\n"
" ssize_t len = readlink(path, buf, 254);\n"
" buf[len] = 0;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" char buf[10];\n"
" ssize_t len = readlink(path, buf, 255);\n"
" buf[len] = 0;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) readlink() buf size is out of bounds: Supplied size 255 is larger than actual size 10.\n", errout.str());
check("void f() {\n"
" char buf[255];\n"
" ssize_t len = readlink(path, buf, 255);\n"
" buf[len] = 0;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) readlink() might return the full size of 'buf'. Lower the supplied size by one.\n", errout.str());
check("void f() {\n"
" char buf[255];\n"
" ssize_t len = readlink(path, buf, 254);\n"
" if (len == -1) {\n"
" return;\n"
" }\n"
" buf[len] = 0;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" char buf[255] = {0};\n"
" readlink(path, buf, 254);\n" // <- doesn't write whole buf
" puts(buf);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void readlinkat() {
check("void f() {\n"
" char buf[255];\n"
" ssize_t len = readlinkat(42, path, buf, 254);\n"
" printf(\"%s\n\", buf);\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) The buffer 'buf' is not null-terminated after the call to readlinkat().\n", "", errout.str());
check("void f() {\n"
" char buf[255];\n"
" ssize_t len = readlinkat(42, path, buf, 254);\n"
" buf[len] = 0;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" char buf[10];\n"
" ssize_t len = readlinkat(42, path, buf, 255);\n"
" buf[len] = 0;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) readlinkat() buf size is out of bounds: Supplied size 255 is larger than actual size 10.\n", errout.str());
check("void f() {\n"
" char buf[255];\n"
" ssize_t len = readlinkat(42, path, buf, 255);\n"
" buf[len] = 0;\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) readlinkat() might return the full size of 'buf'. Lower the supplied size by one.\n", errout.str());
check("void f() {\n"
" char buf[255];\n"
" ssize_t len = readlinkat(42, path, buf, 254);\n"
" if (len == -1) {\n"
" return;\n"
" }\n"
" buf[len] = 0;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void writeOutsideBufferSize() {
check("void f(void){\n"
"write(1, \"Dump string \\n\", 100);\n"
"}"); // ^ number of bytes too big
ASSERT_EQUALS("[test.cpp:2]: (error) Writing 86 bytes outside buffer size.\n", errout.str());
check("void f(void){\n"
"write(1, \"Dump string \\n\", 10);\n"
"}");
ASSERT_EQUALS("", errout.str());
// #4706 avoid crashing when a struct member is used as first argument
check("static struct {\n"
" int i[2];\n"
"} p;\n"
"void foo()\n"
"{\n"
" write(p.i[1], \"\", 1);\n"
"}");
ASSERT_EQUALS("", errout.str());
check("static struct {\n"
" int i[2];\n"
"} p;\n"
"void foo()\n"
"{\n"
" write(p.i[1], \"\", 2);\n"
"}");
ASSERT_EQUALS("[test.cpp:6]: (error) Writing 1 bytes outside buffer size.\n", errout.str());
// #4969
check("void foo()\n"
"{\n"
" write(1, \"\\0\", 1);\n"
"}");
ASSERT_EQUALS("", errout.str());
// that is documented to be ok
check("void foo()\n"
"{\n"
" write(1, 0, 0);\n"
"}");
ASSERT_EQUALS("", errout.str());
// ... that is not ok
check("void foo()\n"
"{\n"
" write(1, 0, 1);\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:3]: (error) Writing 1 bytes outside buffer size.\n", "", errout.str());
}
void negativeMemoryAllocationSizeError() { // #389
check("void f()\n"
"{\n"