From 24530ebd60f39ce19eba60bd48f342088feebe04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 8 Jan 2009 06:24:08 +0000 Subject: [PATCH] sprintf: check for dangerous usage with sprintf|snprintf with overlapping data --- src/checkother.cpp | 43 +++++++++++++++++++++++++++++++++++++++++++ test/testother.cpp | 31 ++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/checkother.cpp b/src/checkother.cpp index da1c90cdf..6332b5ddf 100644 --- a/src/checkother.cpp +++ b/src/checkother.cpp @@ -296,6 +296,7 @@ void CheckOther::WarningIf() void CheckOther::InvalidFunctionUsage() { + // strtol and strtoul.. for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { if ((tok->str() != "strtol") && (tok->str() != "strtoul")) @@ -330,6 +331,48 @@ void CheckOther::InvalidFunctionUsage() } } } + + // sprintf|snprintf overlapping data + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + // Get variable id of target buffer.. + unsigned int varid = 0; + + if ( Token::Match(tok, "sprintf|snprintf ( %var% ,") ) + varid = tok->tokAt(2)->varId(); + + else if ( Token::Match(tok, "sprintf|snprintf ( %var% . %var% ,") ) + varid = tok->tokAt(4)->varId(); + + if ( varid == 0 ) + continue; + + // goto "," + const Token *tok2 = tok->tokAt(3); + while ( tok2 && tok2->str() != "," ) + tok2 = tok2->next(); + + // is any source buffer overlapping the target buffer? + unsigned int parlevel = 0; + while ( (tok2 = tok2->next()) != NULL ) + { + if ( tok2->str() == "(" ) + ++parlevel; + else if ( tok2->str() == ")" ) + { + --parlevel; + if ( parlevel < 0 ) + break; + } + else if ( tok2->varId() == varid ) + { + std::ostringstream ostr; + ostr << _tokenizer->fileLine(tok2) << ": Overlapping data buffer " << tok2->str(); + _errorLogger->reportErr(ostr.str()); + break; + } + } + } } //--------------------------------------------------------------------------- diff --git a/test/testother.cpp b/test/testother.cpp index 549b9bd3a..fffff7dc0 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -35,8 +35,9 @@ private: void run() { TEST_CASE(delete1); - TEST_CASE(delete2); + + TEST_CASE(sprintf1); // Dangerous usage of sprintf } void check(const char code[]) @@ -85,6 +86,34 @@ private: "}\n"); ASSERT_EQUALS(std::string("[test.cpp:3]: Redundant condition. It is safe to deallocate a NULL pointer\n"), errout.str()); } + + + + void sprintfUsage(const char code[]) + { + // Tokenize.. + Tokenizer tokenizer; + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + tokenizer.setVarId(); + + // Clear the error buffer.. + errout.str(""); + + // Check for redundant code.. + CheckOther checkOther(&tokenizer, this); + checkOther.InvalidFunctionUsage(); + } + + void sprintf1() + { + sprintfUsage( "void foo()\n" + "{\n" + " char buf[100];\n" + " sprintf(buf,\"%s\",buf);\n" + "}\n"); + ASSERT_EQUALS(std::string("[test.cpp:4]: Overlapping data buffer buf\n"), errout.str()); + } }; REGISTER_TEST(TestOther)