From 7b793af451e933f65de9ace4a9fb3ec7df2e9720 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Tue, 18 Jan 2022 07:48:02 -0600 Subject: [PATCH] Fix 10728: Crash in CheckStl::checkDereferenceInvalidIterator2 (#3721) * Fix 10728: Crash in CheckStl::checkDereferenceInvalidIterator2 * Format --- lib/checkbufferoverrun.cpp | 2 +- lib/checkstl.cpp | 8 +++++--- lib/token.cpp | 2 +- lib/valueflow.cpp | 10 ++++++---- lib/valueflow.h | 4 +++- test/teststl.cpp | 13 +++++++++++++ 6 files changed, 29 insertions(+), 10 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index a9b6179de..ec1102aaf 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -994,7 +994,7 @@ void CheckBufferOverrun::objectIndex() if (idx->hasKnownIntValue() && idx->getKnownIntValue() == 0) continue; - std::vector values = getLifetimeObjValues(obj, false, true); + std::vector values = getLifetimeObjValues(obj, false, -1); for (const ValueFlow::Value& v:values) { if (v.lifetimeKind != ValueFlow::Value::LifetimeKind::Address) continue; diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index b5bc660da..786529dc7 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -728,9 +728,9 @@ static bool isSameIteratorContainerExpression(const Token* tok1, return false; } -static ValueFlow::Value getLifetimeIteratorValue(const Token* tok) +static ValueFlow::Value getLifetimeIteratorValue(const Token* tok, MathLib::bigint path = 0) { - std::vector values = getLifetimeObjValues(tok); + std::vector values = getLifetimeObjValues(tok, false, path); auto it = std::find_if(values.begin(), values.end(), [](const ValueFlow::Value& v) { return v.lifetimeKind == ValueFlow::Value::LifetimeKind::Iterator; }); @@ -2294,6 +2294,8 @@ void CheckStl::checkDereferenceInvalidIterator2() isInvalidIterator = true; } else { auto it = std::find_if(contValues.begin(), contValues.end(), [&](const ValueFlow::Value& c) { + if (value.path != c.path) + return false; if (value.isIteratorStartValue() && value.intvalue >= c.intvalue) return true; if (value.isIteratorEndValue() && -value.intvalue > c.intvalue) @@ -2329,7 +2331,7 @@ void CheckStl::checkDereferenceInvalidIterator2() inconclusive = true; } if (cValue) { - const ValueFlow::Value& lValue = getLifetimeIteratorValue(tok); + const ValueFlow::Value& lValue = getLifetimeIteratorValue(tok, cValue->path); if (emptyAdvance) outOfBoundsError(emptyAdvance, lValue.tokvalue->expressionString(), diff --git a/lib/token.cpp b/lib/token.cpp index 8ecc05661..677af05c9 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -2426,7 +2426,7 @@ const ValueFlow::Value* Token::getMaxValue(bool condition, MathLib::bigint path) continue; if (value.isImpossible()) continue; - if (value.path != 0 && value.path != path) + if (path > -0 && value.path != 0 && value.path != path) continue; if ((!ret || value.intvalue > ret->intvalue) && ((value.condition != nullptr) == condition)) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 436fac095..7be725a52 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2973,16 +2973,18 @@ std::string lifetimeMessage(const Token *tok, const ValueFlow::Value *val, Error return msg; } -std::vector getLifetimeObjValues(const Token *tok, bool inconclusive, bool subfunction) +std::vector getLifetimeObjValues(const Token* tok, bool inconclusive, MathLib::bigint path) { std::vector result; - auto pred = [&](const ValueFlow::Value &v) { - if (!v.isLocalLifetimeValue() && !(subfunction && v.isSubFunctionLifetimeValue())) + auto pred = [&](const ValueFlow::Value& v) { + if (!v.isLocalLifetimeValue() && !(path != 0 && v.isSubFunctionLifetimeValue())) return false; if (!inconclusive && v.isInconclusive()) return false; if (!v.tokvalue) return false; + if (path >= 0 && v.path != 0 && v.path != path) + return false; return true; }; std::copy_if(tok->values().begin(), tok->values().end(), std::back_inserter(result), pred); @@ -2991,7 +2993,7 @@ std::vector getLifetimeObjValues(const Token *tok, bool inconc ValueFlow::Value getLifetimeObjValue(const Token *tok, bool inconclusive) { - std::vector values = getLifetimeObjValues(tok, inconclusive, false); + std::vector values = getLifetimeObjValues(tok, inconclusive); // There should only be one lifetime if (values.size() != 1) return ValueFlow::Value{}; diff --git a/lib/valueflow.h b/lib/valueflow.h index b974435fe..25187d162 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -513,6 +513,8 @@ std::string lifetimeMessage(const Token *tok, const ValueFlow::Value *val, Value CPPCHECKLIB ValueFlow::Value getLifetimeObjValue(const Token *tok, bool inconclusive = false); -CPPCHECKLIB std::vector getLifetimeObjValues(const Token *tok, bool inconclusive = false, bool subfunction = false); +CPPCHECKLIB std::vector getLifetimeObjValues(const Token* tok, + bool inconclusive = false, + MathLib::bigint path = 0); #endif // valueflowH diff --git a/test/teststl.cpp b/test/teststl.cpp index 95343ca96..aecc12c00 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -692,6 +692,19 @@ private: "test.cpp:2:note:condition 'i<=(int)v.size()'\n" "test.cpp:3:note:Access out of bounds\n", errout.str()); + + check("template\n" + "void b(Iterator d) {\n" + " std::string c = \"a\";\n" + " d + c.length();\n" + "}\n" + "void f() {\n" + " std::string buf;\n" + " b(buf.begin());\n" + "}\n", + true); + ASSERT_EQUALS("test.cpp:4:error:Out of bounds access in expression 'd+c.length()' because 'buf' is empty.\n", + errout.str()); } void outOfBoundsSymbolic()