From 7f682d544e6b7676100e8dcde022bb0932989edf Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Sun, 6 Mar 2022 07:41:09 +0100 Subject: [PATCH] Partial fix for #6615 FN buffer access out of bounds: std::vector (#3873) * Fix #10779 FN: stlOutOfBounds (off by one) * Format * Simplify * Partial fix for #6615 FN buffer access out of bounds: std::vector * Undo * Format * Fix test case --- lib/valueflow.cpp | 27 +++++++++++++++++++++++++-- test/teststl.cpp | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 0ac2a58de..f1310f3ee 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7634,15 +7634,17 @@ static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symbold if (vnt->hasKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE)) continue; const bool isDecl = Token::Match(vnt, "%name% ;"); - bool hasInitList = false, hasInitSize = false; + bool hasInitList = false, hasInitSize = false, isPointerInit = false; if (!isDecl) { hasInitList = Token::Match(vnt, "%name% {") && Token::simpleMatch(vnt->next()->link(), "} ;"); if (!hasInitList) hasInitList = Token::Match(vnt, "%name% ( {") && Token::simpleMatch(vnt->linkAt(2), "} ) ;"); if (!hasInitList) hasInitSize = Token::Match(vnt, "%name% (|{ %num%|%var% )|}"); + if (!hasInitList && !hasInitSize) + isPointerInit = Token::Match(vnt, "%name% ( %var% ,"); } - if (!isDecl && !hasInitList && !hasInitSize) + if (!isDecl && !hasInitList && !hasInitSize && !isPointerInit) continue; if (vnt->astTop() && Token::Match(vnt->astTop()->previous(), "for|while")) known = !isVariableChanged(var, settings, true); @@ -7667,6 +7669,27 @@ static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symbold continue; values.back().intvalue = sizeTok->getKnownIntValue(); } + else if (isPointerInit) { // (ptr, ptr + size) + const Token* commaTok = vnt->next()->astOperand2(); + if (commaTok && commaTok->str() == "," && commaTok->astOperand1() && commaTok->astOperand2() && commaTok->astOperand2()->isArithmeticalOp() && commaTok->astOperand2()->str() == "+") { + const Token* varTok1 = commaTok->astOperand1(); + const Token* plusTok = commaTok->astOperand2(); + if (varTok1->varId() && plusTok->astOperand1() && plusTok->astOperand2() && varTok1->valueType() && varTok1->valueType()->pointer) { + const Token* sizeTok = nullptr; + if (varTok1->varId() == plusTok->astOperand1()->varId()) + sizeTok = plusTok->astOperand2(); + else if (varTok1->varId() == plusTok->astOperand2()->varId()) + sizeTok = plusTok->astOperand1(); + if (!sizeTok || !sizeTok->hasKnownIntValue()) + continue; + values.back().intvalue = sizeTok->getKnownIntValue(); + } + else + continue; + } + else + continue; + } for (const ValueFlow::Value& value : values) valueFlowContainerForward(vnt->next(), vnt, value, tokenlist); } diff --git a/test/teststl.cpp b/test/teststl.cpp index 5da53f3cd..4f8d6c053 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -753,6 +753,40 @@ private: "}\n"); ASSERT_EQUALS("test.cpp:3:error:Out of bounds access in 'v[100]', if 'v' size is 3 and '100' is 100\n", errout.str()); + + check("void f() {\n" + " char c[] = { 1, 2, 3 };\n" + " std::vector v(c, sizeof(c) + c);\n" + " v[100] = 1;\n" + "}\n"); + ASSERT_EQUALS("test.cpp:4:error:Out of bounds access in 'v[100]', if 'v' size is 3 and '100' is 100\n", + errout.str()); + + check("void f() {\n" + " char c[] = { 1, 2, 3 };\n" + " std::vector v{ c, c + sizeof(c) };\n" + " v[100] = 1;\n" + "}\n"); + TODO_ASSERT_EQUALS("test.cpp:4:error:Out of bounds access in 'v[100]', if 'v' size is 3 and '100' is 100\n", + "", + errout.str()); + + check("void f() {\n" + " int i[] = { 1, 2, 3 };\n" + " std::vector v(i, i + sizeof(i) / 4);\n" + " v[100] = 1;\n" + "}\n"); + ASSERT_EQUALS("test.cpp:4:error:Out of bounds access in 'v[100]', if 'v' size is 3 and '100' is 100\n", + errout.str()); + + check("void f() {\n" // #6615 + " int i[] = { 1, 2, 3 };\n" + " std::vector v(i, i + sizeof(i) / sizeof(int));\n" + " v[100] = 1;\n" + "}\n"); + TODO_ASSERT_EQUALS("test.cpp:4:error:Out of bounds access in 'v[100]', if 'v' size is 3 and '100' is 100\n", + "", + errout.str()); } void outOfBoundsSymbolic()