From 8f4edb5e45aa066ca7652e4971bb32c8d165d5e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 21 Feb 2010 15:23:50 +0100 Subject: [PATCH] Fixed #1409 (False positive: Buffer access out-of-bounds with strncpy and an array in typedef'ed struct) --- lib/checkbufferoverrun.cpp | 32 ++++++++++++++++++-------------- lib/checkbufferoverrun.h | 4 ++-- lib/checkclass.cpp | 2 +- test/testbufferoverrun.cpp | 9 ++++++++- test/testclass.cpp | 2 +- 5 files changed, 30 insertions(+), 19 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 62348bcff..f619a30bc 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -81,7 +81,7 @@ void CheckBufferOverrun::arrayIndexOutOfBounds(int size, int index) reportError(_callStack, severity, "arrayIndexOutOfBounds", "Array index out of bounds"); } -void CheckBufferOverrun::bufferOverrun(const Token *tok) +void CheckBufferOverrun::bufferOverrun(const Token *tok, const std::string &varnames) { Severity::e severity; if (_callStack.size() > 0) @@ -95,7 +95,15 @@ void CheckBufferOverrun::bufferOverrun(const Token *tok) severity = Severity::error; } - reportError(tok, severity, "bufferAccessOutOfBounds", "Buffer access out-of-bounds"); + std::string v = varnames; + while (v.find(" ") != std::string::npos) + v.erase(v.find(" "), 1); + + std::string errmsg("Buffer access out-of-bounds"); + if (!v.empty()) + errmsg += ": " + v; + + reportError(tok, severity, "bufferAccessOutOfBounds", errmsg); } void CheckBufferOverrun::dangerousStdCin(const Token *tok) @@ -273,26 +281,22 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vectorstrAt(6); if (MathLib::toLongNumber(num) < 0 || MathLib::toLongNumber(num) > total_size) { - bufferOverrun(tok); + bufferOverrun(tok, varnames); } } } } - else if (Token::Match(tok, "memset|memcpy|memmove|memcmp|strncpy|fgets")) + 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())) { - if (Token::Match(tok->next(), ("( " + varnames + " , %num% , %num% )").c_str()) || - Token::Match(tok->next(), ("( %var% , " + varnames + " , %num% )").c_str())) + const std::string num = tok->strAt(varc + 6); + if (MathLib::toLongNumber(num) < 0 || MathLib::toLongNumber(num) > total_size) { - const std::string num = tok->strAt(varc + 6); - if (MathLib::toLongNumber(num) < 0 || MathLib::toLongNumber(num) > total_size) - { - bufferOverrun(tok); - } + bufferOverrun(tok, varnames); } continue; } - // Loop.. if (Token::simpleMatch(tok, "for (")) { @@ -503,7 +507,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector 0 ? "" : varnames.c_str()); break; } @@ -556,7 +560,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vectortokAt(varc + 4)); if (len >= static_cast(total_size)) { - bufferOverrun(tok); + bufferOverrun(tok, varid > 0 ? "" : varnames.c_str()); continue; } } diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 997b09c5b..71c23984e 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -83,7 +83,7 @@ private: void arrayIndexOutOfBounds(const Token *tok, int size, int index); void arrayIndexOutOfBounds(int size, int index); - void bufferOverrun(const Token *tok); + void bufferOverrun(const Token *tok, const std::string &varnames = ""); void dangerousStdCin(const Token *tok); void strncatUsage(const Token *tok); void outOfBounds(const Token *tok, const std::string &what); @@ -93,7 +93,7 @@ private: void getErrorMessages() { arrayIndexOutOfBounds(0, 2, 2); - bufferOverrun(0); + bufferOverrun(0, std::string("buffer")); dangerousStdCin(0); strncatUsage(0); outOfBounds(0, "index"); diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index ac6e65631..52af58b6c 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -1490,7 +1490,7 @@ void CheckClass::checkConst() continue; // don't warn if type is LP.. - if (tok2->str().compare(0,2,"LP") == 0) + if (tok2->str().compare(0, 2, "LP") == 0) continue; // member function? diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index dfd6e1917..83262f869 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -953,7 +953,7 @@ private: "{\n" " strcpy( abc->str, \"abcdef\" );\n" "}\n"); - ASSERT_EQUALS("[test.cpp:8]: (error) Buffer access out-of-bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:8]: (error) Buffer access out-of-bounds: abc.str\n", errout.str()); } @@ -1574,6 +1574,13 @@ private: " strncpy(c,\"hello!\",sizeof(c)+1);\n" "}\n"); ASSERT_EQUALS("[test.cpp:4]: (error) Buffer access out-of-bounds\n", errout.str()); + + check("struct AB { char a[10]; };\n" + "void foo(AB *ab)\n" + "{\n" + " strncpy(x, ab->a, 100);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void unknownType() diff --git a/test/testclass.cpp b/test/testclass.cpp index e83367883..fbaa15053 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -1798,7 +1798,7 @@ private: "};\n"); ASSERT_EQUALS("", errout.str()); } - + // A function that returns LPVOID can't be const void constLPVOID() {