walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Buffer Overrun: Broke out the checking for negative array index

This commit is contained in:
Daniel Marjamäki 2010-04-18 20:51:39 +02:00
parent af3f2faa41
commit b6ab419a06
3 changed files with 48 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
lib/checkbufferoverrun.cpp
View File

@ -210,7 +210,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
if (Token::Match(tok, "%varid% [ %num% ]", varid))
{
int index = MathLib::toLongNumber(tok->strAt(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::vector<std::str
else if (Token::Match(tok, (varnames + " [ %num% ]").c_str()))
{
int index = MathLib::toLongNumber(tok->strAt(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::vector<std::str
else if (!tok->isName() && !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"

6
lib/checkbufferoverrun.h
View File

@ -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<std::string> &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

34
test/testbufferoverrun.cpp
View File

@ -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()