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 5 commits into
base: main
Choose a base branch
from
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
5 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
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
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
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.

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