diff --git a/src/checkbufferoverrun.cpp b/src/checkbufferoverrun.cpp index 4c736b288..8c5d789ad 100644 --- a/src/checkbufferoverrun.cpp +++ b/src/checkbufferoverrun.cpp @@ -382,11 +382,36 @@ void CheckBufferOverrun::checkScope(const Token *tok, const char *varname[], con // sprintf.. - if (varid > 0 && Token::Match(tok, "sprintf ( %varid% , %str% ,", varid)) + if (varid > 0 && Token::Match(tok, "sprintf ( %varid% , %str% [,)]", varid)) { - int len = 0; + int len = -2; const Token *end = tok->next()->link(); + // check format string + const char *fmt = tok->strAt(4); + while (*fmt) + { + if (*fmt == '\\') + { + ++fmt; + } + else if (*fmt == '%') + { + // FIXME: better handling for format specifiers + fmt += 2; + continue; + } + ++fmt; + ++len; + } + + if (len >= (int)size) + { + bufferOverrun(tok); + } + + // check arguments + len = 0; for (const Token *tok2 = tok->tokAt(6); tok2 && tok2 != end; tok2 = tok2->next()) { if (tok2->str()[0] == '\"') diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 586d62206..0c7112886 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -95,6 +95,7 @@ private: TEST_CASE(sprintf1); TEST_CASE(sprintf2); + TEST_CASE(sprintf3); TEST_CASE(snprintf1); TEST_CASE(snprintf2); @@ -573,7 +574,23 @@ private: " sprintf(str, \"%d: %s\", getnumber(), \"abcde\");\n" "}\n"); ASSERT_EQUALS("[test.cpp:4]: (possible error) Buffer overrun\n", errout.str()); + } + void sprintf3() + { + check("void f()\n" + "{\n" + " char str[3];\n" + " sprintf(str, \"test\");\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (possible error) Buffer overrun\n", errout.str()); + + check("void f()\n" + "{\n" + " char str[5];\n" + " sprintf(str, \"test%s\", "");\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void snprintf1()