Bugprone unsafe format string #3
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NagyDonat
reviewed
Nov 10, 2025
NagyDonat
left a comment
NagyDonat
left a comment
There was a problem hiding this comment.
The code looks reasonable overall, but I have a few suggestions in inline comments. I didn't inspect the test files, but based on a brief look they are probably fine as well.
clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp
Outdated
Show resolved
Hide resolved
Comment on lines
+31
to
+32
| anyOf(hasArgument(0, stringLiteral().bind("format")), | ||
| hasArgument(1, stringLiteral().bind("format")))) |
There was a problem hiding this comment.
This logic is a bit hacky -- it would be more elegant if the checker knew that e.g. the format string of sprintf is always at argument index 1, while the format string of scanf is always at argument index 0. However, code that would confuse this check is wildly incorrect, won't occur in the wild and would produce compiler errors (or at least severe warnings), so this is not a serious issue.
clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp
Outdated
Show resolved
Hide resolved
-Only matching global functions in c and std namespace in c++ -Adding C++ namespace tests -Removing fixit hints -Removing standard header includes from the test files and adding function/type definitions -Other small fixes
dkrupp
commented
Nov 11, 2025
Owner
Author
dkrupp
left a comment
dkrupp
left a comment
There was a problem hiding this comment.
Thanks for the review. Your remarks are fixed.
clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp
Outdated
Show resolved
Hide resolved
NagyDonat
reviewed
Nov 12, 2025
NagyDonat
left a comment
NagyDonat
left a comment
There was a problem hiding this comment.
LGTM if you switch to using system header simulators (as far as it's possible).
Comment on lines
5
to
31
| #include <stdarg.h> | ||
|
|
||
| typedef __SIZE_TYPE__ size_t; | ||
| typedef __WCHAR_TYPE__ wchar_t; | ||
| typedef void *FILE; | ||
| extern FILE *stdin; | ||
| extern FILE *stderr; | ||
|
|
||
| extern int fscanf ( FILE * stream, const char * format, ... ); | ||
| extern int scanf ( const char * format, ... ); | ||
| extern int sscanf ( const char * s, const char * format, ...); | ||
| extern int vscanf( const char *restrict format, va_list vlist ); | ||
| extern int vfscanf ( FILE * stream, const char * format, va_list arg ); | ||
|
|
||
| extern int vsscanf( const char *restrict buffer, const char *restrict format, va_list vlist ); | ||
| extern int vwscanf( const wchar_t* format, va_list vlist ); | ||
| extern int vfwscanf( FILE* stream, const wchar_t* format, va_list vlist ); | ||
| extern int vswscanf( const wchar_t* buffer, const wchar_t* format, va_list vlist ); | ||
| extern int swscanf (const wchar_t* ws, const wchar_t* format, ...); | ||
| extern int wscanf( const wchar_t *format, ... ); | ||
| extern int fwscanf( FILE *stream, const wchar_t *format, ... ); | ||
|
|
||
| extern int printf( const char* format, ... ); | ||
| extern int sprintf( char* buffer, const char* format, ... ); | ||
| extern int vsprintf (char * s, const char * format, va_list arg ); | ||
| extern int vsnprintf (char * s, size_t n, const char * format, va_list arg ); | ||
| extern int fprintf( FILE* stream, const char* format, ... ); | ||
| extern int snprintf( char* restrict buffer, size_t bufsz, | ||
| const char* restrict format, ... ); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a new bugprone-unsafe-format-string clang-tidy checker, which warns for scanf and sprintf like functions invocations with a format string literal with unbounded %s specifier that can cause buffer overflow.