fix #3094 (Buffer access out-of-bounds in struct variable)

lib/checkbufferoverrun.cpp

@@ -1500,69 +1500,55 @@ void CheckBufferOverrun::checkStructVariable()
         if (!scope->isClassOrStruct())
             continue;
 
-        // check all variables
+        // check all variables to see if they are arrays
         std::list<Variable>::const_iterator var;
         for (var = scope->varlist.begin(); var != scope->varlist.end(); ++var)
         {
             // find all array variables
             if (var->isArray())
             {
+                // create ArrayInfo from the array variable
                 ArrayInfo arrayInfo(&*var, _tokenizer);
 
-                // check each function for array variable usage
+                std::list<Scope>::const_iterator func_scope;
-                std::list<Function>::const_iterator func;
+
-                for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func)
+                // find every function
+                for (func_scope = symbolDatabase->scopeList.begin(); func_scope != symbolDatabase->scopeList.end(); ++func_scope)
                 {
-                    // check existing and non-empty function
+                    // only check functions
-                    if (func->hasBody && func->start->next() != func->start->link())
+                    if (func_scope->type != Scope::eFunction)
+                        continue;
+
+                    // is this a member function of this class/struct?
+                    if (func_scope->functionOf == &*scope)
                     {
-                        const Token *tok = func->start->next();
+                        // only check non-empty function
+                        if (func_scope->function->start->next() != func_scope->function->start->link())
+                        {
+                            // start checking after the {
+                            const Token *tok = func_scope->function->start->next();
                             checkScope(tok, arrayInfo);
                         }
                     }
-            }
-        }
-    }
 
-    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
+                    // not a member function of this class/struct
-    {
+                    else
-        if (tok->str() == "{")
-        {
-            tok = tok->link();
-        }
-
-        if (!Token::Match(tok, "struct|class %var% {|:"))
-            continue;
-
-        const std::string &structname = tok->next()->str();
-        const Token *tok2 = tok;
-
-        while (tok2 && tok2->str() != "{")
-            tok2 = tok2->next();
-
-        // Found a struct declaration. Search for arrays..
-        for (; tok2; tok2 = tok2->next())
                     {
                         // skip inner scopes..
-            if (tok2->next() && tok2->next()->str() == "{")
+                        /** @todo false negatives: handle inner scopes someday */
-            {
+                        if (scope->nestedIn->isClassOrStruct())
-                tok2 = tok2->next()->link();
-                continue;
-            }
-
-            if (tok2->str() == "}")
-                break;
-
-            ArrayInfo arrayInfo;
-            if (!arrayInfo.declare(tok2->next(), *_tokenizer))
                             continue;
 
                         // Only handling 1-dimensional arrays yet..
+                        /** @todo false negatives: handle multi-dimension arrays someday */
                         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;
 
@@ -1570,9 +1556,11 @@ void CheckBufferOverrun::checkStructVariable()
                         varname.push_back("");
                         varname.push_back(arrayInfo.varname());
 
-            for (const Token *tok3 = _tokenizer->tokens(); tok3; tok3 = tok3->next())
+                        // search the function and it's parameters
+                        for (const Token *tok3 = func_scope->classDef; tok3 && tok3 != func_scope->classEnd; tok3 = tok3->next())
                         {
-                if (tok3->str() != structname)
+                            // search for the class/struct name
+                            if (tok3->str() != scope->className)
                                 continue;
 
                             // Declare variable: Fred fred1;
@@ -1588,7 +1576,7 @@ void CheckBufferOverrun::checkStructVariable()
 
                             // Goto end of statement.
                             const Token *CheckTok = NULL;
-                while (tok3)
+                            while (tok3 && tok3 != func_scope->classEnd)
                             {
                                 // End of statement.
                                 if (tok3->str() == ";")
@@ -1623,6 +1611,9 @@ void CheckBufferOverrun::checkStructVariable()
                     }
                 }
             }
+        }
+    }
+}
 //---------------------------------------------------------------------------
 
 void CheckBufferOverrun::bufferOverrun()

test/testbufferoverrun.cpp

@@ -1779,7 +1779,7 @@ private:
               "    } abc;\n"
               "    strcpy( abc.str, \"abcdef\" );\n"
               "}\n");
-        TODO_ASSERT_EQUALS("[test.cpp:7]: (error) Buffer access out-of-bounds: abc.str\n", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:7]: (error) Buffer access out-of-bounds: abc.str\n", errout.str());
 
         check("static void f()\n"
               "{\n"
@@ -1791,7 +1791,7 @@ private:
               "    strcpy( abc->str, \"abcdef\" );\n"
               "    free(abc);\n"
               "}\n");
-        TODO_ASSERT_EQUALS("[test.cpp:8]: (error) Buffer access out-of-bounds: abc.str\n", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:8]: (error) Buffer access out-of-bounds: abc.str\n", errout.str());
     }
 
 
@@ -3322,6 +3322,18 @@ private:
               "    x.buf[10] = 0;\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        check("class A {\n"
+              "public:\n"
+              "    struct X { char buf[10]; };\n"
+              "}\n"
+              "\n"
+              "void f()\n"
+              "{\n"
+              "    A::X x;\n"
+              "    x.buf[10] = 0;\n"
+              "}\n");
+        TODO_ASSERT_EQUALS("[test.cpp:9]: (error) Array 'x.buf[10]' index 10 out of bounds\n", "", errout.str());
     }
 
     void getErrorMessages()