Skip to content

Commit 89a9e5e

Browse files
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
1 parent a71a647 commit 89a9e5e

2 files changed

Lines changed: 30 additions & 3 deletions

File tree

lib/checkbufferoverrun.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -739,9 +739,18 @@ void CheckBufferOverrun::stringNotZeroTerminated()
739739
const ValueFlow::Value &bufferSize = getBufferSize(args[0]);
740740
if (bufferSize.intvalue < 0 || sizeToken->getKnownIntValue() < bufferSize.intvalue)
741741
continue;
742-
const Token *srcValue = args[1]->getValueTokenMaxStrLength();
743-
if (srcValue && Token::getStrLength(srcValue) < sizeToken->getKnownIntValue())
744-
continue;
742+
if (Token::simpleMatch(args[1], "(") && Token::simpleMatch(args[1]->astOperand1(), ". c_str") && args[1]->astOperand1()->astOperand1()) {
743+
const std::list<ValueFlow::Value>& contValues = args[1]->astOperand1()->astOperand1()->values();
744+
auto it = std::find_if(contValues.begin(), contValues.end(), [](const ValueFlow::Value& value) {
745+
return value.isContainerSizeValue() && !value.isImpossible();
746+
});
747+
if (it != contValues.end() && it->intvalue < sizeToken->getKnownIntValue())
748+
continue;
749+
} else {
750+
const Token* srcValue = args[1]->getValueTokenMaxStrLength();
751+
if (srcValue && Token::getStrLength(srcValue) < sizeToken->getKnownIntValue())
752+
continue;
753+
}
745754
// Is the buffer zero terminated after the call?
746755
bool isZeroTerminated = false;
747756
for (const Token *tok2 = tok->next()->link(); tok2 != scope->bodyEnd; tok2 = tok2->next()) {

test/testbufferoverrun.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ class TestBufferOverrun : public TestFixture {
289289
TEST_CASE(terminateStrncpy2);
290290
TEST_CASE(terminateStrncpy3);
291291
TEST_CASE(terminateStrncpy4);
292+
TEST_CASE(terminateStrncpy5); // #9944
292293
TEST_CASE(recursive_long_time);
293294

294295
TEST_CASE(crash1); // Ticket #1587 - crash
@@ -4342,6 +4343,23 @@ class TestBufferOverrun : public TestFixture {
43424343
"}");
43434344
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) The buffer 'buf' may not be null-terminated after the call to strncpy().\n", errout.str());
43444345
}
4346+
4347+
void terminateStrncpy5() { // #9944
4348+
check("void f(const std::string& buf) {\n"
4349+
" char v[255];\n"
4350+
" if (buf.size() >= sizeof(v))\n"
4351+
" return;\n"
4352+
" strncpy(v, buf.c_str(), sizeof(v));\n"
4353+
"}\n");
4354+
ASSERT_EQUALS("", errout.str());
4355+
4356+
check("void f(const std::string& buf) {\n"
4357+
" char v[255];\n"
4358+
" if (buf.size() >= sizeof(v))\n"
4359+
" strncpy(v, buf.c_str(), sizeof(v));\n"
4360+
"}\n");
4361+
ASSERT_EQUALS("[test.cpp:4]: (warning, inconclusive) The buffer 'v' may not be null-terminated after the call to strncpy().\n", errout.str());
4362+
}
43454363
// extracttests.enable
43464364

43474365
void recursive_long_time() {

0 commit comments

Comments
 (0)