diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 77106adaa..45808a417 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -120,6 +120,13 @@ void CheckBufferOverrun::sizeArgumentAsChar(const Token *tok) reportError(tok, Severity::possibleError, "sizeArgumentAsChar", "The size argument is given as a char constant"); } + +void CheckBufferOverrun::terminateStrncpyError(const Token *tok) +{ + reportError(tok, Severity::style, "terminateStrncpy", "After a strncpy() the buffer should be zero-terminated"); +} + + //--------------------------------------------------------------------------- @@ -213,6 +220,35 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con // memset, memcmp, memcpy, strncpy, fgets.. if (varid > 0) { + if (_settings->_checkCodingStyle) + { + // check for strncpy which is not terminated + if (Token::Match(tok, "strncpy ( %varid% , %any% , %any% )", varid)) + { + const Token *tokSz = tok->tokAt(6); + if (tokSz->isNumber()) + { + // strncpy takes entire variable length as input size + const char *num = tok->strAt(6); + if (std::atoi(num) == total_size) + { + const Token *tok2 = tok->next()->link()->next()->next(); + for (; tok2; tok2 = tok2->next()) + { + if (Token::Match(tok2, "%varid%", tok->tokAt(2)->varId())) + { + if (!Token::Match(tok2, "%varid% [ %any% ] = 0 ;", tok->tokAt(2)->varId())) + { + terminateStrncpyError(tok); + } else { + break; + } + } + } + } + } + } + } if (Token::Match(tok, "memset|memcpy|memmove|memcmp|strncpy|fgets ( %varid% , %any% , %any% )", varid) || Token::Match(tok, "memset|memcpy|memmove|memcmp|fgets ( %var% , %varid% , %any% )", varid)) { diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 4555ffadb..ed51b1be2 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -86,6 +86,7 @@ private: void strncatUsage(const Token *tok); void outOfBounds(const Token *tok, const std::string &what); void sizeArgumentAsChar(const Token *tok); + void terminateStrncpyError(const Token *tok); void getErrorMessages() { diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index cbad82f7e..f3db045ff 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -54,6 +54,7 @@ private: // Check for buffer overruns.. Settings settings; settings._showAll = showAll; + settings._checkCodingStyle = true; CheckBufferOverrun checkBufferOverrun(&tokenizer, &settings, this); checkBufferOverrun.bufferOverrun(); } @@ -137,6 +138,9 @@ private: TEST_CASE(counter_test); TEST_CASE(strncpy1); TEST_CASE(unknownType); + + TEST_CASE(terminateStrncpy1); + TEST_CASE(terminateStrncpy2); } @@ -1448,6 +1452,39 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); } + + void terminateStrncpy1() + { + check("void foo ( char *bar )\n" + "{\n" + " char baz[100];\n" + " strncpy(baz, bar, sizeof(baz));\n" + " strncpy(baz, bar, sizeof(baz));\n" + " baz[99] = 0;\n" + " strncpy(baz, bar, sizeof(baz));\n" + " baz[sizeof(baz)-1] = 0;\n" + " strncpy(baz, bar, sizeof(baz));\n" + " *(baz + 99) = 0;\n" + " strncpy(baz, bar, sizeof(baz));\n" + " bar[99] = 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (style) After a strncpy() the buffer should be zero-terminated\n", errout.str()); + } + + void terminateStrncpy2() + { + check("char *foo ( char *bar )\n" + "{\n" + " char baz[100];\n" + " strncpy(baz, bar, sizeof(baz));\n" + " bar[99] = 0;\n" + " return baz;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (style) After a strncpy() the buffer should be zero-terminated\n", errout.str()); + } + + + }; REGISTER_TEST(TestBufferOverrun)