From a3b781a181150fd1b8f123e48a31792887b2b2e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 23 Apr 2010 16:26:40 +0200 Subject: [PATCH] Fixed #819 (array index out of bounds not detected for multidimension arrays) --- lib/checkbufferoverrun.cpp | 59 ++++++++++++++++++++++++++++++++------ lib/checkbufferoverrun.h | 2 +- test/testbufferoverrun.cpp | 21 ++++---------- 3 files changed, 57 insertions(+), 25 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 28a5578c1..7b2ac7109 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -82,13 +82,22 @@ void CheckBufferOverrun::arrayIndexOutOfBounds(int size, int index) reportError(_callStack, severity, "arrayIndexOutOfBounds", "Array index out of bounds"); } -void CheckBufferOverrun::arrayIndexOutOfBounds(const Token *tok, const ArrayInfo &arrayInfo, int index) +void CheckBufferOverrun::arrayIndexOutOfBounds(const Token *tok, const ArrayInfo &arrayInfo, const std::vector &index) { std::ostringstream oss; oss << "Array '" << arrayInfo.varname; for (unsigned int i = 0; i < arrayInfo.num.size(); ++i) oss << "[" << arrayInfo.num[i] << "]"; - oss << "' index " << index << " out of bounds"; + oss << "' index "; + if (index.size() == 1) + oss << index[0]; + else + { + oss << arrayInfo.varname; + for (unsigned int i = 0; i < index.size(); ++i) + oss << "[" << index[i] << "]"; + } + oss << " out of bounds"; reportError(tok, Severity::error, "arrayIndexOutOfBounds", oss.str().c_str()); } @@ -860,17 +869,49 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo else if (Token::Match(tok, "%varid% [ %num% ]", arrayInfo.varid)) { - const int index = MathLib::toLongNumber(tok->strAt(2)); - if (index >= (int)arrayInfo.num[0]) + std::vector indexes; + for (const Token *tok2 = tok->next(); Token::Match(tok2, "[ %num% ]"); tok2 = tok2->tokAt(3)) { + const int index = MathLib::toLongNumber(tok2->strAt(1)); + if (index < 0) + { + indexes.clear(); + break; + } + indexes.push_back(index); + } + if (indexes.size() == arrayInfo.num.size()) + { + // Check if the indexes point outside the whole array.. + // char a[10][10]; + // a[0][20] <-- ok. + // a[9][20] <-- error. + + // total number of elements of array.. + unsigned int totalElements = 1; + + // total index.. + unsigned int totalIndex = 0; + + // calculate the totalElements and totalIndex.. + for (unsigned int i = 0; i < indexes.size(); ++i) + { + unsigned int ri = indexes.size() - 1 - i; + totalIndex += indexes[ri] * totalElements; + totalElements *= arrayInfo.num[ri]; + } + // just taking the address? const bool addr(Token::Match(tok->previous(), "[.&]") || Token::simpleMatch(tok->tokAt(-2), "& (")); - // it's ok to take the address of the element past the array - if (!(addr && index == (int)arrayInfo.num[0])) - arrayIndexOutOfBounds(tok, arrayInfo, index); + // Is totalIndex in bounds? + if (totalIndex > totalElements || (!addr && totalIndex == totalElements)) + { + arrayIndexOutOfBounds(tok, arrayInfo, indexes); + } } + } // cin.. @@ -1688,7 +1729,9 @@ private: CheckBufferOverrun *checkBufferOverrun = dynamic_cast(c->owner); if (checkBufferOverrun) { - checkBufferOverrun->arrayIndexOutOfBounds(tok, ai, c->value); + std::vector index; + index.push_back(c->value); + checkBufferOverrun->arrayIndexOutOfBounds(tok, ai, index); break; } } diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 4e2201317..b64b95644 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -171,7 +171,7 @@ public: void arrayIndexOutOfBounds(const Token *tok, int size, int index); void arrayIndexOutOfBounds(int size, int index); - void arrayIndexOutOfBounds(const Token *tok, const ArrayInfo &arrayInfo, int index); + void arrayIndexOutOfBounds(const Token *tok, const ArrayInfo &arrayInfo, const std::vector &index); void bufferOverrun(const Token *tok, const std::string &varnames = ""); void dangerousStdCin(const Token *tok); void strncatUsage(const Token *tok); diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index d9b1fb92d..9b5598f44 100755 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -922,46 +922,35 @@ private: " char a[2][2];\n" " a[2][1] = 'a';\n" "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2]' index 2 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2]' index a[2][1] out of bounds\n", errout.str()); check("void f()\n" "{\n" " char a[2][2];\n" " a[1][2] = 'a';\n" "}\n"); - ASSERT_EQUALS("", errout.str()); // catch changes - TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2]' index 2 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2]' index a[1][2] out of bounds\n", errout.str()); check("void f()\n" "{\n" " char a[2][2][2];\n" " a[2][1][1] = 'a';\n" "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' index 2 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' index a[2][1][1] out of bounds\n", errout.str()); check("void f()\n" "{\n" " char a[2][2][2];\n" " a[1][2][1] = 'a';\n" "}\n"); - ASSERT_EQUALS("", errout.str()); // catch changes - TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Array index out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' index a[1][2][1] out of bounds\n", errout.str()); check("void f()\n" "{\n" " char a[2][2][2];\n" " a[1][1][2] = 'a';\n" "}\n"); - ASSERT_EQUALS("", errout.str()); // catch changes - TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Array index out of bounds\n", errout.str()); - - check("void f()\n" - "{\n" - " char a[2][2][2];\n" - " a[1][1][2] = 'a';\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); // catch changes - TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Array index out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[2][2][2]' index a[1][1][2] out of bounds\n", errout.str()); } void array_index_switch_in_for()