Fixed #2215 (Improve check: Writing outside malloc bounds not detected)

lib/checkbufferoverrun.cpp

@@ -125,9 +125,9 @@ void CheckBufferOverrun::outOfBounds(const Token *tok, const std::string &what)
     reportError(tok, Severity::error, "outOfBounds", what + " is out of bounds");
 }
 
-void CheckBufferOverrun::pointerOutOfBounds(const Token *tok)
+void CheckBufferOverrun::pointerOutOfBounds(const Token *tok, const std::string &object)
 {
-    reportError(tok, Severity::portability, "pointerOutOfBounds", "Undefined behaviour: pointer arithmetic result does not point into or just past the end of the array\n"
+    reportError(tok, Severity::portability, "pointerOutOfBounds", "Undefined behaviour: pointer arithmetic result does not point into or just past the end of the " + object + "\n"
                 "Undefined behaviour: Using pointer arithmetic so that the result does not point into or just past the end of the same object. Further information: https://www.securecoding.cert.org/confluence/display/seccode/ARR30-C.+Do+not+form+or+use+out+of+bounds+pointers+or+array+subscripts");
 }
 
@@ -726,6 +726,10 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
         }
     }
 
+    // If the result of pointer arithmetic means that the pointer is
+    // out of bounds then this flag will be set.
+    bool pointerIsOutOfBounds = false;
+
     int indentlevel = 0;
     for (; tok; tok = tok->next())
     {
@@ -902,6 +906,21 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
         {
             continue;
         }
+
+        // undefined behaviour: result of pointer arithmetic is out of bounds
+        if (Token::Match(tok, "= %varid% + %num% ;", varid))
+        {
+            const MathLib::bigint index = MathLib::toLongNumber(tok->strAt(3));
+            if (index > size && _settings->_checkCodingStyle)
+                pointerOutOfBounds(tok->next(), "buffer");
+            if (index >= size && Token::Match(tok->tokAt(-2), "[;{}] %varid% =", varid))
+                pointerIsOutOfBounds = true;
+        }
+
+        if (pointerIsOutOfBounds && Token::Match(tok, "[;{}=] * %varid% [;=]", varid))
+        {
+            outOfBounds(tok->tokAt(2), tok->strAt(2));
+        }
     }
 }
 
@@ -1156,7 +1175,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
             const MathLib::bigint index = MathLib::toLongNumber(tok->strAt(3));
             if (index < 0 || index > arrayInfo.num[0])
             {
-                pointerOutOfBounds(tok->next());
+                pointerOutOfBounds(tok->next(), "array");
             }
         }
     }

lib/checkbufferoverrun.h

@@ -186,7 +186,7 @@ public:
     void terminateStrncpyError(const Token *tok);
     void negativeIndexError(const Token *tok, MathLib::bigint index);
     void cmdLineArgsError(const Token *tok);
-    void pointerOutOfBounds(const Token *tok);	// UB when result of calculation is out of bounds
+    void pointerOutOfBounds(const Token *tok, const std::string &object);	// UB when result of calculation is out of bounds
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings)
     {
@@ -199,7 +199,7 @@ public:
         c.terminateStrncpyError(0);
         c.negativeIndexError(0, -1);
         c.cmdLineArgsError(0);
-        c.pointerOutOfBounds(0);
+        c.pointerOutOfBounds(0, "array");
     }
 
     std::string name() const

test/testbufferoverrun.cpp

@@ -139,6 +139,7 @@ private:
         // char *p1 = a + 10;  // OK
         // char *p2 = a + 11   // UB
         TEST_CASE(pointer_out_of_bounds_1);
+        TEST_CASE(pointer_out_of_bounds_2);
 
         TEST_CASE(sprintf1);
         TEST_CASE(sprintf2);
@@ -1883,6 +1884,33 @@ private:
         ASSERT_EQUALS("[test.cpp:3]: (portability) Undefined behaviour: pointer arithmetic result does not point into or just past the end of the array\n", errout.str());
     }
 
+    void pointer_out_of_bounds_2()
+    {
+        check("void f() {\n"
+              "    char *p = malloc(10);\n"
+              "    p += 100;\n"
+              "    free(p);"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (portability) Undefined behaviour: pointer arithmetic result does not point into or just past the end of the buffer\n", errout.str());
+
+        check("void f() {\n"
+              "    char *p = malloc(10);\n"
+              "    p += 10;\n"
+              "    *p = 0;\n"
+              "    free(p);"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) p is out of bounds\n", errout.str());
+
+        check("void f() {\n"
+              "    char *p = malloc(10);\n"
+              "    p += 10;\n"
+              "    p = p - 1\n"
+              "    *p = 0;\n"
+              "    free(p);"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void sprintf1()
     {
         check("void f()\n"