CheckStl: Improving checking of container access out of bounds

lib/checkstl.cpp

@@ -149,6 +149,81 @@ void CheckStl::outOfBoundsError(const Token *tok, const ValueFlow::Value *contai
                 (containerSize && containerSize->isInconclusive()) || (index && index->isInconclusive()));
 }
 
+bool CheckStl::isContainerSize(const Token *containerToken, const Token *expr) const
+{
+    if (!Token::simpleMatch(expr, "( )"))
+        return false;
+    if (!Token::Match(expr->astOperand1(), ". %name% ("))
+        return false;
+    if (!isSameExpression(mTokenizer->isCPP(), false, containerToken, expr->astOperand1()->astOperand1(), mSettings->library, false, false))
+        return false;
+    return containerToken->valueType()->container->getYield(expr->previous()->str()) == Library::Container::Yield::SIZE;
+}
+
+bool CheckStl::isContainerSizeGE(const Token * containerToken, const Token *expr) const
+{
+    if (!expr)
+        return false;
+    if (isContainerSize(containerToken, expr))
+        return true;
+    if (expr->str() == "*") {
+        const Token *mul;
+        if (isContainerSize(containerToken, expr->astOperand1()))
+            mul = expr->astOperand2();
+        else if (isContainerSize(containerToken, expr->astOperand2()))
+            mul = expr->astOperand1();
+        else
+            return false;
+        return mul && (!mul->hasKnownIntValue() || mul->values().front().intvalue != 0);
+    }
+    if (expr->str() == "+") {
+        const Token *op;
+        if (isContainerSize(containerToken, expr->astOperand1()))
+            op = expr->astOperand2();
+        else if (isContainerSize(containerToken, expr->astOperand2()))
+            op = expr->astOperand1();
+        else
+            return false;
+        return op && op->getValueGE(0, mSettings);
+    }
+    return false;
+}
+
+void CheckStl::outOfBoundsIndexExpression()
+{
+    for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) {
+        for (const Token *tok = function->bodyStart; tok != function->bodyEnd; tok = tok->next()) {
+            if (!tok->isName() || !tok->valueType())
+                continue;
+            const Library::Container *container = tok->valueType()->container;
+            if (!container)
+                continue;
+            if (!container->arrayLike_indexOp && !container->stdStringLike)
+                continue;
+            if (!Token::Match(tok, "%name% ["))
+                continue;
+            if (isContainerSizeGE(tok, tok->next()->astOperand2()))
+                outOfBoundsIndexExpressionError(tok, tok->next()->astOperand2());
+        }
+    }
+}
+
+void CheckStl::outOfBoundsIndexExpressionError(const Token *tok, const Token *index)
+{
+    const std::string varname = tok ? tok->str() : std::string("var");
+    const std::string i = index ? index->expressionString() : std::string(varname + ".size()");
+
+    std::string errmsg = "Out of bounds access of $symbol, index '" + i + "' is out of bounds.";
+
+    reportError(tok,
+                Severity::error,
+                "containerOutOfBoundsIndexExpression",
+                "$symbol:" + varname +"\n" + errmsg,
+                CWE398,
+                false);
+}
+
+
 
 // Error message for bad iterator usage..
 void CheckStl::invalidIteratorError(const Token *tok, const std::string &iteratorName)

lib/checkstl.h

@@ -61,6 +61,7 @@ public:
 
         CheckStl checkStl(tokenizer, settings, errorLogger);
         checkStl.outOfBounds();
+        checkStl.outOfBoundsIndexExpression();
     }
 
     /** Simplified checks. The token list is simplified. */
@@ -94,6 +95,9 @@ public:
     /** Accessing container out of bounds using ValueFlow */
     void outOfBounds();
 
+    /** Accessing container out of bounds, following index expression */
+    void outOfBoundsIndexExpression();
+
     /**
      * Finds errors like this:
      * for (unsigned ii = 0; ii <= foo.size(); ++ii)
@@ -185,6 +189,9 @@ public:
     void useStlAlgorithm();
 
 private:
+    bool isContainerSize(const Token *container, const Token *expr) const;
+    bool isContainerSizeGE(const Token * containerToken, const Token *expr) const;
+
     void missingComparisonError(const Token* incrementToken1, const Token* incrementToken2);
     void string_c_strThrowError(const Token* tok);
     void string_c_strError(const Token* tok);
@@ -192,6 +199,7 @@ private:
     void string_c_strParam(const Token* tok, unsigned int number);
 
     void outOfBoundsError(const Token *tok, const ValueFlow::Value *containerSize, const ValueFlow::Value *index);
+    void outOfBoundsIndexExpressionError(const Token *tok, const Token *index);
     void stlOutOfBoundsError(const Token* tok, const std::string& num, const std::string& var, bool at);
     void negativeIndexError(const Token* tok, const ValueFlow::Value& index);
     void invalidIteratorError(const Token* tok, const std::string& iteratorName);

test/teststl.cpp

@@ -41,6 +41,7 @@ private:
         LOAD_LIB_2(settings.library, "std.cfg");
 
         TEST_CASE(outOfBounds);
+        TEST_CASE(outOfBoundsIndexExpression);
 
         TEST_CASE(iterator1);
         TEST_CASE(iterator2);
@@ -262,8 +263,30 @@ private:
                     "    c.data();\n"
                     "}\n");
         ASSERT_EQUALS("", errout.str());
+    }
 
+    void outOfBoundsIndexExpression() {
+        setMultiline();
 
+        checkNormal("void f(std::string s) {\n"
+                    "  s[s.size()] = 1;\n"
+                    "}");
+        ASSERT_EQUALS("test.cpp:2:error:Out of bounds access of s, index 's.size()' is out of bounds.\n", errout.str());
+
+        checkNormal("void f(std::string s) {\n"
+                    "  s[s.size()+1] = 1;\n"
+                    "}");
+        ASSERT_EQUALS("test.cpp:2:error:Out of bounds access of s, index 's.size()+1' is out of bounds.\n", errout.str());
+
+        checkNormal("void f(std::string s) {\n"
+                    "  s[1+s.size()] = 1;\n"
+                    "}");
+        ASSERT_EQUALS("test.cpp:2:error:Out of bounds access of s, index '1+s.size()' is out of bounds.\n", errout.str());
+
+        checkNormal("void f(std::string s) {\n"
+                    "  s[x*s.size()] = 1;\n"
+                    "}");
+        ASSERT_EQUALS("test.cpp:2:error:Out of bounds access of s, index 'x*s.size()' is out of bounds.\n", errout.str());
     }
 
     void iterator1() {