fix some CheckBufferOverrun::checkStructVariable() flase negatives for possible variable length structs

This commit is contained in:
Robert Reif 2011-09-09 07:46:06 -04:00
parent 4a71693e0c
commit 27bfa2a346
2 changed files with 49 additions and 8 deletions

View File

@ -1544,14 +1544,6 @@ void CheckBufferOverrun::checkStructVariable()
if (arrayInfo.num().size() > 1) if (arrayInfo.num().size() > 1)
continue; continue;
// Skip array with only 0/1 elements because those are
// often overrun intentionally
/** @todo false negatives: only true if last member of struct and
* dynamically allocated with size larger that struct */
/** @todo false negatives: calculate real array size based on allocated size */
if (arrayInfo.num(0) <= 1)
continue;
std::vector<std::string> varname; std::vector<std::string> varname;
varname.push_back(""); varname.push_back("");
varname.push_back(arrayInfo.varname()); varname.push_back(arrayInfo.varname());
@ -1574,6 +1566,23 @@ void CheckBufferOverrun::checkStructVariable()
else else
continue; continue;
// Skip array with only 0/1 elements because those are
// often overrun intentionally
// is variable public struct member?
if (scope->type == Scope::eStruct && var->isPublic())
{
// last member of a struct with array size of 0 or 1 could be a variable struct
if (var->dimensions().size() == 1 && var->dimension(0) < 2 &&
var->index() == (scope->varlist.size() - 1))
{
// dynamically allocated so could be variable sized array
/** @todo false negatives: only true if dynamically allocated with size larger that struct */
/** @todo false negatives: calculate real array size based on allocated size */
if (tok3->next()->str() == "*")
continue;
}
}
// Goto end of statement. // Goto end of statement.
const Token *CheckTok = NULL; const Token *CheckTok = NULL;
while (tok3 && tok3 != func_scope->classEnd) while (tok3 && tok3 != func_scope->classEnd)

View File

@ -515,6 +515,20 @@ private:
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
// This is not out of bounds because it is not a variable length array
check("struct ABC\n"
"{\n"
" char str[1];\n"
" int x;\n"
"};\n"
"\n"
"static void f()\n"
"{\n"
" struct ABC* x = (struct ABC *)malloc(sizeof(struct ABC) + 10);\n"
" x->str[1] = 0;"
"}\n");
ASSERT_EQUALS("[test.cpp:10]: (error) Array 'str[1]' index 1 out of bounds\n", errout.str());
// This is not out of bounds because it is a variable length array // This is not out of bounds because it is a variable length array
// and the index is within the memory allocated. // and the index is within the memory allocated.
/** @todo this works by accident because we ignore any access to this array */ /** @todo this works by accident because we ignore any access to this array */
@ -2416,6 +2430,15 @@ private:
" struct Foo x;\n" " struct Foo x;\n"
" sprintf(x.a, \"aa\");\n" " sprintf(x.a, \"aa\");\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) Buffer access out-of-bounds\n", errout.str());
check("struct Foo { char a[1]; };\n"
"void f()\n"
"{\n"
" struct Foo *x = malloc(10);\n"
" sprintf(x.a, \"aa\");\n"
" free(x);\n"
"}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
@ -2498,6 +2521,15 @@ private:
" struct Foo x;\n" " struct Foo x;\n"
" snprintf(x.a, 2, \"aa\");\n" " snprintf(x.a, 2, \"aa\");\n"
"}\n"); "}\n");
ASSERT_EQUALS("[test.cpp:5]: (error) snprintf size is out of bounds\n", errout.str());
check("struct Foo { char a[1]; };\n"
"void f()\n"
"{\n"
" struct Foo *x = malloc(10);\n"
" snprintf(x.a, 2, \"aa\");\n"
" free(x);\n"
"}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }