Skip to content

Fix #11765 FN: minsize not checked for string literal, buffer access out of bounds not found #8944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chrchr-github wants to merge 1 commit into
base: main
Choose a base branch
from
+55 −7
Open

Fix #11765 FN: minsize not checked for string literal, buffer access out of bounds not found #8944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions lib/checkbufferoverrun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ void CheckBufferOverrunImpl::pointerArithmeticError(const Token *tok, const Toke

//---------------------------------------------------------------------------

ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, const Settings& settings) const
ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, const Settings& settings, bool checkStringLiteral) const
{
if (!bufTok->valueType())
return ValueFlow::Value(-1);
Expand Down Expand Up @@ -604,6 +604,14 @@ ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, cons
return bufSizeVal;
}
}
} else if (checkStringLiteral && bufTok->tokType() == Token::Type::eString && bufTok->valueType()) {
ValueFlow::Value strSizeVal;
strSizeVal.valueType = ValueFlow::Value::ValueType::BUFFER_SIZE;
const size_t charSize = bufTok->valueType()->getSizeOf(settings, ValueType::Accuracy::ExactOrZero, ValueType::SizeOf::Pointee);
if (charSize > 0) {
strSizeVal.intvalue = Token::getStrArraySize(bufTok) * charSize;
return strSizeVal;
}
}
}

Expand Down Expand Up @@ -715,7 +723,8 @@ void CheckBufferOverrunImpl::bufferOverflow()
if (argtok->valueType() && argtok->valueType()->pointer == 0)
continue;
// TODO: strcpy(buf+10, "hello");
ValueFlow::Value bufferSize = getBufferSize(argtok, mSettings);
const bool strz = mSettings.library.isargstrz(tok, argnr + 1);

@dmcppcheck dmcppcheck Oct 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review

This check only helps if the cfg marks the argument <strz/>. Many bounded string functions have minsize type="argvalue" on an in argument without <strz/>:

  • strnlen/wcsnlen, strncasecmp/wcsncasecmp and wcpncpy (arg 2) in posix.cfg
  • _strnicmp*, _strnicmp_l* and _strncpy_s_l/_mbsncpy_s in windows.cfg
  • g_strncasecmp in gtk.cfg

So strnlen("abc", 100), strncasecmp(s, "abc", 10), _strnicmp(s, "abc", 10), wcpncpy(d, L"ab", 20) and g_strncasecmp(s, "abc", 10) all give bufferAccessOutOfBounds again, and base reports nothing. Adding <strz/> to these arguments in the cfg files would fix it and still find the overread on non-terminated arrays.

ValueFlow::Value bufferSize = getBufferSize(argtok, mSettings, !strz);
if (bufferSize.intvalue <= 0)
continue;
// buffer size == 1 => do not warn for dynamic memory
Expand Down
2 changes: 1 addition & 1 deletion lib/checkbufferoverrun.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class CPPCHECKLIB CheckBufferOverrunImpl : public CheckImpl
void objectIndex();
void objectIndexError(const Token *tok, const ValueFlow::Value *v, bool known);

ValueFlow::Value getBufferSize(const Token *bufTok, const Settings& settings) const;
ValueFlow::Value getBufferSize(const Token *bufTok, const Settings& settings, bool checkStringLiteral = false) const;

// CTU
static bool isCtuUnsafeBufferUsage(const Settings &settings, const Token *argtok, CTU::FileInfo::Value *offset, int type);
Expand Down
3 changes: 2 additions & 1 deletion test/cfg/posix.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,7 @@ size_t bufferAccessOutOfBounds_strnlen(const char *s, size_t maxlen)
size_t len = strnlen(buf,2);
// cppcheck-suppress bufferAccessOutOfBounds
len+=strnlen(buf,3);
len+=strnlen("ab",3);

@dmcppcheck dmcppcheck Oct 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review

This line doesn't test the change. "ab" is 3 bytes, so a bound of 3 is never flagged. strnlen("ab",4) is valid but this PR flags it (Buffer is accessed out of bounds: "ab"). A bound larger than the literal's size would catch the false positive.

return len;
}

Expand Down Expand Up @@ -1606,4 +1607,4 @@ void pthread_define_test()
(void)rwlock;
PTHREAD_SCOPE_PROCESS;
PTHREAD_SCOPE_SYSTEM;
}
}
44 changes: 41 additions & 3 deletions test/testbufferoverrun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ class TestBufferOverrun : public TestFixture {
TEST_CASE(buffer_overrun_36);
TEST_CASE(buffer_overrun_37);
TEST_CASE(buffer_overrun_38);
TEST_CASE(buffer_overrun_39);
TEST_CASE(buffer_overrun_errorpath);
TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch
TEST_CASE(buffer_overrun_function_array_argument);
Expand Down Expand Up @@ -3638,6 +3639,44 @@ class TestBufferOverrun : public TestFixture {
ASSERT_EQUALS("", errout_str());
}

void buffer_overrun_39() { // #11765
check("void f() {\n"
" char buf[10];\n"
" memcpy(buf, \"abc\", 4);\n"
"}\n");
ASSERT_EQUALS("", errout_str());

check("void f() {\n"
" char buf[128];\n"
" memcpy(buf, \"abc\", 5);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3:17]: (warning) Buffer is accessed out of bounds: \"abc\" [bufferAccessOutOfBounds]\n",
errout_str());

check("void f() {\n"
" char buf[128];\n"
" memcpy(buf, L\"abc\", 4 * sizeof(wchar_t));\n"
"}\n");
ASSERT_EQUALS("", errout_str());

check("void f() {\n"
" char buf[128];\n"
" memcpy(buf, L\"abc\", 4 * sizeof(wchar_t) + 1);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3:17]: (warning) Buffer is accessed out of bounds: L\"abc\" [bufferAccessOutOfBounds]\n",
errout_str());

check("int f(char* s) {\n"
" return memcmp(s, \"\\0\\0\\0\", 3);\n"
"}\n");
ASSERT_EQUALS("", errout_str());

check("int f(const char16_t* buf) {\n"
" return memcmp(buf, u\"abc\", 4 * sizeof(char16_t));\n"
"}\n");
ASSERT_EQUALS("", errout_str());
}

void buffer_overrun_errorpath() {
setMultiline();
Settings s = settings0;
Expand Down Expand Up @@ -4618,9 +4657,8 @@ class TestBufferOverrun : public TestFixture {
check("void f() {\n"
" mymemset(\"abc\", 0, 20);\n"
"}\n", settings);
TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Buffer is accessed out of bounds.\n",
"",
errout_str());
ASSERT_EQUALS("[test.cpp:2:12]: (warning) Buffer is accessed out of bounds: \"abc\" [bufferAccessOutOfBounds]\n",
errout_str());

check("void f() {\n"
" mymemset(temp, \"abc\", 4);\n"
Expand Down
Loading