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 4 commits into
base: main
Choose a base branch
from
+84 −11
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
4 commits
Select commit Hold shift + click to select a range
5ec129e
Fix #11765 FN: minsize not checked for string literal, buffer access …
web-flow Oct 17, 2026
1fa524a
Fix
web-flow Oct 18, 2026
167421b
Fix
web-flow Oct 18, 2026
558148b
Fix
web-flow Oct 18, 2026
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
2 changes: 2 additions & 0 deletions cfg/gtk.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5386,11 +5386,13 @@
<arg nr="1" direction="in">
<not-null/>
<not-uninit/>
<strz/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="2" direction="in">
<not-null/>
<not-uninit/>
<strz/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="3" direction="in">
Expand Down
4 changes: 4 additions & 0 deletions cfg/posix.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2123,11 +2123,13 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
<arg nr="1" direction="in">
<not-null/>
<not-uninit/>
<strz/>

@dmcppcheck dmcppcheck Oct 18, 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

<strz/> also turns on the invalidFunctionArgStr check in CheckFunctions, which requires a NUL-terminated argument. strncasecmp reads at most n bytes, so a non-terminated buffer is valid here. With this change, char buf[3] = {'a','b','c'}; strncasecmp(buf, s, 3); and char c = 'a'; strncasecmp(&c, s, 1); give error: Invalid strncasecmp() argument nr 1. A nul-terminated string is required. [invalidFunctionArgStr], and base reports nothing. The same applies to arg 2 (line 2132). Adding <strz/> the same way to the other bounded functions (strnlen, _strnicmp, ...) would spread this false positive.

<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="2" direction="in">
<not-null/>
<not-uninit/>
<strz/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="3" direction="in">
Expand Down Expand Up @@ -5606,6 +5608,7 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
<arg nr="1" direction="in">
<not-null/>
<not-uninit/>
<strz/>
<minsize type="argvalue" arg="2"/>
</arg>
<arg nr="2" direction="in">
Expand Down Expand Up @@ -5782,6 +5785,7 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
<arg nr="2" direction="in">
<not-null/>
<not-uninit/>
<strz/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="3" direction="in">
Expand Down
2 changes: 2 additions & 0 deletions cfg/windows.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4293,11 +4293,13 @@ HFONT CreateFont(
<arg nr="1" direction="in">
<not-null/>
<not-uninit/>
<strz/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="2" direction="in">
<not-null/>
<not-uninit/>
<strz/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="3" direction="in">
Expand Down
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
8 changes: 7 additions & 1 deletion lib/checkfunctions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ void CheckFunctionsImpl::functionCalledError(const Token* tok, Severity severity
reportError(tok, severity, prefix + "Called", msg, CWE477, Certainty::normal);
}

static bool hasMinSize(const Token* ftok, int argnr, const Settings& settings)
{
const std::vector<Library::ArgumentChecks::MinSize>* minsizes = settings.library.argminsizes(ftok, argnr);
return minsizes && !minsizes->empty();
}

//---------------------------------------------------------------------------
// Check <valid>, <strz> and <not-bool>
//---------------------------------------------------------------------------
Expand Down Expand Up @@ -137,7 +143,7 @@ void CheckFunctionsImpl::invalidFunctionUsage()
invalidFunctionArgError(argtok, functionToken->str(), argnr, nullptr, mSettings.library.validarg(functionToken, argnr));
}
// check <strz>
if (mSettings.library.isargstrz(functionToken, argnr)) {
if (mSettings.library.isargstrz(functionToken, argnr) && !hasMinSize(functionToken, argnr, mSettings)) {

@dmcppcheck dmcppcheck Oct 18, 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 turns off the <strz/> check for every argument that has any <minsize>. That covers more than the bounded in arguments of strncasecmp/strnlen. It also covers destinations that really must be NUL-terminated: strcat arg1 (minsize type="strlen"), strncat/wcsncat arg1, strcat_s, strlcat, _mbsncat, _wcsncat_l, and so on.

char buf[3] = {'a','b','c'}; strcat(buf, "x"); and char d[3]={'a','b','c'}; strncat(d, "x", 1); gave invalidFunctionArgStr in base. With this PR they give nothing.

You could limit the exception to direction="in" arguments with minsize type="argvalue", or add a test in test/cfg/std.c for strz on the strcat/strncat destination.

if (Token::Match(argtok, "& %var% !![") && argtok->next() && argtok->next()->valueType()) {
const ValueType * valueType = argtok->next()->valueType();
const Variable * variable = argtok->next()->variable();
Expand Down
15 changes: 14 additions & 1 deletion test/cfg/posix.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,12 @@ void invalidFunctionArgStr_mbsnrtowcs(void)
if (mbsnrtowcs (wenough, &cp, 1, 10, &s) != 1 || wcscmp (wenough, L"AEF") != 0) {}
}

int invalidFunctionArgStr_strncasecmp(const char* s)
{
const char buf[3] = {'a', 'b', 'c'};
return strncasecmp(buf, s, 3);
}

struct tm * ignoredReturnValue_localtime(const time_t *tp)
{
// cppcheck-suppress [ignoredReturnValue,localtimeCalled]
Expand Down Expand Up @@ -719,6 +725,13 @@ 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",5);
return len;
}

size_t bufferAccessOutOfBounds_strncasecmp(const char *s)
{
size_t len = strncasecmp(s, "abc", 5);
return len;
}

Expand Down Expand Up @@ -1606,4 +1619,4 @@ void pthread_define_test()
(void)rwlock;
PTHREAD_SCOPE_PROCESS;
PTHREAD_SCOPE_SYSTEM;
}
}
5 changes: 2 additions & 3 deletions test/cfg/std.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4000,11 +4000,10 @@ void bufferAccessOutOfBounds_strxfrm(void)
{
const char src[3] = "abc";
char dest[1] = "a";
// cppcheck-suppress invalidFunctionArgStr
(void)strxfrm(dest,src,1);
// cppcheck-suppress [bufferAccessOutOfBounds,invalidFunctionArgStr]
// cppcheck-suppress [bufferAccessOutOfBounds]
(void)strxfrm(dest,src,2);
// cppcheck-suppress [bufferAccessOutOfBounds,invalidFunctionArgStr]
// cppcheck-suppress [bufferAccessOutOfBounds]
(void)strxfrm(dest,src,3);
}

Expand Down
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