From b6ab419a0684a46d67db2c850a0b320f6fe5fbb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 18 Apr 2010 20:51:39 +0200 Subject: [PATCH] Buffer Overrun: Broke out the checking for negative array index --- lib/checkbufferoverrun.cpp | 27 ++++++++++++++++++++++++--- lib/checkbufferoverrun.h | 6 ++++++ test/testbufferoverrun.cpp | 34 ++++++++++++++++++---------------- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index e19494036..21bd2d776 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -210,7 +210,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vectorstrAt(2)); - if (index < 0 || index >= size) + if (index >= size) { arrayIndexOutOfBounds(tok, size, index); } @@ -219,7 +219,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vectorstrAt(2 + varc)); - if (index < 0 || index >= size) + if (index >= size) { arrayIndexOutOfBounds(tok->tokAt(varc), size, index); } @@ -264,7 +264,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vectorisName() && !Token::Match(tok, "[.&]") && Token::Match(tok->next(), (varnames + " [ %num% ]").c_str())) { int index = MathLib::toLongNumber(tok->strAt(3 + varc)); - if (index < 0 || index >= size) + if (index >= size) { arrayIndexOutOfBounds(tok->tokAt(1 + varc), size, index); } @@ -1265,6 +1265,27 @@ void CheckBufferOverrun::checkSprintfCall(const Token *tok, int size) } +void CheckBufferOverrun::negativeIndexError(const Token *tok, long index) +{ + std::ostringstream ostr; + ostr << "Array index " << index << " corresponds with " << (unsigned long)index << ", which is likely out of bounds"; + reportError(tok, Severity::error, "negativeIndex", ostr.str()); +} + +void CheckBufferOverrun::negativeIndex() +{ + const char pattern[] = "[ %num% ]"; + for (const Token *tok = Token::findmatch(_tokenizer->tokens(), pattern); tok; tok = Token::findmatch(tok->next(),pattern)) + { + const long index = MathLib::toLongNumber(tok->next()->str()); + if (index < 0) + { + negativeIndexError(tok, index); + } + } +} + + #include "executionpath.h" diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 41e6a72ef..95d1e1a4c 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -61,6 +61,7 @@ public: { CheckBufferOverrun checkBufferOverrun(tokenizer, settings, errorLogger); checkBufferOverrun.bufferOverrun(); + checkBufferOverrun.negativeIndex(); /** ExecutionPath checking.. */ checkBufferOverrun.executionPaths(); @@ -94,6 +95,9 @@ public: /** Check for buffer overruns - locate global variables and local function variables and check them with the checkScope function */ void checkGlobalAndLocalVariable(); + /** Check for negative index */ + void negativeIndex(); + /** Check for buffer overruns - this is the function that performs the actual checking */ void checkScope(const Token *tok, const std::vector &varname, const int size, const int total_size, unsigned int varid); @@ -152,6 +156,7 @@ public: void outOfBounds(const Token *tok, const std::string &what); void sizeArgumentAsChar(const Token *tok); void terminateStrncpyError(const Token *tok); + void negativeIndexError(const Token *tok, long index); void getErrorMessages() { @@ -162,6 +167,7 @@ public: outOfBounds(0, "index"); sizeArgumentAsChar(0); terminateStrncpyError(0); + negativeIndexError(0, -1); } std::string name() const diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index b3721d261..dfc46bf51 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -58,6 +58,7 @@ private: settings._checkCodingStyle = true; CheckBufferOverrun checkBufferOverrun(&tokenizer, &settings, this); checkBufferOverrun.bufferOverrun(); + checkBufferOverrun.negativeIndex(); } void run() @@ -780,8 +781,9 @@ private: " a[-1] = 0;\n" // negative index " a[128] = 0;\n" // 128 > CHAR_MAX "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[128]' index -1 out of bounds\n" - "[test.cpp:4]: (error) Array 'a[128]' index 128 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[128]' index 128 out of bounds\n" + "[test.cpp:3]: (error) Array index -1 corresponds with 4294967295, which is likely out of bounds\n", errout.str()); + } else // plain char is unsigned { @@ -799,40 +801,40 @@ private: " a[-1] = 0;\n" // negative index " a[128] = 0;\n" // 128 > SCHAR_MAX "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[128]' index -1 out of bounds\n" - "[test.cpp:4]: (error) Array 'a[128]' index 128 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[128]' index 128 out of bounds\n" + "[test.cpp:3]: (error) Array index -1 corresponds with 4294967295, which is likely out of bounds\n", errout.str()); check("void f(unsigned char n) {\n" " int a[n];\n" // n <= UCHAR_MAX " a[-1] = 0;\n" // negative index " a[256] = 0;\n" // 256 > UCHAR_MAX "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[256]' index -1 out of bounds\n" - "[test.cpp:4]: (error) Array 'a[256]' index 256 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[256]' index 256 out of bounds\n" + "[test.cpp:3]: (error) Array index -1 corresponds with 4294967295, which is likely out of bounds\n", errout.str()); check("void f(short n) {\n" " int a[n];\n" // n <= SHRT_MAX " a[-1] = 0;\n" // negative index " a[32768] = 0;\n" // 32768 > SHRT_MAX "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[32768]' index -1 out of bounds\n" - "[test.cpp:4]: (error) Array 'a[32768]' index 32768 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[32768]' index 32768 out of bounds\n" + "[test.cpp:3]: (error) Array index -1 corresponds with 4294967295, which is likely out of bounds\n", errout.str()); check("void f(unsigned short n) {\n" " int a[n];\n" // n <= USHRT_MAX " a[-1] = 0;\n" // negative index " a[65536] = 0;\n" // 65536 > USHRT_MAX "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[65536]' index -1 out of bounds\n" - "[test.cpp:4]: (error) Array 'a[65536]' index 65536 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[65536]' index 65536 out of bounds\n" + "[test.cpp:3]: (error) Array index -1 corresponds with 4294967295, which is likely out of bounds\n", errout.str()); check("void f(signed short n) {\n" " int a[n];\n" // n <= SHRT_MAX " a[-1] = 0;\n" // negative index " a[32768] = 0;\n" // 32768 > SHRT_MAX "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[32768]' index -1 out of bounds\n" - "[test.cpp:4]: (error) Array 'a[32768]' index 32768 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[32768]' index 32768 out of bounds\n" + "[test.cpp:3]: (error) Array index -1 corresponds with 4294967295, which is likely out of bounds\n", errout.str()); if (sizeof(int) == 4) { @@ -840,19 +842,19 @@ private: " int a[n];\n" // n <= INT_MAX " a[-1] = 0;\n" // negative index "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[2147483647]' index -1 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 corresponds with 4294967295, which is likely out of bounds\n", errout.str()); check("void f(unsigned int n) {\n" " int a[n];\n" // n <= UINT_MAX " a[-1] = 0;\n" // negative index "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[2147483647]' index -1 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 corresponds with 4294967295, which is likely out of bounds\n", errout.str()); check("void f(signed int n) {\n" " int a[n];\n" // n <= INT_MAX " a[-1] = 0;\n" // negative index "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[2147483647]' index -1 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 corresponds with 4294967295, which is likely out of bounds\n", errout.str()); } } @@ -1012,7 +1014,7 @@ private: " char data[8];\n" " data[-1] = 0;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:4]: (error) Array 'data[8]' index -1 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array index -1 corresponds with 4294967295, which is likely out of bounds\n", errout.str()); } void array_index_for_decr()