From 89a9e5ecc6b6782baf83883e4a6c644c065bdbb5 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Fri, 8 Jul 2022 12:35:21 +0200 Subject: [PATCH] Fix #9944 FP: terminateStrncpy doesn't account for size check (#4252) * Fix #9944 FP: terminateStrncpy doesn't account for size check * Fix container size check * Undo * Format * Rebuild * Rebuild --- lib/checkbufferoverrun.cpp | 15 ++++++++++++--- test/testbufferoverrun.cpp | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 499813323..68b42a1d1 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -739,9 +739,18 @@ void CheckBufferOverrun::stringNotZeroTerminated() const ValueFlow::Value &bufferSize = getBufferSize(args[0]); if (bufferSize.intvalue < 0 || sizeToken->getKnownIntValue() < bufferSize.intvalue) continue; - const Token *srcValue = args[1]->getValueTokenMaxStrLength(); - if (srcValue && Token::getStrLength(srcValue) < sizeToken->getKnownIntValue()) - continue; + if (Token::simpleMatch(args[1], "(") && Token::simpleMatch(args[1]->astOperand1(), ". c_str") && args[1]->astOperand1()->astOperand1()) { + const std::list& contValues = args[1]->astOperand1()->astOperand1()->values(); + auto it = std::find_if(contValues.begin(), contValues.end(), [](const ValueFlow::Value& value) { + return value.isContainerSizeValue() && !value.isImpossible(); + }); + if (it != contValues.end() && it->intvalue < sizeToken->getKnownIntValue()) + continue; + } else { + const Token* srcValue = args[1]->getValueTokenMaxStrLength(); + if (srcValue && Token::getStrLength(srcValue) < sizeToken->getKnownIntValue()) + continue; + } // Is the buffer zero terminated after the call? bool isZeroTerminated = false; for (const Token *tok2 = tok->next()->link(); tok2 != scope->bodyEnd; tok2 = tok2->next()) { diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 6d34d4a12..e09c0732b 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -289,6 +289,7 @@ private: TEST_CASE(terminateStrncpy2); TEST_CASE(terminateStrncpy3); TEST_CASE(terminateStrncpy4); + TEST_CASE(terminateStrncpy5); // #9944 TEST_CASE(recursive_long_time); TEST_CASE(crash1); // Ticket #1587 - crash @@ -4342,6 +4343,23 @@ private: "}"); ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) The buffer 'buf' may not be null-terminated after the call to strncpy().\n", errout.str()); } + + void terminateStrncpy5() { // #9944 + check("void f(const std::string& buf) {\n" + " char v[255];\n" + " if (buf.size() >= sizeof(v))\n" + " return;\n" + " strncpy(v, buf.c_str(), sizeof(v));\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f(const std::string& buf) {\n" + " char v[255];\n" + " if (buf.size() >= sizeof(v))\n" + " strncpy(v, buf.c_str(), sizeof(v));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) The buffer 'v' may not be null-terminated after the call to strncpy().\n", errout.str()); + } // extracttests.enable void recursive_long_time() {