diff --git a/src/checkbufferoverrun.cpp b/src/checkbufferoverrun.cpp index d61a5d5a1..31d4a4fe8 100644 --- a/src/checkbufferoverrun.cpp +++ b/src/checkbufferoverrun.cpp @@ -348,20 +348,12 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con // Writing data into array.. if (Token::Match(tok, ("strcpy|strcat ( " + varnames + " , %str% )").c_str())) { - int len = 0; - const char *str = tok->strAt(varc + 4); - while (*str) - { - if (*str == '\\') - ++str; - ++str; - ++len; - } - if (len > 2 && len >= (int)size + 2) + size_t len = Token::getStrLength(tok->tokAt(varc + 4)); + if (len >= static_cast(size)) { bufferOverrun(tok); + continue; } - continue; } @@ -382,6 +374,23 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con strncatUsage(tok->tokAt(9)); } + // Detect few strcat() calls + if (varid > 0 && Token::Match(tok, "strcat ( %varid% , %str% ) ;", varid)) + { + size_t charactersAppend = 0; + const Token *tok2 = tok; + + while (tok2 && Token::Match(tok2, "strcat ( %varid% , %str% ) ;", varid)) + { + charactersAppend += Token::getStrLength(tok2->tokAt(4)); + if (charactersAppend >= static_cast(size)) + { + bufferOverrun(tok2); + break; + } + tok2 = tok2->tokAt(7); + } + } // sprintf.. if (varid > 0 && Token::Match(tok, "sprintf ( %varid% , %str% [,)]", varid)) @@ -420,15 +429,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con { if (tok2->str()[0] == '\"') { - len -= 2; - const char *str = tok2->str().c_str(); - while (*str) - { - if (*str == '\\') - ++str; - ++str; - ++len; - } + len += Token::getStrLength(tok2); } } if (len >= (int)size) diff --git a/src/token.cpp b/src/token.cpp index 25dfaae72..f0d1295df 100644 --- a/src/token.cpp +++ b/src/token.cpp @@ -491,6 +491,32 @@ bool Token::Match(const Token *tok, const char pattern[], unsigned int varid) return true; } +size_t Token::getStrLength(const Token *tok) +{ + assert(tok != NULL); + + size_t len = 0; + const char *str = tok->strAt(0); + + assert(str[0] == '"'); + assert(str[strlen(str)-1] == '"'); + + while (*str) + { + if (*str == '\\') + ++str; + ++str; + ++len; + } + + assert(len >= 2); + + // don't count quotes + len -= 2; + + return len; +} + bool Token::isStandardType() const { bool ret = false; diff --git a/src/token.h b/src/token.h index a40831d69..4d48d7d80 100644 --- a/src/token.h +++ b/src/token.h @@ -117,6 +117,15 @@ public: */ static bool Match(const Token *tok, const char pattern[], unsigned int varid = 0); + /** + * Return length of C-string. + * + * Should be called for %str% tokens only. + * + * @param tok token with C-string + **/ + static size_t getStrLength(const Token *tok); + bool isName() const { return _isName; diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 440aea3e7..d4babff2b 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -93,6 +93,7 @@ private: TEST_CASE(buffer_overrun_3); TEST_CASE(buffer_overrun_4); TEST_CASE(buffer_overrun_5); + TEST_CASE(buffer_overrun_6); TEST_CASE(sprintf1); TEST_CASE(sprintf2); @@ -558,6 +559,16 @@ private: ASSERT_EQUALS("", errout.str()); } + void buffer_overrun_6() + { + check("void f()\n" + "{\n" + " char n[5];\n" + " strcat(n, \"abc\");\n" + " strcat(n, \"def\");\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (possible error) Buffer overrun\n", errout.str()); + } void sprintf1() { diff --git a/test/testtoken.cpp b/test/testtoken.cpp index 8b2292a37..0ba458634 100644 --- a/test/testtoken.cpp +++ b/test/testtoken.cpp @@ -34,6 +34,7 @@ private: { TEST_CASE(nextprevious); TEST_CASE(multiCompare); + TEST_CASE(getStrLength); } void nextprevious() @@ -78,6 +79,23 @@ private: ASSERT_EQUALS(static_cast(-1), static_cast(Token::multiCompare("abc|def", "abcd"))); ASSERT_EQUALS(static_cast(-1), static_cast(Token::multiCompare("abc|def", "default"))); } + + void getStrLength() + { + Token *tok = new Token(); + + tok->str("\"\""); + ASSERT_EQUALS(0, Token::getStrLength(tok)); + + tok->str("\"test\""); + ASSERT_EQUALS(4, Token::getStrLength(tok)); + + tok->str("\"test \\\\test\""); + ASSERT_EQUALS(10, Token::getStrLength(tok)); + + Tokenizer::deleteTokens(tok); + } + }; REGISTER_TEST(TestTOKEN)