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

Fixed #3688 (false positive: (inconclusive, posix) (warning) The buffer 'cBuffer' is not zero-terminated after the call to readlink().)

This commit is contained in:
Daniel Marjamäki 2014-01-02 10:46:19 +01:00
parent 12df5300ba
commit a1b0d190df
3 changed files with 27 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
lib/checkbufferoverrun.cpp
View File

@ -1304,12 +1304,8 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
} }
// readlink() / readlinkat() buffer usage // readlink() / readlinkat() buffer usage
if (_settings->standards.posix) { if (_settings->standards.posix && Token::Match(tok, "readlink|readlinkat ("))
if (Token::simpleMatch(tok, "readlink (") && Token::Match(tok->tokAt(2)->nextArgument(), "%varid% , %num% )", arrayInfo.declarationId())) checkReadlinkBufferUsage(tok, scope_begin, arrayInfo.declarationId(), total_size);
checkReadlinkBufferUsage(tok, scope_begin, total_size, false);
else if (Token::simpleMatch(tok, "readlinkat (") && Token::Match(tok->tokAt(2)->nextArgument()->nextArgument(), "%varid% , %num% )", arrayInfo.declarationId()))
checkReadlinkBufferUsage(tok, scope_begin, total_size, true);
}
// undefined behaviour: result of pointer arithmetic is out of bounds // undefined behaviour: result of pointer arithmetic is out of bounds
if (_settings->isEnabled("portability") && Token::Match(tok, "= %varid% + %num% ;", arrayInfo.declarationId())) { if (_settings->isEnabled("portability") && Token::Match(tok, "= %varid% + %num% ;", arrayInfo.declarationId())) {
@ -1344,12 +1340,15 @@ bool CheckBufferOverrun::isArrayOfStruct(const Token* tok, int &position)
return false; return false;
} }
void CheckBufferOverrun::checkReadlinkBufferUsage(const Token* tok, const Token *scope_begin, const MathLib::bigint total_size, const bool is_readlinkat) void CheckBufferOverrun::checkReadlinkBufferUsage(const Token* ftok, const Token *scope_begin, const unsigned int varid, const MathLib::bigint total_size)
{ {
const Token* bufParam = tok->tokAt(2)->nextArgument(); const std::string funcname = ftok->str();
if (is_readlinkat)
bufParam = bufParam->nextArgument(); const Token* bufParam = ftok->tokAt(2)->nextArgument();
const std::string funcname = is_readlinkat ? "readlinkat" : "readlink"; if (funcname == "readlinkat")
bufParam = bufParam ? bufParam->nextArgument() : NULL;
if (!Token::Match(bufParam, "%varid% , %num% )", varid))
return;
const MathLib::bigint n = MathLib::toLongNumber(bufParam->strAt(2)); const MathLib::bigint n = MathLib::toLongNumber(bufParam->strAt(2));
if (total_size > 0 && n > total_size) if (total_size > 0 && n > total_size)
@ -1358,6 +1357,10 @@ void CheckBufferOverrun::checkReadlinkBufferUsage(const Token* tok, const Token
if (!_settings->inconclusive) if (!_settings->inconclusive)
return; 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. // readlink()/readlinkat() never terminates the buffer, check the end of the scope for buffer termination.
bool found_termination = false; bool found_termination = false;
const Token *scope_end = scope_begin->link(); const Token *scope_end = scope_begin->link();
@ -1369,9 +1372,9 @@ void CheckBufferOverrun::checkReadlinkBufferUsage(const Token* tok, const Token
} }
if (!found_termination) { if (!found_termination) {
bufferNotZeroTerminatedError(tok, bufParam->str(), funcname); bufferNotZeroTerminatedError(ftok, bufParam->str(), funcname);
} else if (n == total_size) { } else if (n == total_size) {
possibleReadlinkBufferOverrunError(tok, funcname, bufParam->str()); possibleReadlinkBufferOverrunError(ftok, funcname, bufParam->str());
} }
} }

2
lib/checkbufferoverrun.h
View File

@ -191,7 +191,7 @@ public:
void parse_for_body(const Token *tok2, const ArrayInfo &arrayInfo, const std::string &strindex, bool condition_out_of_bounds, unsigned int counter_varid, const std::string &min_counter_value, const std::string &max_counter_value); void parse_for_body(const Token *tok2, const ArrayInfo &arrayInfo, const std::string &strindex, bool condition_out_of_bounds, unsigned int counter_varid, const std::string &min_counter_value, const std::string &max_counter_value);
/** Check readlink or readlinkat() buffer usage */ /** Check readlink or readlinkat() buffer usage */
void checkReadlinkBufferUsage(const Token *tok, const Token *scope_begin, const MathLib::bigint total_size, const bool is_readlinkat); 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 * Helper function for checkFunctionCall - check a function parameter

13
test/testbufferoverrun.cpp
View File

@ -3940,7 +3940,7 @@ private:
" ssize_t len = readlink(path, buf, 254);\n" " ssize_t len = readlink(path, buf, 254);\n"
" printf(\"%s\n\", buf);\n" " printf(\"%s\n\", buf);\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) The buffer 'buf' is not null-terminated after the call to readlink().\n", errout.str()); 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 // C only: Primitive pointer simplification
check("void f() {\n" check("void f() {\n"
@ -3948,7 +3948,7 @@ private:
" ssize_t len = readlink(path, &buf[0], 254);\n" " ssize_t len = readlink(path, &buf[0], 254);\n"
" printf(\"%s\n\", buf);\n" " printf(\"%s\n\", buf);\n"
"}\n", true, "test.c"); "}\n", true, "test.c");
ASSERT_EQUALS("[test.c:3]: (warning, inconclusive) The buffer 'buf' is not null-terminated after the call to readlink().\n", errout.str()); 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" check("void f() {\n"
" char buf[255];\n" " char buf[255];\n"
@ -3980,6 +3980,13 @@ private:
" buf[len] = 0;\n" " buf[len] = 0;\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); 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() { void readlinkat() {
@ -3988,7 +3995,7 @@ private:
" ssize_t len = readlinkat(42, path, buf, 254);\n" " ssize_t len = readlinkat(42, path, buf, 254);\n"
" printf(\"%s\n\", buf);\n" " printf(\"%s\n\", buf);\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) The buffer 'buf' is not null-terminated after the call to readlinkat().\n", errout.str()); 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" check("void f() {\n"
" char buf[255];\n" " char buf[255];\n"