New check: Division by sizeof() as parameter to memset/memcpy/memmove/etc. as they expect a size in bytes (#5698)
Refactorizations in sizeof checking: - Changed severity of sizeofwithsilentarraypointer to warning - Made pointerSize message conclusive - there seems to be no reason for inconclusive
This commit is contained in:
PKEuS
parent
c4635cf698
commit
5c238692e6
|
|
@ -87,7 +87,7 @@ void CheckSizeof::checkSizeofForArrayParameter()
|
|||
|
||||
void CheckSizeof::sizeofForArrayParameterError(const Token *tok)
|
||||
{
|
||||
reportError(tok, Severity::error,
|
||||
reportError(tok, Severity::warning,
|
||||
"sizeofwithsilentarraypointer", "Using 'sizeof' on array given as function argument "
|
||||
"returns size of a pointer.\n"
|
||||
"Using 'sizeof' for array given as function argument returns the size of a pointer. "
|
||||
|
|
@ -111,8 +111,9 @@ void CheckSizeof::checkSizeofForPointerSize()
|
|||
for (std::size_t i = 0; i < functions; ++i) {
|
||||
const Scope * scope = symbolDatabase->functionScopes[i];
|
||||
for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
|
||||
const Token *tokVar;
|
||||
const Token *variable;
|
||||
const Token* tokSize;
|
||||
const Token* tokFunc;
|
||||
const Token *variable = nullptr;
|
||||
const Token *variable2 = nullptr;
|
||||
|
||||
// Find any function that may use sizeof on a pointer
|
||||
|
|
@ -121,32 +122,45 @@ void CheckSizeof::checkSizeofForPointerSize()
|
|||
// - tokVar pointing on the argument where sizeof may be used
|
||||
if (Token::Match(tok, "[*;{}] %var% = malloc|alloca (")) {
|
||||
variable = tok->next();
|
||||
tokVar = tok->tokAt(5);
|
||||
|
||||
tokSize = tok->tokAt(5);
|
||||
tokFunc = tok->tokAt(3);
|
||||
} else if (Token::Match(tok, "[*;{}] %var% = calloc (")) {
|
||||
variable = tok->next();
|
||||
tokVar = tok->tokAt(5)->nextArgument();
|
||||
|
||||
tokSize = tok->tokAt(5)->nextArgument();
|
||||
tokFunc = tok->tokAt(3);
|
||||
} else if (Token::Match(tok, "return malloc|alloca (")) {
|
||||
tokSize = tok->tokAt(3);
|
||||
tokFunc = tok->next();
|
||||
} else if (Token::simpleMatch(tok, "return calloc (")) {
|
||||
tokSize = tok->tokAt(3)->nextArgument();
|
||||
tokFunc = tok->next();
|
||||
} else if (Token::simpleMatch(tok, "memset (")) {
|
||||
variable = tok->tokAt(2);
|
||||
tokVar = variable->tokAt(2)->nextArgument();
|
||||
|
||||
// The following tests can be inconclusive in case the variable in sizeof
|
||||
// is constant string by intention
|
||||
} else if (!_settings->inconclusive) {
|
||||
continue;
|
||||
|
||||
tokSize = variable->nextArgument();
|
||||
if (tokSize)
|
||||
tokSize = tokSize->nextArgument();
|
||||
tokFunc = tok;
|
||||
} else if (Token::Match(tok, "memcpy|memcmp|memmove|strncpy|strncmp|strncat (")) {
|
||||
variable = tok->tokAt(2);
|
||||
variable2 = variable->nextArgument();
|
||||
if (!variable2)
|
||||
continue;
|
||||
tokVar = variable2->nextArgument();
|
||||
|
||||
tokSize = variable2->nextArgument();
|
||||
tokFunc = tok;
|
||||
} else {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (tokFunc && tokSize) {
|
||||
for (const Token* tok2 = tokSize; tok2 != tokFunc->linkAt(1); tok2 = tok2->next()) {
|
||||
if (Token::simpleMatch(tok2, "/ sizeof"))
|
||||
divideBySizeofError(tok2, tokFunc->str());
|
||||
}
|
||||
}
|
||||
|
||||
if (!variable)
|
||||
continue;
|
||||
|
||||
// Ensure the variables are in the symbol database
|
||||
// Also ensure the variables are pointers
|
||||
// Only keep variables which are pointers
|
||||
|
|
@ -170,16 +184,16 @@ void CheckSizeof::checkSizeofForPointerSize()
|
|||
|
||||
// Jump to the next sizeof token in the function and in the parameter
|
||||
// This is to allow generic operations with sizeof
|
||||
for (; tokVar && tokVar->str() != ")" && tokVar->str() != "," && tokVar->str() != "sizeof"; tokVar = tokVar->next()) {}
|
||||
for (; tokSize && tokSize->str() != ")" && tokSize->str() != "," && tokSize->str() != "sizeof"; tokSize = tokSize->next()) {}
|
||||
|
||||
// Now check for the sizeof usage. Once here, everything using sizeof(varid) or sizeof(&varid)
|
||||
// looks suspicious
|
||||
// Do it for first variable
|
||||
if (variable && (Token::Match(tokVar, "sizeof ( &| %varid% )", variable->varId()) ||
|
||||
Token::Match(tokVar, "sizeof &| %varid%", variable->varId()))) {
|
||||
if (variable && (Token::Match(tokSize, "sizeof ( &| %varid% )", variable->varId()) ||
|
||||
Token::Match(tokSize, "sizeof &| %varid%", variable->varId()))) {
|
||||
sizeofForPointerError(variable, variable->str());
|
||||
} else if (variable2 && (Token::Match(tokVar, "sizeof ( &| %varid% )", variable2->varId()) ||
|
||||
Token::Match(tokVar, "sizeof &| %varid%", variable2->varId()))) {
|
||||
} else if (variable2 && (Token::Match(tokSize, "sizeof ( &| %varid% )", variable2->varId()) ||
|
||||
Token::Match(tokSize, "sizeof &| %varid%", variable2->varId()))) {
|
||||
sizeofForPointerError(variable2, variable2->str());
|
||||
}
|
||||
}
|
||||
|
|
@ -192,7 +206,13 @@ void CheckSizeof::sizeofForPointerError(const Token *tok, const std::string &var
|
|||
"Size of pointer '" + varname + "' used instead of size of its data.\n"
|
||||
"Size of pointer '" + varname + "' used instead of size of its data. "
|
||||
"This is likely to lead to a buffer overflow. You probably intend to "
|
||||
"write 'sizeof(*" + varname + ")'.", true);
|
||||
"write 'sizeof(*" + varname + ")'.");
|
||||
}
|
||||
|
||||
void CheckSizeof::divideBySizeofError(const Token *tok, const std::string &memfunc)
|
||||
{
|
||||
reportError(tok, Severity::warning, "sizeofDivisionMemfunc",
|
||||
"Division by result of sizeof(). " + memfunc + "() expects a size in bytes, did you intend to multiply instead?\n");
|
||||
}
|
||||
|
||||
//-----------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -92,6 +92,7 @@ private:
|
|||
void divideSizeofError(const Token* tok);
|
||||
void sizeofForArrayParameterError(const Token* tok);
|
||||
void sizeofForPointerError(const Token* tok, const std::string &varname);
|
||||
void divideBySizeofError(const Token* tok, const std::string &memfunc);
|
||||
void sizeofForNumericParameterError(const Token* tok);
|
||||
void sizeofVoidError(const Token *tok);
|
||||
void sizeofDereferencedVoidPointerError(const Token *tok, const std::string &varname);
|
||||
|
|
@ -102,6 +103,7 @@ private:
|
|||
|
||||
c.sizeofForArrayParameterError(0);
|
||||
c.sizeofForPointerError(0, "varname");
|
||||
c.divideBySizeofError(0, "memset");
|
||||
c.sizeofForNumericParameterError(0);
|
||||
c.sizeofsizeofError(0);
|
||||
c.sizeofCalculationError(0, false);
|
||||
|
|
@ -118,7 +120,6 @@ private:
|
|||
|
||||
std::string classInfo() const {
|
||||
return "sizeof() usage checks\n"
|
||||
|
||||
"* sizeof for array given as function argument\n"
|
||||
"* sizeof for numeric given as function argument\n"
|
||||
"* using sizeof(pointer) instead of the size of pointed data\n"
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ private:
|
|||
TEST_CASE(sizeofsizeof);
|
||||
TEST_CASE(sizeofCalculation);
|
||||
TEST_CASE(checkPointerSizeof);
|
||||
TEST_CASE(sizeofDivisionMemset);
|
||||
TEST_CASE(sizeofForArrayParameter);
|
||||
TEST_CASE(sizeofForNumericParameter);
|
||||
TEST_CASE(suspiciousSizeofCalculation);
|
||||
|
|
@ -165,21 +166,21 @@ private:
|
|||
" std::cout << sizeof(a) / sizeof(int) << std::endl;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as "
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Using 'sizeof' on array given as "
|
||||
"function argument returns size of a pointer.\n", errout.str());
|
||||
|
||||
check("void f( int a[]) {\n"
|
||||
" std::cout << sizeof a / sizeof(int) << std::endl;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as "
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Using 'sizeof' on array given as "
|
||||
"function argument returns size of a pointer.\n", errout.str());
|
||||
|
||||
check("void f( int a[3] ) {\n"
|
||||
" std::cout << sizeof(a) / sizeof(int) << std::endl;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as "
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Using 'sizeof' on array given as "
|
||||
"function argument returns size of a pointer.\n", errout.str());
|
||||
|
||||
check("void f(int *p) {\n"
|
||||
|
|
@ -223,7 +224,7 @@ private:
|
|||
" std::cout << sizeof(a) / sizeof(int) << std::endl;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as "
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Using 'sizeof' on array given as "
|
||||
"function argument returns size of a pointer.\n", errout.str());
|
||||
|
||||
// ticket #2510
|
||||
|
|
@ -231,7 +232,7 @@ private:
|
|||
" std::cout << sizeof(a) / sizeof(int) << std::endl;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (error) Using 'sizeof' on array given as "
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Using 'sizeof' on array given as "
|
||||
"function argument returns size of a pointer.\n", errout.str());
|
||||
|
||||
// ticket #2510
|
||||
|
|
@ -322,25 +323,25 @@ private:
|
|||
" int *x = malloc(sizeof(x));\n"
|
||||
" free(x);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int *x = malloc(sizeof(&x));\n"
|
||||
" free(x);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int *x = malloc(100 * sizeof(x));\n"
|
||||
" free(x);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int *x = malloc(sizeof(x) * 100);\n"
|
||||
" free(x);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int *x = malloc(sizeof *x);\n"
|
||||
|
|
@ -352,13 +353,13 @@ private:
|
|||
" int *x = malloc(sizeof x);\n"
|
||||
" free(x);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int *x = malloc(100 * sizeof x);\n"
|
||||
" free(x);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int *x = calloc(1, sizeof(*x));\n"
|
||||
|
|
@ -376,13 +377,13 @@ private:
|
|||
" int *x = calloc(1, sizeof(x));\n"
|
||||
" free(x);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int *x = calloc(1, sizeof x);\n"
|
||||
" free(x);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int *x = calloc(1, sizeof(int));\n"
|
||||
|
|
@ -434,28 +435,28 @@ private:
|
|||
" memset(x, 0, sizeof x);\n"
|
||||
" free(x);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:3]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int *x = malloc(sizeof(int));\n"
|
||||
" memset(x, 0, sizeof(x));\n"
|
||||
" free(x);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:3]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int *x = malloc(sizeof(int) * 10);\n"
|
||||
" memset(x, 0, sizeof(x) * 10);\n"
|
||||
" free(x);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:3]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int *x = malloc(sizeof(int) * 10);\n"
|
||||
" memset(x, 0, sizeof x * 10);\n"
|
||||
" free(x);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:3]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" int *x = malloc(sizeof(int) * 10);\n"
|
||||
|
|
@ -484,13 +485,13 @@ private:
|
|||
" const char *buf1_ex = \"foobarbaz\";\n"
|
||||
" return strncmp(buf1, buf1_ex, sizeof(buf1_ex)) == 0;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) Size of pointer 'buf1_ex' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:4]: (warning) Size of pointer 'buf1_ex' used instead of size of its data.\n", errout.str());
|
||||
|
||||
check(
|
||||
"int fun(const char *buf1) {\n"
|
||||
" return strncmp(buf1, foo(buf2), sizeof(buf1)) == 0;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Size of pointer 'buf1' used instead of size of its data.\n", errout.str());
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'buf1' used instead of size of its data.\n", errout.str());
|
||||
|
||||
// #ticket 3874
|
||||
check("void f()\n"
|
||||
|
|
@ -501,6 +502,18 @@ private:
|
|||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void sizeofDivisionMemset() {
|
||||
check("void foo(memoryMapEntry_t* entry, memoryMapEntry_t* memoryMapEnd) {\n"
|
||||
" memmove(entry, entry + 1, (memoryMapEnd - entry) / sizeof(entry));\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Division by result of sizeof(). memmove() expects a size in bytes, did you intend to multiply instead?\n", errout.str());
|
||||
|
||||
check("Foo* allocFoo(int num) {\n"
|
||||
" return malloc(num / sizeof(Foo));\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Division by result of sizeof(). malloc() expects a size in bytes, did you intend to multiply instead?\n", errout.str());
|
||||
}
|
||||
|
||||
void sizeofVoid() {
|
||||
check("void f() {\n"
|
||||
" int size = sizeof(void);\n"
|
||||
|
|
|
|||
Loading…
Reference in New Issue