calculate array size for variable length structures with array at end in CheckBufferOverrun::checkStructVariable() when possible

lib/checkbufferoverrun.cpp

@@ -1566,18 +1566,17 @@ void CheckBufferOverrun::checkStructVariable()
                             else
                                 continue;
 
-                            // Skip array with only 0/1 elements because those are
+                            // check for variable sized structure
-                            // 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 sized struct
+                                // last member of a struct with array size of 0 or 1 could be a variable sized structure
                                 if (var->dimensions().size() == 1 && var->dimension(0) < 2 &&
                                     var->index() == (scope->varlist.size() - 1))
                                 {
-                                    // dynamically allocated so could be variable sized array
+                                    // dynamically allocated so could be variable sized structure
                                     if (tok3->next()->str() == "*")
                                     {
+                                        // check for allocation
                                         if ((Token::Match(tok3->tokAt(3), "; %var% = malloc ( %num% ) ;") ||
                                              (Token::Match(tok3->tokAt(3), "; %var% = (") &&
                                               Token::Match(tok3->tokAt(6)->link(), ") malloc ( %num% ) ;"))) &&
@@ -1591,15 +1590,29 @@ void CheckBufferOverrun::checkStructVariable()
                                             else
                                                 size = MathLib::toLongNumber(tok3->strAt(8));
 
-                                            if (size != 100) // magic number for size of class or struct
+                                            // We don't calculate the size of a structure even when we know
+                                            // the size of the members.  We just assign a length of 100 for
+                                            // any struct.  If the size is less than 100, we assume the
+                                            // programmer knew the size and specified it rather than using
+                                            // sizeof(struct). If the size is greater than 100, we assume
+                                            // the programmer specified the size as sizeof(struct) + number.
+                                            // Either way, this is just a guess and could be wrong.  The
+                                            // information to make the right decision has been simplified
+                                            // away by the time we get here.
+                                            if (size != 100) // magic number for size of struct
                                             {
-                                                /** @todo false negatives: only true if dynamically allocated with size larger that struct */
+                                                // check if a real size was specified and give up
-                                                /** @todo false negatives: calculate real array size based on allocated size */
+                                                // malloc(10) rather than malloc(sizeof(struct))
-                                                continue;
+                                                if (size < 100)
+                                                    continue;
+
+                                                // calculate real array size based on allocated size
+                                                MathLib::bigint elements = (size - 100) / arrayInfo.element_size();
+                                                arrayInfo.num(0, arrayInfo.num(0) + elements);
                                             }
                                         }
 
-                                        // size unknown so assume it is a dynamically sized struct
+                                        // size unknown so assume it is a variable sized structure
                                         else
                                             continue;
                                     }

lib/checkbufferoverrun.h

@@ -165,6 +165,10 @@ public:
         {
             return _num[index];
         }
+        void num(size_t index, MathLib::bigint number)
+        {
+            _num[index] = number;
+        }
 
         /** size of each element */
         MathLib::bigint element_size() const

test/testbufferoverrun.cpp

@@ -545,7 +545,7 @@ private:
         ASSERT_EQUALS("", errout.str());
 
         // This is out of bounds because it is outside the memory allocated.
-        /** @todo this doesn't work because we ignore any access to this array */
+        /** @todo this doesn't work because of a bug in sizeof(struct)  */
         check("struct ABC\n"
               "{\n"
               "    char str[1];\n"
@@ -558,8 +558,21 @@ private:
               "}\n");
         TODO_ASSERT_EQUALS("[test.cpp:9]: (error) Array 'str[1]' index 11 out of bounds\n", "", errout.str());
 
+        // This is out of bounds because it is outside the memory allocated.
+        check("struct ABC\n"
+              "{\n"
+              "    char str[1];\n"
+              "};\n"
+              "\n"
+              "static void f()\n"
+              "{\n"
+              "    struct ABC* x = (struct ABC *)malloc(sizeof(ABC) + 10);\n"
+              "    x->str[11] = 0;"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:9]: (error) Array 'str[11]' index 11 out of bounds\n", errout.str());
+
         // This is out of bounds because it is outside the memory allocated
-        /** @todo this doesn't work because we ignore any access to this array */
+        /** @todo this doesn't work because of a bug in sizeof(struct) */
         check("struct ABC\n"
               "{\n"
               "    char str[1];\n"
@@ -572,6 +585,19 @@ private:
               "}\n");
         TODO_ASSERT_EQUALS("[test.cpp:9]: (error) Array 'str[1]' index 1 out of bounds\n", "", errout.str());
 
+        // This is out of bounds because it is outside the memory allocated
+        check("struct ABC\n"
+              "{\n"
+              "    char str[1];\n"
+              "};\n"
+              "\n"
+              "static void f()\n"
+              "{\n"
+              "    struct ABC* x = (struct ABC *)malloc(sizeof(ABC));\n"
+              "    x->str[1] = 0;"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:9]: (error) Array 'str[1]' index 1 out of bounds\n", errout.str());
+
         // This is out of bounds because it is not a variable array
         check("struct ABC\n"
               "{\n"