From ec44967e1317df50e4b79148afd99b78ad8e517b Mon Sep 17 00:00:00 2001
From: Pierre Schweitzer <pierre@reactos.org>
Date: Tue, 20 Mar 2012 18:50:05 +0100
Subject: [PATCH] Add a new test to check improper sizeof usage. It's for the
 moment limited to malloc calls.

---
 lib/checkother.cpp | 34 ++++++++++++++++++++++++++++++++++
 lib/checkother.h   |  7 +++++++
 test/testother.cpp | 28 ++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index 7096bff53..7455a90ef 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -562,6 +562,40 @@ void CheckOther::sizeofForStrncmpError(const Token *tok)
                             "compared, which is probably not what was expected.");
 }
 
+void CheckOther::checkSizeofForMallocSize()
+{
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+        if (Token::Match(tok, "[*;{}] %var% = malloc|alloca (")) {
+            const Token *tokVar = tok->tokAt(5);
+            if (Token::Match(tokVar, "sizeof ( %var% ) )") || Token::Match(tokVar, "sizeof %var% )")) {
+                // Get the variable name
+                tokVar = tokVar->next();
+                if (tokVar->str() == "(")
+                    tokVar = tokVar->next();
+
+                // Check if it matches the assignment variable
+                if (tok->tokAt(1)->str() == tokVar->str()) {
+                    sizeofForMallocError(tok->tokAt(1), tok->tokAt(1)->str());
+                }
+            }
+        }
+    }
+
+    // TODO: This test should be renamed into something more generic
+    // and should provide tests for functions like memcpy, memset and so
+    // on. Test would be done the same way. To prevent misuse with arrays
+    // var->isPointer() is to be used.
+}
+
+void CheckOther::sizeofForMallocError(const Token *tok, const std::string &varname)
+{
+    reportInconclusiveError(tok, Severity::warning, "mallocSize",
+                            "Using size of pointer " + varname + " for allocation.\n"
+                            "Using size of pointer " + varname + " for allocation instead "
+                            "of using the size of the type. This is likely to lead to a "
+                            "buffer overflow. You should use sizeof(*" + varname + ")");
+}
+
 //---------------------------------------------------------------------------
 //    switch (x)
 //    {
diff --git a/lib/checkother.h b/lib/checkother.h
index 380a64681..08513b616 100644
--- a/lib/checkother.h
+++ b/lib/checkother.h
@@ -61,6 +61,7 @@ public:
         checkOther.checkAssignmentInAssert();
         checkOther.checkSizeofForArrayParameter();
         checkOther.checkSizeofForStrncmpSize();
+        checkOther.checkSizeofForMallocSize();
         checkOther.checkSizeofForNumericParameter();
         checkOther.checkSelfAssignment();
         checkOther.checkDuplicateIf();
@@ -205,6 +206,9 @@ public:
     /** @brief %Check for using sizeof with a char pointer */
     void checkSizeofForStrncmpSize();
 
+    /** @brief %Check for using sizeof of a variable when allocating it */
+    void checkSizeofForMallocSize();
+
     /** @brief %Check for using sizeof with numeric given as function argument */
     void checkSizeofForNumericParameter();
 
@@ -294,6 +298,7 @@ private:
     void memsetZeroBytesError(const Token *tok, const std::string &varname);
     void sizeofForArrayParameterError(const Token *tok);
     void sizeofForStrncmpError(const Token *tok);
+    void sizeofForMallocError(const Token *tok, const std::string &varname);
     void sizeofForNumericParameterError(const Token *tok);
     void incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string, const std::string &len);
     void incorrectStringBooleanError(const Token *tok, const std::string& string);
@@ -328,6 +333,7 @@ private:
         c.misusedScopeObjectError(NULL, "varname");
         c.sizeofForArrayParameterError(0);
         c.sizeofForStrncmpError(0);
+        c.sizeofForMallocError(0, "varname");
         c.sizeofForNumericParameterError(0);
         c.coutCerrMisusageError(0, "cout");
         c.doubleFreeError(0, "varname");
@@ -396,6 +402,7 @@ private:
                "* assignment in an assert statement\n"
                "* sizeof for array given as function argument\n"
                "* sizeof for numeric given as function argument\n"
+               "* using sizeof(pointer) for its own allocation\n"
                "* incorrect length arguments for 'substr' and 'strncmp'\n"
                "* invalid usage of output stream. For example: std::cout << std::cout;'\n"
                "* wrong number of arguments given to 'printf' or 'scanf;'\n"
diff --git a/test/testother.cpp b/test/testother.cpp
index 58f0b926e..81fad2b80 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -151,6 +151,7 @@ private:
 
         TEST_CASE(alwaysTrueFalseStringCompare);
         TEST_CASE(checkStrncmpSizeof);
+        TEST_CASE(checkMallocSizeof);
         TEST_CASE(checkSignOfUnsignedVariable);
 
         TEST_CASE(checkForSuspiciousSemicolon1);
@@ -4181,6 +4182,33 @@ private:
         ASSERT_EQUALS("[test.cpp:2]: (warning) Passing sizeof(pointer) as the last argument to strncmp.\n", errout.str());
     }
 
+    void checkMallocSizeof() {
+        check(
+            "int *x = malloc(sizeof(*x));\n"
+            "free(x);");
+        ASSERT_EQUALS("", errout.str());
+
+        check(
+            "int *x = malloc(sizeof(int));\n"
+            "free(x);");
+        ASSERT_EQUALS("", errout.str());
+
+        check(
+            "int *x = malloc(sizeof(x));\n"
+            "free(x);");
+        ASSERT_EQUALS("[test.cpp:1]: (warning) Using size of pointer x for allocation.\n", errout.str());
+
+        check(
+            "int *x = malloc(sizeof *x);\n"
+            "free(x);");
+        ASSERT_EQUALS("", errout.str());
+
+        check(
+            "int *x = malloc(sizeof x);\n"
+            "free(x);");
+        ASSERT_EQUALS("[test.cpp:1]: (warning) Using size of pointer x for allocation.\n", errout.str());
+    }
+
     void check_signOfUnsignedVariable(const char code[], bool inconclusive=false) {
         // Clear the error buffer..
         errout.str("");