Repository navigation
fix(libc): reject snprintf capacities above INT_MAX + 1 - #9296
Conversation
…xceeds INT_MAX FreeBSD 11.4 vsnprintf checks the size minus the terminator against INT_MAX before formatting and, when it is larger, sets errno to EOVERFLOW, writes a terminator to the buffer and returns EOF (lib/libc/stdio/vsnprintf.c:61-67). The guest formatted into the buffer and returned the length. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Linux data point for
AI-assisted (Claude Code). |
harisibrar1
left a comment
harisibrar1
left a comment
There was a problem hiding this comment.
Checked against main a6646f8 and FreeBSD 11.4 lib/libc/stdio/snprintf.c / vsnprintf.c:
- FreeBSD decrements a nonzero
nand then rejectsn > INT_MAXwithEOVERFLOW,*str = '\0'andEOF. SoINT_MAX + 1is accepted andINT_MAX + 2and up are rejected, with no clamping and no formatting.SizeOverflowsIntmatches that boundary, the errno value (84) and the single NUL write. The null-buffer guard is the one deviation, and the debt entry documents it. - The check runs before
va_start, so%ntargets are never touched, as on FreeBSD. - Main still has no size check in
snprintforvsnprintf, and this head merges with main cleanly. - Minor: the new debt bullet is separated from the list above it by a blank line, so it renders as its own list.
Reviewed with AI assistance (Claude Code); the checks above were run locally on my machine.
|
Checked the size guard against FreeBSD 11.4 |
4 participants
What
Fixes #8510.
snprintfandvsnprintfcurrently format into a buffer whose declared capacity exceedsINT_MAX + 1. Reject that capacity before reading formatting arguments: return -1, set guest errno toEOVERFLOW(84), and write only the first terminating NUL. PreserveINT_MAX + 1as a valid capacity.Carry @xXPBSXx's offered commit
f676d6d17524ca6a1dec5a0dfe66680fc0b5f65c, preserving authorship. The behaviour follows FreeBSD 11.4 snprintf.c and vsnprintf.c. Console verification and the null-buffer deviation are recorded in libc's debt file.The follow-up extends
guest_formattingto check both exports, narrow and guest-wide formatting,INT_MAX + 2andSIZE_MAXrejection, unchanged%ndestinations and buffer-tail bytes, valid capacities 0/1/INT_MAX/INT_MAX + 1, and null-buffer size queries. Theva_listwrapper also checks that the supplied list is unchanged.Related: #9087 changes secure formatting. This change leaves the secure variants untouched; a nonmutating
git merge-treewith #9087 succeeds.Tested
Windows x64, WinLibs MinGW-w64 GCC 15.2.0, CMake/Ninja, Release,
BUILD_TESTING=ON, full project configuration, rebased onto main99844c7ee.Formatting.cpprestored to maind34da5d69,guest_formattingfails withFormatting check failed. Restoring the fix makes the same test pass.libstarget: passed, including NID-patched library artifacts. After the final rebase, rebuilt the library artifacts and test executables again.patched_libraries,guest_formatting,guest_wide_formatting, the threeguest_standard_streamsmodes, andguest_syslog). Total 2.57 seconds;guest_formatting0.20 seconds.tools/check_conventions.py --base origin/mainandgit diff --check: passed.No console or commercial title was tested; Linux/macOS behaviour was not executed locally.
Overlap review
The documentation-only follow-up moves the snprintf debt entry away from the section-start insertion used by #8937.
git merge-tree --write-treewith #8937 succeeds. C++ source and test contents are unchanged; conventions and whitespace checks pass.Manually reviewed the shared-file diffs of #3965, #4627, #5021, #5303, #6044, #6469, #6879, #7081, #7201, #7447, #7648, #7715, #8150 and #8520. None implements this size check. The other debt edits concern different entries; the shared test changes in #4627, #5021 and #7201 add platform/ABI includes. These are not implementation dependencies.
#6044 reorganizes the tests and deletes the old
core/libs/tests/Formatting.cpppath. That is a genuine future modify/delete integration concern: preserve or portCheckSizeLimitsand its invocation when combining the two PRs. It does not provide functionality this fix requires, soDepends onremains none. The overlap bot may continue to report shared files it cannot compare until those older branches are rebased.Checklist
main; no other open PR implements this capacity check