From 8ccd95a6434e56afc1af0471bdaecd7e1dd5a771 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 24 Apr 2010 21:48:58 +0200 Subject: [PATCH] Fixed #836 (buffer overrun: memmove) --- lib/checkbufferoverrun.cpp | 45 +++++++++++++++++++++++++++++++++---- lib/checkbufferoverrun.h | 3 +++ lib/tokenize.cpp | 6 +++++ test/testbufferoverrun.cpp | 17 ++++++++++++++ test/testsimplifytokens.cpp | 11 +++++---- 5 files changed, 72 insertions(+), 10 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 7b2ac7109..d762579a4 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -967,11 +967,40 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo // Check function call.. - if (Token::Match(tok, "%var% ( %varid% ,", arrayInfo.varid)) - checkFunctionCall(*tok, 1, arrayInfo); - if (Token::Match(tok, "%var% ( %var% , %varid% ,", arrayInfo.varid)) - checkFunctionCall(*tok, 2, arrayInfo); + if (Token::Match(tok, "%var% (")) + { + // 1st parameter.. + if (Token::Match(tok->tokAt(2), "%varid% ,", arrayInfo.varid)) + checkFunctionCall(*tok, 1, arrayInfo); + else if (Token::Match(tok->tokAt(2), "%varid% + %num% ,", arrayInfo.varid)) + { + const ArrayInfo ai(arrayInfo.limit(MathLib::toLongNumber(tok->strAt(4)))); + checkFunctionCall(*tok, 1, ai); + } + // goto 2nd parameter and check it.. + for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) + { + if (tok2->str() == "(") + { + tok2 = tok2->link(); + continue; + } + if (tok2->str() == ";" || tok2->str() == ")") + break; + if (tok2->str() == ",") + { + if (Token::Match(tok2, ", %varid% ,", arrayInfo.varid)) + checkFunctionCall(*tok, 2, arrayInfo); + else if (Token::Match(tok2, ", %varid% + %num% ,", arrayInfo.varid)) + { + const ArrayInfo ai(arrayInfo.limit(MathLib::toLongNumber(tok2->strAt(3)))); + checkFunctionCall(*tok, 2, ai); + } + break; + } + } + } if (_settings->_checkCodingStyle) { @@ -1596,6 +1625,14 @@ CheckBufferOverrun::ArrayInfo::ArrayInfo(unsigned int id, const std::string &nam _varname = name; } +CheckBufferOverrun::ArrayInfo CheckBufferOverrun::ArrayInfo::limit(long value) const +{ + unsigned int n = 1; + for (unsigned int i = 0; i < num.size(); ++i) + n *= num[i]; + return ArrayInfo(varid, varname, element_size, value > (int)n ? 0 : n - value); +} + bool CheckBufferOverrun::ArrayInfo::declare(const Token *tok, const Tokenizer &tokenizer) { _num.clear(); diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index b64b95644..1a89d113b 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -130,6 +130,9 @@ public: */ ArrayInfo(unsigned int id, const std::string &name, unsigned int size1, unsigned int n); + /** Create a copy ArrayInfo where the number of elements have been limited by a value */ + ArrayInfo limit(long value) const; + /** * Declare array - set info * \param tok first token in array declaration diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 013a6ea3b..9551b2302 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -1414,6 +1414,12 @@ void Tokenizer::arraySize() if (Token::Match(tok2, "%any% } ;")) tok->next()->insertToken(MathLib::toString(sz)); } + + else if (Token::Match(tok, "%var% [ ] = %str% ;")) + { + unsigned int sz = tok->strAt(4).length() - 1; + tok->next()->insertToken(MathLib::toString(sz)); + } } } diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 28d42c62c..fe48f1136 100755 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -117,6 +117,7 @@ private: TEST_CASE(buffer_overrun_10); TEST_CASE(buffer_overrun_11); TEST_CASE(buffer_overrun_12); + TEST_CASE(buffer_overrun_13); TEST_CASE(sprintf1); TEST_CASE(sprintf2); @@ -1451,6 +1452,22 @@ private: ASSERT_EQUALS("[test.cpp:3]: (error) Buffer access out-of-bounds\n", errout.str()); } + void buffer_overrun_13() + { + // ticket #836 + check("void f() {\n" + " char a[10];\n" + " memset(a+5, 0, 10);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Buffer access out-of-bounds: a\n", errout.str()); + + check("void f() {\n" + " char a[10];\n" + " memmove(a, a+5, 10);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Buffer access out-of-bounds: a\n", errout.str()); + } + void sprintf1() { check("void f()\n" diff --git a/test/testsimplifytokens.cpp b/test/testsimplifytokens.cpp index f0777fe2b..d0882eacc 100644 --- a/test/testsimplifytokens.cpp +++ b/test/testsimplifytokens.cpp @@ -573,7 +573,7 @@ private: void declareArray() { const char code[] = "void f ( ) { char str [ ] = \"100\" ; }"; - const char expected[] = "void f ( ) { char * str ; str = \"100\" ; }"; + const char expected[] = "void f ( ) { char str [ 4 ] = \"100\" ; }"; ASSERT_EQUALS(expected, tok(code)); } @@ -638,7 +638,7 @@ private: " char a[] = \"p\";\n" " ++a[0];\n" "}\n"; - ASSERT_EQUALS("void f ( ) { char * a ; a = \"p\" ; ++ a [ 0 ] ; }", tok(code)); + ASSERT_EQUALS("void f ( ) { char a [ 2 ] = \"p\" ; ++ a [ 0 ] ; }", tok(code)); } } @@ -865,7 +865,7 @@ private: const char code[] = "; const char str[] = \"1\"; sizeof(str);"; std::ostringstream expected; - expected << "; const char * str ; str = \"1\" ; " << sizeofFromTokenizer("char")*2 << " ;"; + expected << "; const char str [ 2 ] = \"1\" ; " << sizeofFromTokenizer("char")*2 << " ;"; ASSERT_EQUALS(expected.str(), sizeof_(code)); } @@ -1308,11 +1308,10 @@ private: "}\n"; const char expected[] = "void f ( ) " "{" - " const char * s ;" - " s = \"abcd\" ;" + " const char s [ 5 ] = \"abcd\" ;" " 4 ; " "}"; - ASSERT_EQUALS(expected, tok(code)); + TODO_ASSERT_EQUALS(expected, tok(code)); } }