diff --git a/lib/checkother.cpp b/lib/checkother.cpp index f691c0ef2..487176a08 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -3392,6 +3392,35 @@ void CheckOther::sizeofCalculationError(const Token *tok, bool inconclusive) "sizeofCalculation", "Found calculation inside sizeof().", inconclusive); } +//----------------------------------------------------------------------------- +// Check for code like: +// seteuid(geteuid()) or setuid(getuid()), which first gets and then sets the +// (effective) user id to itself. Very often this indicates a copy and paste +// error. +//----------------------------------------------------------------------------- +void CheckOther::redundantGetAndSetUserId() +{ + if (_settings->isEnabled("warning") + && _settings->standards.posix) { + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { + if (Token::simpleMatch(tok, "setuid ( getuid ( ) )") + || Token::simpleMatch(tok, "seteuid ( geteuid ( ) )") + || Token::simpleMatch(tok, "setgid ( getgid ( ) )") + || Token::simpleMatch(tok, "setegid ( getegid ( ) )")) { + redundantGetAndSetUserIdError(tok); + } + } + } +} +void CheckOther::redundantGetAndSetUserIdError(const Token *tok) +{ + reportError(tok, Severity::warning, + "redundantGetAndSetUserId", "Redundant get and set of user id.\n" + "Redundant statement without any effect. First the user id is retrieved" + "by get(e)uid() and then set with set(e)uid().", false); +} + //----------------------------------------------------------------------------- // Check for code like sizeof()*sizeof() or sizeof(ptr)/value //----------------------------------------------------------------------------- diff --git a/lib/checkother.h b/lib/checkother.h index 86f8e3cd5..b67b55850 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -99,6 +99,7 @@ public: checkOther.checkMathFunctions(); checkOther.checkCCTypeFunctions(); + checkOther.redundantGetAndSetUserId(); checkOther.checkIncorrectLogicOperator(); checkOther.checkMisusedScopedObject(); checkOther.checkComparisonOfFuncReturningBool(); @@ -180,6 +181,9 @@ public: /** @brief %Check for calculations inside sizeof */ void sizeofCalculation(); + /** @brief % Check for seteuid(geteuid()) or setuid(getuid())*/ + void redundantGetAndSetUserId(); + /** @brief %Check for suspicious calculations with sizeof results */ void suspiciousSizeofCalculation(); @@ -304,6 +308,7 @@ private: void clarifyStatementError(const Token* tok); void sizeofsizeofError(const Token *tok); void sizeofCalculationError(const Token *tok, bool inconclusive); + void redundantGetAndSetUserIdError(const Token *tok); void multiplySizeofError(const Token *tok); void divideSizeofError(const Token *tok); void cstyleCastError(const Token *tok); @@ -512,6 +517,7 @@ private: "* incorrect usage of functions from ctype library.\n" "* Comparisons of modulo results that are always true/false.\n" "* Array filled incompletely using memset/memcpy/memmove.\n" + "* redundant get and set function of user id (--std=posix).\n" "* Passing NULL pointer to function with variable number of arguments leads to UB on some platforms.\n"; } diff --git a/test/testother.cpp b/test/testother.cpp index 0c0cc555f..f83894a49 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -123,6 +123,8 @@ private: TEST_CASE(sizeofForArrayParameter); TEST_CASE(sizeofForNumericParameter); + TEST_CASE(redundantGetAndSetUserId); + TEST_CASE(suspiciousSizeofCalculation); TEST_CASE(clarifyCalculation); @@ -193,16 +195,18 @@ private: TEST_CASE(varFuncNullUB); } - void check(const char code[], const char *filename = NULL, bool experimental = false, bool inconclusive = true) { + void check(const char code[], const char *filename = NULL, bool experimental = false, bool inconclusive = true, bool posix = false) { // Clear the error buffer.. errout.str(""); Settings settings; settings.addEnabled("style"); + settings.addEnabled("warning"); settings.addEnabled("portability"); settings.addEnabled("performance"); settings.inconclusive = inconclusive; settings.experimental = experimental; + settings.standards.posix = posix; // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -4383,6 +4387,26 @@ private: ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious usage of 'sizeof' with a numeric constant as parameter.\n", errout.str()); } + void redundantGetAndSetUserId() { + check("seteuid(geteuid());\n", NULL, false , false, true); + ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str()); + check("setuid(getuid());\n", NULL, false , false, true); + ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str()); + check("setgid(getgid());\n", NULL, false , false, true); + ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str()); + check("setegid(getegid());\n", NULL, false , false, true); + ASSERT_EQUALS("[test.cpp:1]: (warning) Redundant get and set of user id.\n", errout.str()); + + check("seteuid(getuid());\n", NULL, false , false, true); + ASSERT_EQUALS("", errout.str()); + check("seteuid(getuid());\n", NULL, false , false, true); + ASSERT_EQUALS("", errout.str()); + check("seteuid(foo());\n", NULL, false , false, true); + ASSERT_EQUALS("", errout.str()); + check("foo(getuid());\n", NULL, false , false, true); + ASSERT_EQUALS("", errout.str()); + } + void suspiciousSizeofCalculation() { check("int* p;\n" "return sizeof(p)/5;");