From 897e8637b472168d8d75f9620da03856bfb3fc6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 10 Dec 2011 19:26:12 +0100 Subject: [PATCH] Fixed #3168 (false negative: buffer overflow in subfunction) --- lib/checkbufferoverrun.cpp | 31 +++++++++++++++++++++---------- lib/checkbufferoverrun.h | 6 ++++-- test/testbufferoverrun.cpp | 10 ++++++++-- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 6b7cb8637..cdb0da490 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -514,7 +514,7 @@ void CheckBufferOverrun::parse_for_body(const Token *tok2, const ArrayInfo &arra -void CheckBufferOverrun::checkFunctionParameter(const Token &tok, unsigned int par, const ArrayInfo &arrayInfo) +void CheckBufferOverrun::checkFunctionParameter(const Token &tok, unsigned int par, const ArrayInfo &arrayInfo, std::list functionNames) { // total_size : which parameter in function call takes the total size? std::map total_size; @@ -677,21 +677,31 @@ void CheckBufferOverrun::checkFunctionParameter(const Token &tok, unsigned int p } } } + + // Calling function.. + if (Token::Match(ftok, "%var% (")) { + ArrayInfo ai(arrayInfo); + ai.varid(parameterVarId); + checkFunctionCall(ftok, ai, functionNames); + } } } } } -void CheckBufferOverrun::checkFunctionCall(const Token *tok, const ArrayInfo &arrayInfo) +void CheckBufferOverrun::checkFunctionCall(const Token *tok, const ArrayInfo &arrayInfo, std::list functionNames) { + if (std::find(functionNames.begin(), functionNames.end(), tok->str()) != functionNames.end()) + return; + functionNames.push_back(tok->str()); // 1st parameter.. if (Token::Match(tok->tokAt(2), "%varid% ,|)", arrayInfo.varid())) - checkFunctionParameter(*tok, 1, arrayInfo); + checkFunctionParameter(*tok, 1, arrayInfo, functionNames); else if (Token::Match(tok->tokAt(2), "%varid% + %num% ,|)", arrayInfo.varid())) { const ArrayInfo ai(arrayInfo.limit(MathLib::toLongNumber(tok->strAt(4)))); - checkFunctionParameter(*tok, 1, ai); + checkFunctionParameter(*tok, 1, ai, functionNames); } // goto 2nd parameter and check it.. @@ -704,10 +714,10 @@ void CheckBufferOverrun::checkFunctionCall(const Token *tok, const ArrayInfo &ar break; if (tok2->str() == ",") { if (Token::Match(tok2, ", %varid% ,|)", arrayInfo.varid())) - checkFunctionParameter(*tok, 2, arrayInfo); + checkFunctionParameter(*tok, 2, arrayInfo, functionNames); else if (Token::Match(tok2, ", %varid% + %num% ,|)", arrayInfo.varid())) { const ArrayInfo ai(arrayInfo.limit(MathLib::toLongNumber(tok2->strAt(3)))); - checkFunctionParameter(*tok, 2, ai); + checkFunctionParameter(*tok, 2, ai, functionNames); } break; } @@ -901,9 +911,9 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector 0) { if (Token::Match(tok, ("%var% ( " + varnames + " ,").c_str())) - checkFunctionParameter(*tok, 1, arrayInfo); + checkFunctionParameter(*tok, 1, arrayInfo, std::list()); if (Token::Match(tok, ("%var% ( %var% , " + varnames + " ,").c_str())) - checkFunctionParameter(*tok, 2, arrayInfo); + checkFunctionParameter(*tok, 2, arrayInfo, std::list()); } // Loop.. @@ -975,7 +985,8 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector functionNames; + checkFunctionCall(tok, arrayInfo1, functionNames); } // undefined behaviour: result of pointer arithmetic is out of bounds @@ -1099,7 +1110,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo // Check function call.. if (Token::Match(tok, "%var% (")) { - checkFunctionCall(tok, arrayInfo); + checkFunctionCall(tok, arrayInfo, std::list()); } if (Token::Match(tok, "strncpy|memcpy|memmove ( %varid% , %str% , %num% )", arrayInfo.varid())) { diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 0b4539669..1739aeda7 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -203,15 +203,17 @@ public: * \param tok token for the function name * \param par on what parameter is the array used * \param arrayInfo the array information + * \param functionNames function names (to prevent endless recursion) */ - void checkFunctionParameter(const Token &tok, const unsigned int par, const ArrayInfo &arrayInfo); + void checkFunctionParameter(const Token &tok, const unsigned int par, const ArrayInfo &arrayInfo, std::list functionNames); /** * Helper function that checks if the array is used and if so calls the checkFunctionCall * @param tok token that matches "%var% (" * @param arrayInfo the array information + * \param functionNames function names (to prevent endless recursion) */ - void checkFunctionCall(const Token *tok, const ArrayInfo &arrayInfo); + void checkFunctionCall(const Token *tok, const ArrayInfo &arrayInfo, std::list functionNames); void arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector &index); void arrayIndexOutOfBoundsError(const std::list &callstack, const ArrayInfo &arrayInfo, const std::vector &index); diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 202ac5147..85054fcb5 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -33,8 +33,6 @@ public: private: - - void check(const char code[], bool experimental = true, const std::string &filename="test.cpp") { // Clear the error buffer.. errout.str(""); @@ -724,6 +722,14 @@ private: " foo(p+1);\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + // #3168 + check("void a(char *p) { memset(p,0,100); }\n" + "void b() {\n" + " char buf[10];\n" + " a(buf);" + "}"); + ASSERT_EQUALS("[test.cpp:1]: (error) Buffer access out-of-bounds: buf\n", errout.str()); }