diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 93665682e..a8ae96145 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -1544,14 +1544,6 @@ void CheckBufferOverrun::checkStructVariable() if (arrayInfo.num().size() > 1) 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 varname; varname.push_back(""); varname.push_back(arrayInfo.varname()); @@ -1574,6 +1566,23 @@ void CheckBufferOverrun::checkStructVariable() else 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. const Token *CheckTok = NULL; while (tok3 && tok3 != func_scope->classEnd) diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 92c74d15e..d0566669a 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -515,6 +515,20 @@ private: "}\n"); 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 // and the index is within the memory allocated. /** @todo this works by accident because we ignore any access to this array */ @@ -2416,6 +2430,15 @@ private: " struct Foo x;\n" " sprintf(x.a, \"aa\");\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()); } @@ -2498,6 +2521,15 @@ private: " struct Foo x;\n" " snprintf(x.a, 2, \"aa\");\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()); }