Fixed #2029 (new check: free invalid address)

lib/checkother.cpp

@@ -2409,6 +2409,63 @@ void CheckOther::checkDuplicateBranch()
     }
 }
 
+
+//-----------------------------------------------------------------------------
+// Check for a free() of an invalid address
+// char* p = malloc(100);
+// free(p + 10);
+//-----------------------------------------------------------------------------
+void CheckOther::checkInvalidFree()
+{
+    std::set<unsigned int> allocatedVariables;
+    for (const Token* tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+
+        // Keep track of which variables were assigned addresses to newly-allocated memory
+        if (Token::Match(tok, "%var% = malloc|g_malloc|new")) {
+            allocatedVariables.insert(tok->varId());
+        }
+
+        // If these variables assigned new values before being used to free the memory, we can't
+        // say anything about whether the resulting expression is valid
+        else if (Token::Match(tok, "%var% =")) {
+            allocatedVariables.erase(tok->varId());
+        }
+
+        // If a variable that was previously assigned a newly-allocated memory location is
+        // added or subtracted from when used to free the memory, report an error.
+        else if (Token::Match(tok, "free|g_free|delete ( %any% +|- %any%") ||
+                 Token::Match(tok, "delete [ ] ( %any% +|- %any%") ||
+                 Token::Match(tok, "delete %any% +|- %any%")) {
+
+            int varIdx = tok->strAt(1) == "(" ? 2 :
+                         tok->strAt(3) == "(" ? 4 : 1;
+            unsigned int var1 = tok->tokAt(varIdx)->varId();
+            unsigned int var2 = tok->tokAt(varIdx + 2)->varId();
+            if (allocatedVariables.find(var1) != allocatedVariables.end() ||
+                allocatedVariables.find(var2) != allocatedVariables.end()) {
+                invalidFreeError(tok);
+            }
+        }
+
+        // If the previously-allocated variable is passed in to another function
+        // as a parameter, it might be modified, so we shouldn't report an error
+        // if it is later used to free memory
+        else if (Token::Match(tok, "%var% (")) {
+            const Token* tok2 = Token::findmatch(tok->tokAt(1), "%var%", tok->linkAt(1));
+            while (tok2 != NULL) {
+                allocatedVariables.erase(tok2->varId());
+                tok2 = Token::findmatch(tok2->next(), "%var%", tok->linkAt(1));
+            }
+        }
+    }
+}
+
+void CheckOther::invalidFreeError(const Token *tok)
+{
+    reportError(tok, Severity::error, "invalidFree", "Invalid memory address freed.");
+}
+
+
 //-----------------------------------------------------------------------------
 // Check for double free
 // free(p); free(p);

lib/checkother.h

@@ -106,6 +106,7 @@ public:
 
         checkOther.checkAssignBoolToPointer();
         checkOther.checkBitwiseOnBoolean();
+        checkOther.checkInvalidFree();
         checkOther.checkDoubleFree();
         checkOther.checkRedundantCopy();
         checkOther.checkNegativeBitwiseShift();
@@ -247,6 +248,10 @@ public:
     /** @brief %Check for suspicious use of semicolon */
     void checkSuspiciousSemicolon();
 
+    /** @brief %Check for free() operations on invalid memory locations */
+    void checkInvalidFree();
+    void invalidFreeError(const Token *tok);
+
     /** @brief %Check for double free or double close operations */
     void checkDoubleFree();
     void doubleFreeError(const Token *tok, const std::string &varname);
@@ -407,6 +412,7 @@ private:
                "* sizeof for numeric given as function argument\n"
                "* using sizeof(pointer) instead of the size of pointed data\n"
                "* incorrect length arguments for 'substr' and 'strncmp'\n"
+               "* free() or delete of an invalid memory location\n"
                "* double free() or double closedir()\n"
                "* bitwise operation with negative right operand\n"
 

test/testother.cpp

@@ -162,6 +162,7 @@ private:
         TEST_CASE(checkForSuspiciousSemicolon2);
 
         TEST_CASE(checkDoubleFree);
+        TEST_CASE(checkInvalidFree);
 
         TEST_CASE(checkRedundantCopy);
 
@@ -5864,6 +5865,89 @@ private:
         ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str());
     }
 
+
+    void checkInvalidFree() {
+        check(
+            "void foo(char *p) {\n"
+            "  char *a = (char *)malloc(1024);\n"
+            "  free(a + 10);\n"
+            "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Invalid memory address freed.\n", errout.str());
+
+        check(
+            "void foo(char *p) {\n"
+            "  char *a = (char *)malloc(1024);\n"
+            "  free(a - 10);\n"
+            "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Invalid memory address freed.\n", errout.str());
+
+        check(
+            "void foo(char *p) {\n"
+            "  char *a = (char *)malloc(1024);\n"
+            "  free(10 + a);\n"
+            "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Invalid memory address freed.\n", errout.str());
+
+        check(
+            "void foo(char *p) {\n"
+            "  char *a = new char[1024];\n"
+            "  delete[] (a + 10);\n"
+            "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Invalid memory address freed.\n", errout.str());
+
+        check(
+            "void foo(char *p) {\n"
+            "  char *a = new char;\n"
+            "  delete (a + 10);\n"
+            "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Invalid memory address freed.\n", errout.str());
+
+
+        check(
+            "void foo(char *p) {\n"
+            "  char *a = new char;\n"
+            "  a = a - 10;\n"
+            "  delete (a + 10);\n"
+            "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check(
+            "void foo(char *p) {\n"
+            "  char *a = new char;\n"
+            "  bar(a);\n"
+            "  delete (a + 10);\n"
+            "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check(
+            "void foo(char *p) {\n"
+            "  char *a = new char;\n"
+            "  char *b = new char;\n"
+            "  bar(a);\n"
+            "  delete (a + 10);\n"
+            "  delete (b + 10);\n"
+            "}");
+        ASSERT_EQUALS("[test.cpp:6]: (error) Invalid memory address freed.\n", errout.str());
+
+        check(
+            "void foo(char *p) {\n"
+            "  char *a = new char;\n"
+            "  char *b = new char;\n"
+            "  bar(a, b);\n"
+            "  delete (a + 10);\n"
+            "  delete (b + 10);\n"
+            "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check(
+            "void foo(char *p) {\n"
+            "  char *a = new char;\n"
+            "  bar()\n"
+            "  delete (a + 10);\n"
+            "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Invalid memory address freed.\n", errout.str());
+    }
+
     void check_redundant_copy(const char code[]) {
         // Clear the error buffer..
         errout.str("");