From 166e4cafcd2be34579ce93330ff48df8a9353051 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Thu, 15 Mar 2018 04:24:17 -0500 Subject: [PATCH] Check for functions calls in sizeof calculations (#1111) * Check for functions calls in sizeof calculations * Use seperate message and id for sizeofFunction * Check for overloads * Using decltype with a function should not be an error * Fix warning * Fix false positives when running pass the close paren * Fix test error * Try to fix more false positives * Traverse using astOperand2 * Only check first argument * Update fixes from feedback from PR --- lib/checksizeof.cpp | 40 +++++++++++++++++++++++++++++ lib/checksizeof.h | 7 +++++ test/testsizeof.cpp | 62 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+) diff --git a/lib/checksizeof.cpp b/lib/checksizeof.cpp index ec2423a40..086f6f42f 100644 --- a/lib/checksizeof.cpp +++ b/lib/checksizeof.cpp @@ -317,6 +317,46 @@ void CheckSizeof::sizeofCalculationError(const Token *tok, bool inconclusive) "sizeofCalculation", "Found calculation inside sizeof().", CWE682, inconclusive); } +//----------------------------------------------------------------------------- + +void CheckSizeof::sizeofFunction() +{ + if (!_settings->isEnabled(Settings::WARNING)) + return; + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { + if (Token::simpleMatch(tok, "sizeof (")) { + + // ignore if the `sizeof` result is cast to void inside a macro, i.e. the calculation is + // expected to be parsed but skipped, such as in a disabled custom ASSERT() macro + if (tok->isExpandedMacro() && tok->previous()) { + const Token *cast_end = (tok->previous()->str() == "(") ? tok->previous() : tok; + if (Token::simpleMatch(cast_end->tokAt(-3), "( void )") || + Token::simpleMatch(cast_end->previous(), "static_cast")) { + continue; + } + } + + if (const Token *argument = tok->next()->astOperand2()) { + const Token *checkToken = argument->previous(); + if (checkToken->tokType() == Token::eName) + break; + const Function * fun = checkToken->function(); + // Dont report error if the function is overloaded + if (fun && fun->nestedIn->functionMap.count(checkToken->str()) == 1) { + sizeofFunctionError(tok); + } + } + } + } +} + +void CheckSizeof::sizeofFunctionError(const Token *tok) +{ + reportError(tok, Severity::warning, + "sizeofFunctionCall", "Found function call inside sizeof().", CWE682, false); +} + //----------------------------------------------------------------------------- // Check for code like sizeof()*sizeof() or sizeof(ptr)/value //----------------------------------------------------------------------------- diff --git a/lib/checksizeof.h b/lib/checksizeof.h index e6f762aac..8aec7baff 100644 --- a/lib/checksizeof.h +++ b/lib/checksizeof.h @@ -56,6 +56,7 @@ public: // Checks checkSizeof.sizeofsizeof(); checkSizeof.sizeofCalculation(); + checkSizeof.sizeofFunction(); checkSizeof.suspiciousSizeofCalculation(); checkSizeof.checkSizeofForArrayParameter(); checkSizeof.checkSizeofForPointerSize(); @@ -73,6 +74,9 @@ public: /** @brief %Check for calculations inside sizeof */ void sizeofCalculation(); + /** @brief %Check for function call inside sizeof */ + void sizeofFunction(); + /** @brief %Check for suspicious calculations with sizeof results */ void suspiciousSizeofCalculation(); @@ -92,6 +96,7 @@ private: // Error messages.. void sizeofsizeofError(const Token* tok); void sizeofCalculationError(const Token* tok, bool inconclusive); + void sizeofFunctionError(const Token* tok); void multiplySizeofError(const Token* tok); void divideSizeofError(const Token* tok); void sizeofForArrayParameterError(const Token* tok); @@ -111,6 +116,7 @@ private: c.sizeofForNumericParameterError(nullptr); c.sizeofsizeofError(nullptr); c.sizeofCalculationError(nullptr, false); + c.sizeofFunctionError(nullptr); c.multiplySizeofError(nullptr); c.divideSizeofError(nullptr); c.sizeofVoidError(nullptr); @@ -129,6 +135,7 @@ private: "- using sizeof(pointer) instead of the size of pointed data\n" "- look for 'sizeof sizeof ..'\n" "- look for calculations inside sizeof()\n" + "- look for function calls inside sizeof()\n" "- look for suspicious calculations with sizeof()\n" "- using 'sizeof(void)' which is undefined\n"; } diff --git a/test/testsizeof.cpp b/test/testsizeof.cpp index d32205bf4..8ff21d8b2 100644 --- a/test/testsizeof.cpp +++ b/test/testsizeof.cpp @@ -40,6 +40,7 @@ private: TEST_CASE(sizeofsizeof); TEST_CASE(sizeofCalculation); + TEST_CASE(sizeofFunction); TEST_CASE(checkPointerSizeof); TEST_CASE(checkPointerSizeofStruct); TEST_CASE(sizeofDivisionMemset); @@ -162,6 +163,67 @@ private: "[test.cpp:5]: (warning, inconclusive) Found calculation inside sizeof().\n", errout.str()); } + void sizeofFunction() { + check("class Foo\n" + "{\n" + " int bar() { return 1; };\n" + "}\n" + "Foo f;int a=sizeof(f.bar());"); + ASSERT_EQUALS("[test.cpp:5]: (warning) Found function call inside sizeof().\n", errout.str()); + + check("class Foo\n" + "{\n" + " int bar() { return 1; };\n" + " int bar() const { return 1; };\n" + "}\n" + "Foo f;int a=sizeof(f.bar());"); + ASSERT_EQUALS("", errout.str()); + + check("class Foo\n" + "{\n" + " int bar() { return 1; };\n" + "}\n" + "Foo * fp;int a=sizeof(fp->bar());"); + ASSERT_EQUALS("[test.cpp:5]: (warning) Found function call inside sizeof().\n", errout.str()); + + check("int a=sizeof(foo());"); + ASSERT_EQUALS("", errout.str()); + + check("int foo() { return 1; }; int a=sizeof(foo());"); + ASSERT_EQUALS("[test.cpp:1]: (warning) Found function call inside sizeof().\n", errout.str()); + + check("int foo() { return 1; }; sizeof(decltype(foo()));"); + ASSERT_EQUALS("", errout.str()); + + check("int foo(int) { return 1; }; int a=sizeof(foo(0))"); + ASSERT_EQUALS("[test.cpp:1]: (warning) Found function call inside sizeof().\n", errout.str()); + + check("char * buf; int a=sizeof(*buf);"); + ASSERT_EQUALS("", errout.str()); + + check("int a=sizeof(foo())"); + ASSERT_EQUALS("", errout.str()); + + check("int foo(int) { return 1; }; char buf[1024]; int a=sizeof(buf), foo(0)"); + ASSERT_EQUALS("", errout.str()); + + check("template\n" + "struct A\n" + "{\n" + " static B f(const B &);\n" + " static A f(const A &);\n" + " static A &g();\n" + " static T &h();\n" + "\n" + " enum {\n" + " X = sizeof(f(g() >> h())) == sizeof(A),\n" + " Y = sizeof(f(g() << h())) == sizeof(A),\n" + " Z = X & Y\n" + " };\n" + "};\n"); + ASSERT_EQUALS("", errout.str()); + } + void sizeofForArrayParameter() { check("void f() {\n" " int a[10];\n"