From b0eab2587d390e503787ef7d3da71e2548656a38 Mon Sep 17 00:00:00 2001
From: Robert Reif <reif@earthlink.net>
Date: Fri, 9 Sep 2011 08:37:24 -0400
Subject: [PATCH] better detection of variable sized structure in
 CheckBufferOverrun::checkStructVariable()

---
 lib/checkbufferoverrun.cpp | 31 +++++++++++++++++++++++++++----
 test/testbufferoverrun.cpp | 22 ++++++++++++++++++++--
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
index a8ae96145..c86f08abd 100644
--- a/lib/checkbufferoverrun.cpp
+++ b/lib/checkbufferoverrun.cpp
@@ -1571,15 +1571,38 @@ void CheckBufferOverrun::checkStructVariable()
                             // 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
+                                // last member of a struct with array size of 0 or 1 could be a variable sized 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;
+                                    {
+                                        if ((Token::Match(tok3->tokAt(3), "; %var% = malloc ( %num% ) ;") ||
+                                             (Token::Match(tok3->tokAt(3), "; %var% = (") &&
+                                              Token::Match(tok3->tokAt(6)->link(), ") malloc ( %num% ) ;"))) &&
+                                            (tok3->strAt(4) == tok3->strAt(2)))
+                                        {
+                                            MathLib::bigint size;
+
+                                            // find size of allocation
+                                            if (tok3->strAt(3) == "(") // has cast
+                                                size = MathLib::toLongNumber(tok3->tokAt(6)->link()->strAt(3));
+                                            else
+                                                size = MathLib::toLongNumber(tok3->strAt(8));
+
+                                            if (size != 100) // magic number for size of class or struct
+                                            {
+                                                /** @todo false negatives: only true if dynamically allocated with size larger that struct */
+                                                /** @todo false negatives: calculate real array size based on allocated size */
+                                                continue;
+                                            }
+                                        }
+
+                                        // size unknown so assume it is a dynamically sized struct
+                                        else
+                                            continue;
+                                    }
                                 }
                             }
 
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index d0566669a..edec4ecb6 100644
--- a/test/testbufferoverrun.cpp
+++ b/test/testbufferoverrun.cpp
@@ -2435,7 +2435,16 @@ private:
         check("struct Foo { char a[1]; };\n"
               "void f()\n"
               "{\n"
-              "  struct Foo *x = malloc(10);\n"
+              "  struct Foo *x = malloc(sizeof(Foo));\n"
+              "  sprintf(x.a, \"aa\");\n"
+              "  free(x);\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(sizeof(Foo) + 10);\n"
               "  sprintf(x.a, \"aa\");\n"
               "  free(x);\n"
               "}\n");
@@ -2526,7 +2535,16 @@ private:
         check("struct Foo { char a[1]; };\n"
               "void f()\n"
               "{\n"
-              "  struct Foo *x = malloc(10);\n"
+              "  struct Foo *x = malloc(sizeof(Foo));\n"
+              "  snprintf(x.a, 2, \"aa\");\n"
+              "  free(x);\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(sizeof(Foo) + 10);\n"
               "  snprintf(x.a, 2, \"aa\");\n"
               "  free(x);\n"
               "}\n");