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 9 commits into
base: main
Choose a base branch
from
+108 −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
9 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
a372e8d
Update std.c [skip ci]
chrchr-github Oct 19, 2026
8dac772
Update checkfunctions.cpp
chrchr-github Oct 19, 2026
e183741
Update std.c
chrchr-github Oct 19, 2026
062dd28
Update std.c
chrchr-github Oct 19, 2026
bb3c7f8
Fix severity, use index, fix config, restore warning
web-flow Oct 19, 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
6 changes: 6 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/>
Comment thread
chrchr-github marked this conversation as resolved.
<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 All @@ -2146,11 +2148,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/>
<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 +5610,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 +5787,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
4 changes: 4 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 All @@ -4317,11 +4319,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
14 changes: 12 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,15 @@ ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, cons
return bufSizeVal;
}
}
} else if (checkStringLiteral && bufTok->tokType() == Token::Type::eString && bufTok->valueType()) {
ValueFlow::Value strSizeVal;
Comment thread
chrchr-github marked this conversation as resolved.
strSizeVal.setKnown();
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) - index) * charSize;
return strSizeVal;
}
}
}

Expand Down Expand Up @@ -715,7 +724,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);
Comment thread
chrchr-github marked this conversation as resolved.
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
14 changes: 13 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,18 @@ void CheckFunctionsImpl::functionCalledError(const Token* tok, Severity severity
reportError(tok, severity, prefix + "Called", msg, CWE477, Certainty::normal);
}

static bool hasArgMinReadSize(const Token *ftok, int argnr, const Library &library)
{
if (library.getArgDirection(ftok, argnr) == Library::ArgumentChecks::Direction::DIR_IN) {
if (const std::vector<Library::ArgumentChecks::MinSize> *minsizes = library.argminsizes(ftok, argnr)) {
return std::any_of(minsizes->begin(), minsizes->end(), [](const Library::ArgumentChecks::MinSize &ms) {
return ms.type == Library::ArgumentChecks::MinSize::Type::ARGVALUE;
});
}
}
return false;
}

//---------------------------------------------------------------------------
// Check <valid>, <strz> and <not-bool>
//---------------------------------------------------------------------------
Expand Down Expand Up @@ -137,7 +149,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) && !hasArgMinReadSize(functionToken, argnr, mSettings.library)) {

@dmcppcheck dmcppcheck Oct 20, 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 exception also covers strxfrm/std::strxfrm arg 2 (std.cfg) and strxfrm_l arg 2 (posix.cfg). Those functions read the whole source up to its NUL terminator; n only limits what is written to the destination, so the strz check is needed there. The PR misses const char src[3] = {'a','b','c'}; strxfrm(d, src, 1);, char c='a'; strxfrm(d, &c, 1); and the same strxfrm_l call, all of which base reports as invalidFunctionArgStr.

Two possible fixes:

  • Mark the functions that should skip the check explicitly instead of inferring it from minsize argvalue.
  • Remove the misleading minsize arg="3" from the source argument of strxfrm/strxfrm_l; it bounds the destination, not the source.

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;
}
}
12 changes: 9 additions & 3 deletions test/cfg/std.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ void invalidFunctionArgStr_strncat(void)
// No warning is expected for
strncat (str1, &src, 1);
puts (str1);
char d[] = {'a','b','c'};
// cppcheck-suppress invalidFunctionArgStr
strncat(d, "def", 1);
}

char * invalidFunctionArgStr_strpbrk( const char *p )
Expand Down Expand Up @@ -4000,11 +4003,10 @@ void bufferAccessOutOfBounds_strxfrm(void)
{
const char src[3] = "abc";
char dest[1] = "a";
// cppcheck-suppress invalidFunctionArgStr
(void)strxfrm(dest,src,1);

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

src is {'a','b','c'}, which has no NUL terminator. strxfrm reads all of src whatever n is, so invalidFunctionArgStr is a real error here. Removing these suppressions makes the cfg test accept the detection that hasArgMinReadSize() loses, instead of catching it.

// 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 Expand Up @@ -4913,6 +4915,10 @@ void invalidFunctionArgString(char c)
char d='d';
/* cppcheck-suppress invalidFunctionArgStr */
(void)strcat(str,&d);

char a[] = { 'a', 'b', 'c' };
/* cppcheck-suppress [bufferAccessOutOfBounds,invalidFunctionArgStr] */
(void)strcat(a, "def");
}

void ignoredReturnValue_abs(int i)
Expand Down
50 changes: 47 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,50 @@ 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]: (error) 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]: (error) 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());

check("int f(const char* buf) {\n"
" return memcmp(buf, &\"abc\"[2], 4 * sizeof(char));\n"
"}\n");
ASSERT_EQUALS("[test.cpp:2:24]: (error) Buffer is accessed out of bounds: &\"abc\"[2] [bufferAccessOutOfBounds]\n",
errout_str());
}

void buffer_overrun_errorpath() {
setMultiline();
Settings s = settings0;
Expand Down Expand Up @@ -4618,9 +4663,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]: (error) Buffer is accessed out of bounds: \"abc\" [bufferAccessOutOfBounds]\n",
errout_str());

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