From 4ec9d418ffd574f1939f41d4b1533bda9fbd384d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <danielm77@spray.se>
Date: Sat, 1 Jan 2011 20:56:21 +0100
Subject: [PATCH] Fixed #2215 (Improve check: Writing outside malloc bounds not
 detected)

---
 lib/checkbufferoverrun.cpp | 25 ++++++++++++++++++++++---
 lib/checkbufferoverrun.h   |  4 ++--
 test/testbufferoverrun.cpp | 28 ++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
index e86c4f55a..bfeb4c4f7 100644
--- a/lib/checkbufferoverrun.cpp
+++ b/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");
             }
         }
     }
diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h
index b8e6444dd..81006cc99 100644
--- a/lib/checkbufferoverrun.h
+++ b/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
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index 1f53b6d3b..5fd547f4c 100644
--- a/test/testbufferoverrun.cpp
+++ b/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"