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
+52 −37
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
0383f66
Update testbufferoverrun.cpp
chrchr-github Oct 15, 2026
c6eab5f
Update checkbufferoverrun.cpp
chrchr-github Oct 15, 2026
dcee57b
Fix, add test
web-flow Oct 15, 2026
a387948
Fix
web-flow Oct 15, 2026
cd753bf
Fix
web-flow Oct 15, 2026
775b826
Update posix.cfg [skip ci]
chrchr-github Oct 16, 2026
778cb2e
Update windows.cfg [skip ci]
chrchr-github Oct 16, 2026
ecee9f7
Update sqlite3.cfg
chrchr-github Oct 16, 2026
9527653
Update posix.c
chrchr-github Oct 16, 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
7 changes: 0 additions & 7 deletions cfg/posix.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2123,12 +2123,10 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
<arg nr="1" direction="in">
<not-null/>
<not-uninit/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="2" direction="in">
<not-null/>
<not-uninit/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="3" direction="in">
<not-bool/>
Expand All @@ -2146,12 +2144,10 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
<arg nr="1" direction="in">
<not-null/>
<not-uninit/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="2" direction="in">
<not-null/>
<not-uninit/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="3" direction="in">
<not-bool/>
Expand Down Expand Up @@ -4200,7 +4196,6 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
<not-null/>
<not-uninit/>
<strz/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="3" direction="in">
<not-uninit/>
Expand Down Expand Up @@ -5472,7 +5467,6 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
<not-null/>
<not-uninit/>
<not-bool/>
<minsize type="argvalue" arg="2"/>
<strz/>
</arg>
<arg nr="2" direction="in">
Expand Down Expand Up @@ -5606,7 +5600,6 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
<arg nr="1" direction="in">
<not-null/>
<not-uninit/>
<minsize type="argvalue" arg="2"/>
</arg>
<arg nr="2" direction="in">
<not-uninit/>
Expand Down
2 changes: 0 additions & 2 deletions cfg/sqlite3.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,13 +1403,11 @@
<not-uninit/>
<not-null/>
<strz/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="2" direction="in">
<not-uninit/>
<not-null/>
<strz/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="3" direction="in">
<not-uninit/>
Expand Down
3 changes: 0 additions & 3 deletions cfg/std.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5030,7 +5030,6 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
<arg nr="3" direction="in">
<not-null/>
<not-uninit/>
<minsize type="argvalue" arg="4"/>
<strz/>
</arg>
<arg nr="4" direction="in">
Expand Down Expand Up @@ -5116,7 +5115,6 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
<arg nr="3" direction="in">
<not-null/>
<not-uninit/>
<minsize type="argvalue" arg="4"/>
<strz/>
</arg>
<arg nr="4" direction="in">
Expand Down Expand Up @@ -5233,7 +5231,6 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
<not-null/>
<not-uninit/>
<strz/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="3" direction="in">
<not-uninit/>
Expand Down
3 changes: 0 additions & 3 deletions cfg/windows.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4293,12 +4293,10 @@ HFONT CreateFont(
<arg nr="1" direction="in">
<not-null/>
<not-uninit/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="2" direction="in">
<not-null/>
<not-uninit/>
<minsize type="argvalue" arg="3"/>
</arg>
<arg nr="3" direction="in">
<not-uninit/>
Expand Down Expand Up @@ -4632,7 +4630,6 @@ HFONT CreateFont(
<arg nr="3" direction="in">
<not-null/>
<not-uninit/>
<minsize type="argvalue" arg="4"/>
</arg>
<arg nr="4" direction="in">
<not-uninit/>
Expand Down
8 changes: 8 additions & 0 deletions lib/checkbufferoverrun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,14 @@ ValueFlow::Value CheckBufferOverrunImpl::getBufferSize(const Token *bufTok, cons
return bufSizeVal;
}
}
} else if (bufTok->tokType() == Token::Type::eString && bufTok->valueType()) {

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

With this branch, every input-argument minsize type="argvalue" in the library configs is now checked against string literals. That is wrong for bounded string functions, which stop reading at the NUL. Valid code now gets bufferAccessOutOfBounds, and base did not warn on any of these:

strncasecmp(s, "abc", 10);      // posix.cfg
strnlen("abc", 100);             // posix.cfg
_strnicmp(s, "abc", 10);        // windows.cfg
strxfrm_l(d, "abc", 10, loc);   // posix.cfg, same issue as the strxfrm entry removed from std.cfg

The same pattern is also in wcsncasecmp, wcsnlen and wcswidth (posix.cfg), _strncpy_s_l/_mbsncpy_s (windows.cfg) and sqlite3_stricmp (sqlite3.cfg). These configs need the same cleanup as std.cfg. The other option is to not use the literal size for strz arguments.

@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 still causes false positives for bounded string functions whose library entries keep minsize type="argvalue" on input arguments:

  • _strnicmp_l(s, "abc", 10, l) (windows.cfg)
  • g_strncasecmp(s, "abc", 10) (gtk.cfg)
  • wcpncpy(d, L"ab", 20) (posix.cfg, arg 2)

All three now give bufferAccessOutOfBounds, and base gives nothing. Removing the minsize entries one function at a time also loses real detections on arrays that are not NUL-terminated (see the strnlen(buf,3) TODO in posix.c). Ignoring the literal size when the argument has <strz/>, or limiting it to non-strz arguments, would avoid both problems.

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
5 changes: 3 additions & 2 deletions test/cfg/posix.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,8 +717,9 @@ size_t bufferAccessOutOfBounds_strnlen(const char *s, size_t maxlen)
{
const char buf[2]={'4','2'};
size_t len = strnlen(buf,2);
// cppcheck-suppress bufferAccessOutOfBounds
// TODO cppcheck-suppress bufferAccessOutOfBounds

@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 is a regression. buf is {'4','2'}, which is not NUL-terminated, so strnlen(buf,3) really reads past the end, and base detected it. The minsize entry was removed from strnlen only so that the literal case stays quiet. The same detection is lost for strncasecmp, strxfrm, strncpy_s, strncat_s, sqlite3_stricmp and the others. If the literal size were not applied to strz arguments, the cfg entries and this check could stay.

len+=strnlen(buf,3);
len+=strnlen("ab",3);
return len;
}

Expand Down Expand Up @@ -1606,4 +1607,4 @@ void pthread_define_test()
(void)rwlock;
PTHREAD_SCOPE_PROCESS;
PTHREAD_SCOPE_SYSTEM;
}
}
61 changes: 41 additions & 20 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 @@ -3484,23 +3485,6 @@ class TestBufferOverrun : public TestFixture {
" (void)strxfrm(dest,src,3);\n" // <<
"}\n");
ASSERT_EQUALS("[test.cpp:6:19]: (error) Buffer is accessed out of bounds: dest [bufferAccessOutOfBounds]\n", errout_str());
// source size is too small
check("void f(void) {\n"
" const char src[2] = \"ab\";\n"
" char dest[3] = \"abc\";\n"
" (void)strxfrm(dest,src,1);\n"
" (void)strxfrm(dest,src,2);\n"
" (void)strxfrm(dest,src,3);\n" // <<
"}\n");
ASSERT_EQUALS("[test.cpp:6:24]: (error) Buffer is accessed out of bounds: src [bufferAccessOutOfBounds]\n", errout_str());
// source size is too small
check("void f(void) {\n"
" const char src[1] = \"a\";\n"
" char dest[3] = \"abc\";\n"
" (void)strxfrm(dest,src,1);\n"
" (void)strxfrm(dest,src,2);\n" // <<
"}\n");
ASSERT_EQUALS("[test.cpp:5:24]: (error) Buffer is accessed out of bounds: src [bufferAccessOutOfBounds]\n", errout_str());
}

void buffer_overrun_33() { // #2019
Expand Down Expand Up @@ -3638,6 +3622,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 +4640,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