Fixed #4096 (Improve check: Buffer overrun in for loop, postfix increment in array access)

lib/checkbufferoverrun.cpp

@@ -796,6 +796,54 @@ void CheckBufferOverrun::checkScopeForBody(const Token *tok, const ArrayInfo &ar
     parse_for_body(tok2->next(), arrayInfo, counter_name, condition_out_of_bounds, counter_varid, min_counter_value, max_counter_value);
 }
 
+void CheckBufferOverrun::arrayIndexInForLoop(const Token *tok, const ArrayInfo &arrayInfo)
+{
+    const MathLib::bigint size = arrayInfo.num(0);
+    const Token *tok3 = tok->tokAt(2);
+    std::string counter_name;
+    unsigned int counter_varid = 0;
+    std::string counter_init_value;
+
+    tok3 = for_init(tok3, counter_varid, counter_name, counter_init_value);
+
+    bool maxMinFlipped = false;
+    std::string min_counter_value = counter_init_value;
+    std::string max_counter_value;
+    MathLib::bigint max_value = MathLib::toLongNumber(max_counter_value);
+
+    for_condition(tok3, counter_varid, min_counter_value, max_counter_value, maxMinFlipped);
+    while (tok3 && tok3->str() != ";") {
+        tok3 = tok3->next();
+    }
+
+    for (const Token* tok2 = tok; tok2; tok2 = tok2->next()) {
+        if (Token::Match(tok2, "%var% < %num%")) {
+            max_value = MathLib::toLongNumber(tok2->strAt(2));
+            max_value = max_value - 1;
+        }
+    }
+
+    bool usedInArray = false;
+
+    if (max_value > size) {
+        if (tok3->strAt(1) == ")") {
+
+            for (const Token *loopTok = tok3->tokAt(2); loopTok->str() != "}" ; loopTok = loopTok->next()) {
+                if (loopTok->varId() == arrayInfo.varid() && loopTok->tokAt(2)->varId() == counter_varid)
+                    usedInArray = true;
+            }
+
+            for (const Token *loopTok = tok3->tokAt(2); loopTok->str() != "}" ; loopTok = loopTok->next()) {
+                if (usedInArray && (counter_varid == loopTok->varId())) {
+                    if (Token::Match(loopTok->next(), " ++ ") ||
+                        (loopTok->previous()->type() == Token::eIncDecOp)) {
+                        bufferOverrunError(tok, arrayInfo.varname());
+                    }
+                }
+            }
+        }
+    }
+}
 
 void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::string> &varname, const ArrayInfo &arrayInfo)
 {
@@ -1102,6 +1150,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
         // Loop..
         else if (Token::simpleMatch(tok, "for (")) {
             bool bailout = false;
+            arrayIndexInForLoop(tok, arrayInfo);
             checkScopeForBody(tok, arrayInfo, bailout);
             if (bailout)
                 break;

lib/checkbufferoverrun.h

@@ -209,6 +209,7 @@ public:
     void checkFunctionCall(const Token *tok, const ArrayInfo &arrayInfo, std::list<const Token *> callstack);
 
     void arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector<MathLib::bigint> &index);
+    void arrayIndexInForLoop(const Token *tok, const ArrayInfo &arrayInfo);
 private:
     void arrayIndexOutOfBoundsError(const std::list<const Token *> &callstack, const ArrayInfo &arrayInfo, const std::vector<MathLib::bigint> &index);
     void bufferOverrunError(const Token *tok, const std::string &varnames = "");

test/testbufferoverrun.cpp

@@ -154,6 +154,8 @@ private:
         TEST_CASE(buffer_overrun_21);
         TEST_CASE(buffer_overrun_22); // #3124
         TEST_CASE(buffer_overrun_23); // #3153
+        TEST_CASE(buffer_overrun_24); // #4106
+        TEST_CASE(buffer_overrun_25); // #4096
         TEST_CASE(buffer_overrun_bailoutIfSwitch);  // ticket #2378 : bailoutIfSwitch
         TEST_CASE(possible_buffer_overrun_1); // #3035
 
@@ -2534,6 +2536,38 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+
+    void buffer_overrun_24() { // ticket #4106
+        check("void main() {\n"
+              "   int array[] = {1,2};\n"
+              "   int x = 0;\n"
+              "   for( int i = 0; i<6; ) { \n"
+              "      x += array[i];\n"
+              "       i++;  }\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: array\n", errout.str());
+
+        check("void main() {\n"
+              "   int array[] = {1,2};\n"
+              "   int x = 0;\n"
+              "   for( int i = 0; i<6; ) { \n"
+              "       i++;  }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void buffer_overrun_25() { // ticket #4096
+        check("void main() {\n"
+              "   int array[] = {1,2};\n"
+              "   int x = 0;\n"
+              "   for( int i = 0; i<6; ) { \n"
+              "      x += array[i++];\n"
+              "         }\n"
+              "}\n");
+
+        ASSERT_EQUALS("[test.cpp:4]: (error) Buffer is accessed out of bounds: array\n", errout.str());
+    }
+
     void buffer_overrun_bailoutIfSwitch() {
         // No false positive
         check("void f1(char *s) {\n"