From 93ac5a41cdb3c4a8d7e8af06681c64581026c313 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Mon, 22 Dec 2014 09:38:00 +0100
Subject: [PATCH] Fixed #6346 (pointer calculation overflow)

---
 lib/checkbufferoverrun.cpp | 30 ++++++++++++++++++++++++------
 test/testbufferoverrun.cpp | 19 +++++++++++++++++--
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
index 269af706c..5e998c1f4 100644
--- a/lib/checkbufferoverrun.cpp
+++ b/lib/checkbufferoverrun.cpp
@@ -175,7 +175,7 @@ void CheckBufferOverrun::outOfBoundsError(const Token *tok, const std::string &w
 
 void CheckBufferOverrun::pointerOutOfBoundsError(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 " + object + ".\n"
+    reportError(tok, Severity::error, "pointerOutOfBounds", "Undefined behaviour: Pointer arithmetic result does not point into or just past the end of the " + object + ".\n"
                 "Undefined behaviour: The result of this pointer arithmetic does not point into or just one element past the end of the " + 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");
 }
 
@@ -829,7 +829,6 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
 
     const unsigned int declarationId = arrayInfo.declarationId();
 
-    const bool isPortabilityEnabled = _settings->isEnabled("portability");
     const bool isWarningEnabled = _settings->isEnabled("warning");
 
     for (const Token* const end = tok->scope()->classEnd; tok != end; tok = tok->next()) {
@@ -838,13 +837,32 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
                 valueFlowCheckArrayIndex(tok->next(), arrayInfo);
             }
 
-            // undefined behaviour: result of pointer arithmetic is out of bounds
-            else if (isPortabilityEnabled && Token::Match(tok->previous(), "= %varid% + %num% ;", declarationId)) {
-                const MathLib::bigint index = MathLib::toLongNumber(tok->strAt(2));
-                if (index < 0 || index > arrayInfo.num(0)) {
+            else if (tok->astParent() && tok->astParent()->str() == "+") {
+                const ValueFlow::Value *index;
+                if (tok == tok->astParent()->astOperand1())
+                    index = tok->astParent()->astOperand2()->getMaxValue(false);
+                else
+                    index = tok->astParent()->astOperand1()->getMaxValue(false);
+
+                // undefined behaviour: result of pointer arithmetic is out of bounds
+                if (index && (index->intvalue < 0 || index->intvalue > arrayInfo.num(0))) {
                     pointerOutOfBoundsError(tok, "array");
                 }
             }
+
+            else if (tok->astParent() && tok->astParent()->str() == "-") {
+                const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(declarationId);
+                if (var && var->isArray()) {
+                    const Token *index;
+                    if (tok == tok->astParent()->astOperand1())
+                        index = tok->astParent()->astOperand2();
+                    else
+                        index = tok->astParent()->astOperand1();
+
+                    if (index && index->getValueGE(1,_settings))
+                        pointerOutOfBoundsError(tok, "array");
+                }
+            }
         }
 
         else if (!tok->scope()->isExecutable()) // No executable code outside of executable scope - continue to increase performance
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index a7bfc9c0a..db8120e4a 100644
--- a/test/testbufferoverrun.cpp
+++ b/test/testbufferoverrun.cpp
@@ -232,6 +232,7 @@ private:
         // char *p2 = a + 11   // UB
         TEST_CASE(pointer_out_of_bounds_1);
         TEST_CASE(pointer_out_of_bounds_2);
+        TEST_CASE(pointer_out_of_bounds_sub);
 
         TEST_CASE(sprintf1);
         TEST_CASE(sprintf2);
@@ -2947,7 +2948,13 @@ private:
               "    char a[10];\n"
               "    char *p = a + 100;\n"
               "}");
-        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());
+        ASSERT_EQUALS("[test.cpp:3]: (error) Undefined behaviour: Pointer arithmetic result does not point into or just past the end of the array.\n", errout.str());
+
+        check("void f() {\n"
+              "    char a[10];\n"
+              "    return a + 100;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) 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() {
@@ -2956,7 +2963,7 @@ private:
               "    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());
+        ASSERT_EQUALS("[test.cpp:3]: (error) 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"
@@ -2985,6 +2992,14 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void pointer_out_of_bounds_sub() {
+        check("void f() {\n"
+              "    char x[10];\n"
+              "    return x-1;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Undefined behaviour: Pointer arithmetic result does not point into or just past the end of the array.\n", errout.str());
+    }
+
     void sprintf1() {
         check("void f()\n"
               "{\n"