From 5074c11b536c6751c8d9f67edaf0be7124ca8d79 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Mon, 9 Nov 2015 10:30:39 +0100
Subject: [PATCH] CheckBufferOverrun: Fixed FP when accessing string that
 contains '\0'. Refactoring address-of.

---
 lib/checkbufferoverrun.cpp | 22 ++++++++++++----------
 test/testbufferoverrun.cpp | 23 +++++++++++++++++++++++
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
index 89cc7414e..847bdd742 100644
--- a/lib/checkbufferoverrun.cpp
+++ b/lib/checkbufferoverrun.cpp
@@ -259,6 +259,14 @@ void CheckBufferOverrun::negativeMemoryAllocationSizeError(const Token *tok)
 //---------------------------------------------------------------------------
 
 
+static bool isAddressOf(const Token *tok)
+{
+    const Token *tok2 = tok->astParent();
+    while (Token::Match(tok2, "%name%|.|::|["))
+        tok2 = tok2->astParent();
+    return tok2 && tok2->str() == "&" && !(tok2->astOperand1() && tok2->astOperand2());
+}
+
 /**
  * bailout if variable is used inside if/else/switch block or if there is "break"
  * @param tok token for "if" or "switch"
@@ -781,13 +789,7 @@ void CheckBufferOverrun::valueFlowCheckArrayIndex(const Token * const tok, const
     */
     const bool printInconclusive = _settings->inconclusive;
     // Taking address?
-    bool addressOf = false;
-    {
-        const Token *tok2 = tok->astParent();
-        while (Token::Match(tok2, "%name%|.|::|["))
-            tok2 = tok2->astParent();
-        addressOf = tok2 && tok2->str() == "&" && !(tok2->astOperand1() && tok2->astOperand2());
-    }
+    const bool addressOf = isAddressOf(tok);
 
     // Look for errors first
     for (int warn = 0; warn == 0 || warn == 1; ++warn) {
@@ -1068,9 +1070,9 @@ void CheckBufferOverrun::checkGlobalAndLocalVariable()
     // check string literals
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
         if (Token::Match(tok, "%str% [") && tok->next()->astOperand2()) {
-            const std::size_t strLen = Token::getStrLength(tok);
+            const std::size_t size = Token::getStrSize(tok);
             const ValueFlow::Value *value = tok->next()->astOperand2()->getMaxValue(false);
-            if (value && value->intvalue > strLen)
+            if (value && value->intvalue >= (isAddressOf(tok) ? size + 1U : size))
                 bufferOverrunError(tok, tok->str());
         }
 
@@ -1097,7 +1099,7 @@ void CheckBufferOverrun::checkGlobalAndLocalVariable()
                         continue;
 
                     const MathLib::bigint index = arrayInfo.totalIndex(indexes);
-                    if (index <= elements)
+                    if (index < (isAddressOf(tok) ? elements + 1U : elements))
                         continue;
 
                     std::list<const Token *> callstack;
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index a209afa23..30bd04695 100644
--- a/test/testbufferoverrun.cpp
+++ b/test/testbufferoverrun.cpp
@@ -2070,6 +2070,23 @@ private:
               "}");
         ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Array 'a[10]' accessed at index 20, which is out of bounds.\n", errout.str());
 
+        {
+            // address of
+            check("void f() {\n"
+                  "  int a[10];\n"
+                  "  int *p = a;\n"
+                  "  p[10] = 0;\n"
+                  "}");
+            ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Array 'a[10]' accessed at index 10, which is out of bounds.\n", errout.str());
+
+            check("void f() {\n"
+                  "  int a[10];\n"
+                  "  int *p = a;\n"
+                  "  dostuff(&p[10]);\n"
+                  "}");
+            ASSERT_EQUALS("", errout.str());
+        }
+
         check("void f() {\n"
               "  int a[X];\n" // unknown size
               "  int *p = a;\n"
@@ -2447,6 +2464,12 @@ private:
     void buffer_overrun_28() {
         check("char c = \"abc\"[4];");
         ASSERT_EQUALS("[test.cpp:1]: (error) Buffer is accessed out of bounds: \"abc\"\n", errout.str());
+
+        check("p = &\"abc\"[4];");
+        ASSERT_EQUALS("", errout.str());
+
+        check("char c = \"\\0abc\"[2];");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void buffer_overrun_bailoutIfSwitch() {