From f057e127a0ebd1f0faf18b1ff89c2236e0b1eac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 21 Apr 2010 19:27:28 +0200 Subject: [PATCH] CheckBufferOverrun: Refactoring the checking of function calls --- lib/checkbufferoverrun.cpp | 94 +++++++++++++++++++++++++------------- lib/checkbufferoverrun.h | 2 + test/testbufferoverrun.cpp | 16 +++---- 3 files changed, 71 insertions(+), 41 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 6cede4533..b2e642bc2 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -462,6 +462,55 @@ void CheckBufferOverrun::parse_for_body(const Token *tok2, const ArrayInfo &arra +void CheckBufferOverrun::checkFunctionCall(const Token *tok, const ArrayInfo &arrayInfo) +{ + std::map total_size; + total_size["fgets"] = 2; // The second argument for fgets can't exceed the total size of the array + total_size["memcmp"] = 3; + total_size["memcpy"] = 3; + total_size["memmove"] = 3; + total_size["memset"] = 3; + total_size["strncat"] = 3; + total_size["strncmp"] = 3; + total_size["strncpy"] = 3; + + std::map::const_iterator it = total_size.find(tok->str()); + if (it != total_size.end()) + { + unsigned int arg = it->second; + for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) + { + if (tok2->str() == "(") + { + tok2 = tok2->link(); + continue; + } + if (tok2->str() == ")") + break; + if (tok2->str() == ",") + { + --arg; + if (arg == 1) + { + if (Token::Match(tok2, ", %num% ,|)")) + { + const long sz = MathLib::toLongNumber(tok2->strAt(1)); + unsigned int elements = 1; + for (unsigned int i = 0; i < arrayInfo.num.size(); ++i) + elements *= arrayInfo.num[i]; + if (sz < 0 || sz > int(elements * arrayInfo.element_size)) + { + bufferOverrun(tok, arrayInfo.varname); + } + } + break; + } + } + } + } +} + + void CheckBufferOverrun::checkScope(const Token *tok, const std::vector &varname, const int size, const int total_size, unsigned int varid) { @@ -548,19 +597,12 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector 0) + if (varid == 0 && + (Token::Match(tok, ("%var% ( " + varnames + " ,").c_str()) || + Token::Match(tok, ("%var% ( %var% , " + varnames + " ,").c_str()))) { - - } - else if (Token::Match(tok, ("memset|memcpy|memmove|memcmp|strncpy|fgets ( " + varnames + " , %num% , %num% )").c_str()) || - Token::Match(tok, ("memcpy|memcmp ( %var% , " + varnames + " , %num% )").c_str())) - { - const std::string num = tok->strAt(varc + 6); - if (MathLib::toLongNumber(num) < 0 || MathLib::toLongNumber(num) > total_size) - { - bufferOverrun(tok, varnames); - } - continue; + ArrayInfo arrayInfo(0, varnames, 1, total_size); + checkFunctionCall(tok, arrayInfo); } // Loop.. @@ -867,33 +909,19 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo } - // Writing data into array.. - if (Token::Match(tok, "fgets ( %varid% , %num% , %any% )", arrayInfo.varid) && - MathLib::isInt(tok->strAt(4))) - { - const unsigned long len = MathLib::toLongNumber(tok->strAt(4)); - if (len > total_size) - { - bufferOverrun(tok, arrayInfo.varname); - continue; - } - } + // Check function call.. + if (Token::Match(tok, "%var% ( %varid% ,", arrayInfo.varid) || + Token::Match(tok, "%var% ( %var% , %varid% ,", arrayInfo.varid)) + checkFunctionCall(tok, arrayInfo); - else if (Token::Match(tok, "memset|memcpy|memmove|memcmp|strncpy|fgets ( %varid% , %any% , %any% )", arrayInfo.varid) || - Token::Match(tok, "memset|memcpy|memmove|memcmp|fgets ( %var% , %varid% , %any% )", arrayInfo.varid)) + + if (Token::Match(tok, "memset|memcpy|memmove|memcmp|strncpy|fgets ( %varid% , %any% , %any% )", arrayInfo.varid) || + Token::Match(tok, "memset|memcpy|memmove|memcmp|fgets ( %var% , %varid% , %any% )", arrayInfo.varid)) { const Token *tokSz = tok->tokAt(6); if (tokSz->str()[0] == '\'') sizeArgumentAsChar(tok); - else if (tokSz->isNumber()) - { - const std::string num = tok->strAt(6); - if ((unsigned int)MathLib::toLongNumber(num) > total_size) - { - bufferOverrun(tok, arrayInfo.varname); - } - } if (_settings->_checkCodingStyle) { diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index d35782a26..8f6cc204b 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -158,6 +158,8 @@ public: /** Helper function used when parsing for-loops */ void parse_for_body(const Token *tok2, const ArrayInfo &arrayInfo, const std::string &strindex, bool condition_out_of_bounds, unsigned int counter_varid, const std::string &min_counter_value, const std::string &max_counter_value); + /** Helper function for checkScope - check a function call */ + void checkFunctionCall(const Token *tok2, const ArrayInfo &arrayInfo); /** callstack - used during intra-function checking */ std::list _callStack; diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 148124db1..a8dbb0ee4 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -1840,12 +1840,12 @@ private: { check("void f()\n" "{\n" - " char a[6];\n" - " char c[7];\n" - " strcpy(a,\"hello\");\n" - " strncpy(c,a,sizeof(c));\n" + " char a[6];\n" + " char c[7];\n" + " strcpy(a, \"hello\");\n" + " strncpy(c, a, sizeof(c));\n" "}\n"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:6]: (error) Buffer access out-of-bounds: a\n", errout.str()); check("void f()\n" "{\n" @@ -1874,8 +1874,8 @@ private: check("void f()\n" "{\n" - " char c[6];\n" - " strncpy(c,\"hello!\",sizeof(c)+1);\n" + " char c[6];\n" + " strncpy(c,\"hello!\",sizeof(c)+1);\n" "}\n"); ASSERT_EQUALS("[test.cpp:4]: (error) Buffer access out-of-bounds: c\n", errout.str()); @@ -1884,7 +1884,7 @@ private: "{\n" " strncpy(x, ab->a, 100);\n" "}\n"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Buffer access out-of-bounds: ab.a\n", errout.str()); } void unknownType()