Skip to content

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

Open
chrchr-github wants to merge 5 commits into
cppcheck-opensource:mainfrom
chrchr-github:chr_11765
Open

chrchr-github wants to merge 5 commits into
cppcheck-opensource:mainfrom
chrchr-github:chr_11765

Conversation

@chrchr-github

Copy link
Copy Markdown
Collaborator

No description provided.

@chrchr-github
chrchr-github marked this pull request as ready for review October 15, 2026 19:17
@dmcppcheck

Copy link
Copy Markdown

Test results for commit cd753bf (tools/test-my-pr.py, main compared to this PR):

Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8944/
packages: 100
Differing warnings: 2 (+2 -0)
AI review: 2 improvement (http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8944/2026-10-15_18-02-29-cd753bfd6b6e-review.txt)
Timing: -0.7% (http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8944/2026-10-15_18-02-29-cd753bfd6b6e_timing.html)

Posted automatically by the cppcheck PR test runner. +N: warnings only with this PR, -N: warnings only with main. The AI review is written by Claude and can be wrong.

@dmcppcheck dmcppcheck left a comment

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

The PR adds new false positives. String literals now get a BUFFER_SIZE. As a result, every library minsize type="argvalue" on an input argument is checked against the literal's size. That is wrong for bounded string functions, which stop reading at the terminating NUL. Valid calls now get bufferAccessOutOfBounds warnings:

  • strncasecmp(s, "abc", 10)
  • strnlen("abc", 100)
  • _strnicmp(s, "abc", 10)
  • strxfrm_l(d, "abc", 10, l)

The PR removes minsize from strxfrm, strncpy_s and strncat_s in std.cfg for exactly this reason. It leaves the same pattern in other places:

  • strxfrm_l in posix.cfg, which is the direct counterpart of strxfrm
  • strncasecmp, wcsncasecmp, strnlen/wcsnlen and wcswidth in posix.cfg
  • _strnicmp* and _strncpy_s_l/_mbsncpy_s in windows.cfg
  • sqlite3_stricmp in sqlite3.cfg

These configs need the same cleanup. The other option is to apply the literal size only for functions that are really memory-based (memcpy, memcmp, write, …). The new warnings on memory functions (memcmp(s, "abc", 10), write(fd, "abc", 10)) are correct. Wide, u8/U and concatenated literals give the expected sizes.

Testing

Built: make -j8 cppcheck testrunner in pr/ and make -j8 cppcheck in base/. Both built fine.

Unit tests: ./testrunner TestBufferOverrun passed: 198 tests, 0 failures.

Library test files: I ran test/cfg/{std.c,std.cpp,posix.c,gnu.c,bsd.c,windows.cpp} through the cppcheck of both base and the PR with the runtests.sh options. The output is the same on both, so no existing cfg test covers these cases.

False positives found: scratch/a.c:

#include <string.h>
#include <strings.h>
#include <locale.h>
int f1(const char *s) { return strncasecmp(s, "abc", 10); }
size_t f3(void) { return strnlen("abc", 100); }
int f4(const char *s) { return _strnicmp(s, "abc", 10); }
size_t f5(char *d, locale_t l) { return strxfrm_l(d, "abc", 10, l); }

Command: cppcheck -q --library=posix --library=windows a.c

PR output:

a.c:5:47: warning: Buffer is accessed out of bounds: "abc" [bufferAccessOutOfBounds]   (strncasecmp)
a.c:7:34: warning: Buffer is accessed out of bounds: "abc" [bufferAccessOutOfBounds]   (strnlen)
a.c:8:45: warning: Buffer is accessed out of bounds: "abc" [bufferAccessOutOfBounds]   (_strnicmp)
a.c:9:54: warning: Buffer is accessed out of bounds: "abc" [bufferAccessOutOfBounds]   (strxfrm_l)

Base: no warnings. All four calls are valid, because these functions stop at the NUL.

Checked and correct:

  • memcmp(s,"abc",10) and write(fd,"abc",10) are now flagged. Both are real overreads.
  • memcpy(d, "ab" "cd", 6) is OK and 5 is OK; I did not test a size large enough to overread this literal.
  • memcpy(d, U"ab", 12) is OK and 13 is flagged.
  • wcsncasecmp(s, L"abc", 10) and wcswidth(L"ab", 10) are not flagged here only because 10 is within the literal's size in bytes. They would be flagged with a larger bound.

Automated review of cd753bf by Claude. The pull request was built and tested locally.

return bufSizeVal;
}
}
} else if (bufTok->tokType() == Token::Type::eString && bufTok->valueType()) {

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants