https://github.com/dkrupp updated https://github.com/llvm/llvm-project/pull/168691
>From 03433eec012de3ad51e44d0d96721d004ef1c7d7 Mon Sep 17 00:00:00 2001 From: Daniel Krupp <[email protected]> Date: Thu, 30 Oct 2025 13:39:23 +0100 Subject: [PATCH 1/6] [clang-tidy] New bugprone-unsafe-format-string check Adds a new check which warns for the usage of unbounded %s format string specifiers in the sprintf and scanf family functions, which may cause buffer overlfow. --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../bugprone/UnsafeFormatStringCheck.cpp | 149 +++++++++++ .../bugprone/UnsafeFormatStringCheck.h | 34 +++ .../checks/bugprone/unsafe-format-string.rst | 73 ++++++ .../system-header-simulator.h | 57 ++++ .../checkers/bugprone/unsafe-format-string.c | 243 ++++++++++++++++++ .../bugprone/unsafe-format-string.cpp | 58 +++++ 8 files changed, 618 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unsafe-format-string/system-header-simulator.h create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string.c create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 6859dc97c112a..15e8a6c3afb6a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -107,6 +107,7 @@ #include "UnintendedCharOstreamOutputCheck.h" #include "UniquePtrArrayMismatchCheck.h" #include "UnsafeFunctionsCheck.h" +#include "UnsafeFormatStringCheck.h" #include "UnusedLocalNonTrivialVariableCheck.h" #include "UnusedRaiiCheck.h" #include "UnusedReturnValueCheck.h" @@ -308,6 +309,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-crtp-constructor-accessibility"); CheckFactories.registerCheck<UnsafeFunctionsCheck>( "bugprone-unsafe-functions"); + CheckFactories.registerCheck<UnsafeFormatStringCheck>( + "bugprone-unsafe-format-string"); CheckFactories.registerCheck<UnusedLocalNonTrivialVariableCheck>( "bugprone-unused-local-non-trivial-variable"); CheckFactories.registerCheck<UnusedRaiiCheck>("bugprone-unused-raii"); diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index db1256d91d311..0e9439423ce5a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -108,6 +108,7 @@ add_clang_library(clangTidyBugproneModule STATIC UnhandledSelfAssignmentCheck.cpp UniquePtrArrayMismatchCheck.cpp UnsafeFunctionsCheck.cpp + UnsafeFormatStringCheck.cpp UnusedLocalNonTrivialVariableCheck.cpp UnusedRaiiCheck.cpp UnusedReturnValueCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp new file mode 100644 index 0000000000000..cd8f6b85ee1e2 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp @@ -0,0 +1,149 @@ +//===--- UnsafeFormatStringCheck.cpp - clang-tidy -----------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "UnsafeFormatStringCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/Support/ConvertUTF.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +UnsafeFormatStringCheck::UnsafeFormatStringCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + +void UnsafeFormatStringCheck::registerMatchers(MatchFinder *Finder) { + // Matches sprintf and scanf family functions in std namespace in C++ and + // globally in C. + auto VulnerableFunctions = + hasAnyName("sprintf", "vsprintf", "scanf", "fscanf", "sscanf", "vscanf", + "vfscanf", "vsscanf", "wscanf", "fwscanf", "swscanf", + "vwscanf", "vfwscanf", "vswscanf"); + Finder->addMatcher( + callExpr(callee(functionDecl(VulnerableFunctions, + anyOf(isInStdNamespace(), + hasParent(translationUnitDecl())))), + anyOf(hasArgument(0, stringLiteral().bind("format")), + hasArgument(1, stringLiteral().bind("format")))) + .bind("call"), + this); +} + +void UnsafeFormatStringCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call"); + const auto *Format = Result.Nodes.getNodeAs<StringLiteral>("format"); + + if (!Call || !Format) + return; + + std::string FormatString; + if (Format->getCharByteWidth() == 1) { + FormatString = Format->getString().str(); + } else if (Format->getCharByteWidth() == 2) { + // Handle wide strings by converting to narrow string for analysis + convertUTF16ToUTF8String(Format->getBytes(), FormatString); + } else if (Format->getCharByteWidth() == 4) { + // Handle wide strings by converting to narrow string for analysis + convertUTF32ToUTF8String(Format->getBytes(), FormatString); + } + + const auto *Callee = cast<FunctionDecl>(Call->getCalleeDecl()); + StringRef FunctionName = Callee->getName(); + + bool IsScanfFamily = FunctionName.contains("scanf"); + + if (!hasUnboundedStringSpecifier(FormatString, IsScanfFamily)) + return; + + auto Diag = diag(Call->getBeginLoc(), + IsScanfFamily + ? "format specifier '%%s' without field width may cause buffer overflow; consider using '%%Ns' where N limits input length" + : "format specifier '%%s' without precision may cause buffer overflow; consider using '%%.Ns' where N limits output length") + << Call->getSourceRange(); +} + +bool UnsafeFormatStringCheck::hasUnboundedStringSpecifier(StringRef Fmt, + bool IsScanfFamily) { + size_t Pos = 0; + size_t N = Fmt.size(); + while ((Pos = Fmt.find('%', Pos)) != StringRef::npos) { + if (Pos + 1 >= N) + break; + + // Skip %% + if (Fmt[Pos + 1] == '%') { + Pos += 2; + continue; + } + + size_t SpecPos = Pos + 1; + + // Skip flags + while (SpecPos < N && + (Fmt[SpecPos] == '-' || Fmt[SpecPos] == '+' || Fmt[SpecPos] == ' ' || + Fmt[SpecPos] == '#' || Fmt[SpecPos] == '0')) { + SpecPos++; + } + + // Check for field width + bool HasFieldWidth = false; + if (SpecPos < N && Fmt[SpecPos] == '*') { + HasFieldWidth = true; + SpecPos++; + } else { + while (SpecPos < N && isdigit(Fmt[SpecPos])) { + HasFieldWidth = true; + SpecPos++; + } + } + + // Check for precision + bool HasPrecision = false; + if (SpecPos < N && Fmt[SpecPos] == '.') { + SpecPos++; + if (SpecPos < N && Fmt[SpecPos] == '*') { + HasPrecision = true; + SpecPos++; + } else { + while (SpecPos < N && isdigit(Fmt[SpecPos])) { + HasPrecision = true; + SpecPos++; + } + } + } + + // Skip length modifiers + while (SpecPos < N && (Fmt[SpecPos] == 'h' || Fmt[SpecPos] == 'l' || + Fmt[SpecPos] == 'L' || Fmt[SpecPos] == 'z' || + Fmt[SpecPos] == 'j' || Fmt[SpecPos] == 't')) { + SpecPos++; + } + + // Check for 's' specifier + if (SpecPos < N && Fmt[SpecPos] == 's') { + if (IsScanfFamily) { + // For scanf family, field width provides protection + if (!HasFieldWidth) { + return true; + } + } else { + // For sprintf family, only precision provides protection + if (!HasPrecision) { + return true; + } + } + } + + Pos = SpecPos + 1; + } + + return false; +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h new file mode 100644 index 0000000000000..b61c7345a0484 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h @@ -0,0 +1,34 @@ +//===--- UnsafeFormatStringCheck.h - clang-tidy ---------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFEFORMATSTRINGCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFEFORMATSTRINGCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Detects usage of vulnerable format string functions with unbounded %s +/// specifiers that can cause buffer overflows. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/unsafe-format-string.html +class UnsafeFormatStringCheck : public ClangTidyCheck { +public: + UnsafeFormatStringCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + bool hasUnboundedStringSpecifier(StringRef FormatString, bool IsScanfFamily); + std::string getSafeAlternative(StringRef FunctionName); +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFEFORMATSTRINGCHECK_H diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst new file mode 100644 index 0000000000000..9f7ee63fb57f9 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst @@ -0,0 +1,73 @@ +.. title:: clang-tidy - bugprone-unsafe-format-string + +bugprone-unsafe-format-string +============================== + +Detects usage of vulnerable format string functions with unbounded ``%s`` +specifiers that can cause buffer overflows. + +The check identifies calls to format string functions like ``sprintf``, ``scanf``, +and their variants that use ``%s`` format specifiers without proper limits. +This can lead to buffer overflow vulnerabilities when the input string is longer +than the destination buffer. + +Format Specifier Behavior +-------------------------- + +The check distinguishes between different function families: + +**scanf family functions**: Field width limits input length + - ``%s`` - unsafe (no limit) + - ``%99s`` - safe (reads at most 99 characters) + +**sprintf family functions**: Precision limits output length + - ``%s`` - unsafe (no limit) + - ``%99s`` - unsafe (minimum width, no maximum) + - ``%.99s`` - safe (outputs at most 99 characters) + - ``%10.99s`` - safe (minimum 10 chars, maximum 99 chars) + +Examples +-------- + +.. code-block:: c + + char buffer[100]; + const char* input = "user input"; + + // Unsafe sprintf usage + sprintf(buffer, "%s", input); // No limit + sprintf(buffer, "%99s", input); // Field width is minimum, not maximum + + // Safe sprintf usage + sprintf(buffer, "%.99s", input); // Precision limits to 99 chars + sprintf(buffer, "%10.99s", input); // Min 10, max 99 chars + + // Unsafe scanf usage + scanf("%s", buffer); // No limit + + // Safe scanf usage + scanf("%99s", buffer); // Field width limits to 99 chars + + // Safe alternative: use safer functions + snprintf(buffer, sizeof(buffer), "%s", input); + +Checked Functions +----------------- + +The check detects unsafe format strings in these functions: + +**sprintf family** (precision ``.N`` provides safety): +* ``sprintf``, ``vsprintf`` + +**scanf family** (field width ``N`` provides safety): +* ``scanf``, ``fscanf``, ``sscanf`` +* ``vscanf``, ``vfscanf``, ``vsscanf`` +* ``wscanf``, ``fwscanf``, ``swscanf`` +* ``vwscanf``, ``vfwscanf``, ``vswscanf`` + +Recommendations +--------------- + +* For ``sprintf`` family: Use precision specifiers (``%.Ns``) or ``snprintf`` +* For ``scanf`` family: Use field width specifiers (``%Ns``) +* Consider using safer string handling functions when possible diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unsafe-format-string/system-header-simulator.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unsafe-format-string/system-header-simulator.h new file mode 100644 index 0000000000000..fef2c10ae339a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unsafe-format-string/system-header-simulator.h @@ -0,0 +1,57 @@ +#pragma clang system_header + +#ifdef __cplusplus +#define restrict /*restrict*/ +#endif + +#ifndef __cplusplus +typedef __WCHAR_TYPE__ wchar_t; +#endif + +typedef __typeof(sizeof(int)) size_t; +typedef long long __int64_t; +typedef __int64_t __darwin_off_t; +typedef __darwin_off_t fpos_t; +typedef int off_t; +typedef long ssize_t; + +typedef struct _FILE FILE; + +extern FILE *stdin; +extern FILE *stdout; +extern FILE *stderr; + +typedef __builtin_va_list va_list; +#define va_start(ap, param) __builtin_va_start(ap, param) +#define va_end(ap) __builtin_va_end(ap) +#define va_arg(ap, type) __builtin_va_arg(ap, type) +#define va_copy(dst, src) __builtin_va_copy(dst, src) + + +#ifdef __cplusplus +namespace std { +#endif +extern int fscanf ( FILE *restrict stream, const char *restrict format, ... ); +extern int scanf ( const char *restrict format, ... ); +extern int sscanf ( const char *restrict s, const char *restrict format, ...); +extern int vscanf( const char *restrict format, va_list vlist ); +extern int vfscanf ( FILE *restrict stream, const char *restrict 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, ... ); +#ifdef __cplusplus +} //namespace std { +#endif \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string.c new file mode 100644 index 0000000000000..2e3077ecab35a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string.c @@ -0,0 +1,243 @@ +// RUN: %check_clang_tidy %s bugprone-unsafe-format-string %t -- -- -isystem %S/Inputs/unsafe-format-string + +#include <system-header-simulator.h> + +void test_sprintf() { + char buffer[100]; + const char* input = "user input"; + + /* Positive: unsafe %s without field width */ + sprintf(buffer, "%s", input); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without precision may cause buffer overflow; consider using '%.Ns' where N limits output length [bugprone-unsafe-format-string] + + /* Positive: field width doesn't prevent overflow in sprintf */ + sprintf(buffer, "%99s", input); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without precision may cause buffer overflow; consider using '%.Ns' where N limits output length [bugprone-unsafe-format-string] + + /* Positive: dynamic field width doesn't prevent overflow */ + sprintf(buffer, "%*s", 10, input); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without precision may cause buffer overflow; consider using '%.Ns' where N limits output length [bugprone-unsafe-format-string] + + /*Negative: precision limits string length */ + sprintf(buffer, "%.99s", input); + /* no-warning */ + + /*Negative: precision with field width */ + sprintf(buffer, "%1.99s", input); + /* no-warning */ + + /*Negative: dynamic precision */ + sprintf(buffer, "%.*s", 99, input); + /* no-warning */ + + /*Negative: field width with dynamic precision */ + sprintf(buffer, "%1.*s", 99, input); + /* no-warning */ + + /*Negative: dynamic field width with fixed precision */ + sprintf(buffer, "%*.99s", 10, input); + /* no-warning */ + + /*Negative: dynamic field width and precision */ + sprintf(buffer, "%*.*s", 10, 99, input); + /* no-warning */ + + /*Negative: other format specifiers are safe */ + sprintf(buffer, "%d %f", 42, 3.14); + /* no-warning */ +} + +void test_vsprintf() { + char buffer[100]; + va_list args; + + /* Positive: unsafe %s without field width */ + vsprintf(buffer, "%s", args); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without precision may cause buffer overflow; consider using '%.Ns' where N limits output length [bugprone-unsafe-format-string] + + /* Positive: field width doesn't prevent overflow in vsprintf */ + vsprintf(buffer, "%99s", args); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without precision may cause buffer overflow; consider using '%.Ns' where N limits output length [bugprone-unsafe-format-string] + + /* Positive: precision limits string length */ + vsprintf(buffer, "%.99s", args); + /* no-warning */ +} + +void test_vsnprintf(int count, ...) { + va_list args; + va_start(args, count); + char buffer[100]; + + /*Negative: vsnprintf is safe */ + vsnprintf(buffer, sizeof(buffer), "%99s", args); + /* no-warning */ + + va_end(args); +} + +void test_scanf() { + char buffer[100]; + + /* Positive: unsafe %s without field width */ + scanf("%s", buffer); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + scanf("%99s", buffer); + /* no-warning */ +} + +void test_fscanf() { + char buffer[100]; + FILE* file = 0; + + /* Positive: unsafe %s without field width */ + fscanf(file, "%s", buffer); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + fscanf(file, "%99s", buffer); + /* no-warning */ +} + +void test_sscanf(char *source) { + char buffer[100]; + + /* Positive: unsafe %s without field width */ + sscanf(source, "%s", buffer); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + sscanf(source, "%99s", buffer); + /* no-warning */ +} + +void test_vfscanf() { + FILE* file = 0; + va_list args; + + /* Positive: unsafe %s without field width */ + vfscanf(file, "%s", args); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + vfscanf(file, "%99s", args); + /* no-warning */ +} + +void test_vsscanf(char * source) { + va_list args; + + /* Positive: unsafe %s without field width */ + vsscanf(source, "%s", args); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + vsscanf(source, "%99s", args); + /* no-warning */ +} + +void test_vscanf() { + va_list args; + + /* Positive: unsafe %s without field width */ + vscanf("%s", args); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + vscanf("%99s", args); + /* no-warning */ +} + +void test_wscanf() { + wchar_t buffer[100]; + + /* Positive: unsafe %s without field width */ + wscanf(L"%s", buffer); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + wscanf(L"%99s", buffer); + /* no-warning */ +} + +void test_fwscanf() { + wchar_t buffer[100]; + FILE* file = 0; + + /* Positive: unsafe %s without field width */ + fwscanf(file, L"%s", buffer); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + fwscanf(file, L"%99s", buffer); + /* no-warning */ +} + +void test_swscanf(wchar_t *source) { + wchar_t buffer[100]; + + /* Positive: unsafe %s without field width */ + swscanf(source, L"%s", buffer); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + swscanf(source, L"%99s", buffer); + /* no-warning */ +} + +void test_vwscanf() { + va_list args; + + /* Positive: unsafe %s without field width */ + vwscanf(L"%s", args); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + vwscanf(L"%99s", args); + /* no-warning */ +} + +void test_vfwscanf() { + FILE* file = 0; + va_list args; + + /* Positive: unsafe %s without field width */ + vfwscanf(file, L"%s", args); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + vfwscanf(file, L"%99s", args); + /* no-warning */ +} + +void test_vswscanf() { + const wchar_t* source = L"input"; + va_list args; + + /* Positive: unsafe %s without field width */ + vswscanf(source, L"%s", args); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + vswscanf(source, L"%99s", args); + /* no-warning */ +} + +void test_safe_alternatives() { + char buffer[100]; + const char* input = "user input"; + + /*Negative: snprintf is inherently safe */ + snprintf(buffer, sizeof(buffer), "%s", input); + /* no-warning */ + + /*Negative: printf family doesn't write to buffers */ + printf("%s", input); + /* no-warning */ + + /*Negative: fprintf doesn't write to user buffers */ + fprintf(stderr, "%s", input); + /* no-warning */ +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string.cpp new file mode 100644 index 0000000000000..07117e37b7521 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string.cpp @@ -0,0 +1,58 @@ +// RUN: %check_clang_tidy %s bugprone-unsafe-format-string %t -- -- -isystem %S/Inputs/unsafe-format-string + +#include <system-header-simulator.h> + +class TestClass{ + int sprintf( char* buffer, const char* format, ... ); + void test_sprintf() { + char buffer[100]; + const char* input = "user input"; + /* Negative: no warning for calling member functions */ + sprintf(buffer, "%s", input); + } +}; + +void test_sprintf() { + char buffer[100]; + const char* input = "user input"; + + /* Positive: unsafe %s without field width */ + std::sprintf(buffer, "%s", input); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without precision may cause buffer overflow; consider using '%.Ns' where N limits output length [bugprone-unsafe-format-string] + + /* Positive: field width doesn't prevent overflow in sprintf */ + std::sprintf(buffer, "%99s", input); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without precision may cause buffer overflow; consider using '%.Ns' where N limits output length [bugprone-unsafe-format-string] + + /* Positive: dynamic field width doesn't prevent overflow */ + std::sprintf(buffer, "%*s", 10, input); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without precision may cause buffer overflow; consider using '%.Ns' where N limits output length [bugprone-unsafe-format-string] + + /*Negative: precision limits string length */ + std::sprintf(buffer, "%.99s", input); + /* no-warning */ + + /*Negative: precision with field width */ + std::sprintf(buffer, "%1.99s", input); + /* no-warning */ + + /*Negative: dynamic precision */ + std::sprintf(buffer, "%.*s", 99, input); + /* no-warning */ + + /*Negative: field width with dynamic precision */ + std::sprintf(buffer, "%1.*s", 99, input); + /* no-warning */ + + /*Negative: dynamic field width with fixed precision */ + std::sprintf(buffer, "%*.99s", 10, input); + /* no-warning */ + + /*Negative: dynamic field width and precision */ + std::sprintf(buffer, "%*.*s", 10, 99, input); + /* no-warning */ + + /*Negative: other format specifiers are safe */ + std::sprintf(buffer, "%d %f", 42, 3.14); + /* no-warning */ +} >From cefe2742bab0507b774abf635eb3679cf7fc235d Mon Sep 17 00:00:00 2001 From: Daniel Krupp <[email protected]> Date: Wed, 19 Nov 2025 12:04:37 +0100 Subject: [PATCH 2/6] Fixup formmatting --- .../clang-tidy/bugprone/BugproneTidyModule.cpp | 2 +- .../bugprone/UnsafeFormatStringCheck.cpp | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 15e8a6c3afb6a..940d13bf60a18 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -106,8 +106,8 @@ #include "UnhandledSelfAssignmentCheck.h" #include "UnintendedCharOstreamOutputCheck.h" #include "UniquePtrArrayMismatchCheck.h" -#include "UnsafeFunctionsCheck.h" #include "UnsafeFormatStringCheck.h" +#include "UnsafeFunctionsCheck.h" #include "UnusedLocalNonTrivialVariableCheck.h" #include "UnusedRaiiCheck.h" #include "UnusedReturnValueCheck.h" diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp index cd8f6b85ee1e2..48a05aa27076c 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp @@ -61,11 +61,15 @@ void UnsafeFormatStringCheck::check(const MatchFinder::MatchResult &Result) { if (!hasUnboundedStringSpecifier(FormatString, IsScanfFamily)) return; - auto Diag = diag(Call->getBeginLoc(), - IsScanfFamily - ? "format specifier '%%s' without field width may cause buffer overflow; consider using '%%Ns' where N limits input length" - : "format specifier '%%s' without precision may cause buffer overflow; consider using '%%.Ns' where N limits output length") - << Call->getSourceRange(); + auto Diag = + diag( + Call->getBeginLoc(), + IsScanfFamily + ? "format specifier '%%s' without field width may cause buffer " + "overflow; consider using '%%Ns' where N limits input length" + : "format specifier '%%s' without precision may cause buffer " + "overflow; consider using '%%.Ns' where N limits output length") + << Call->getSourceRange(); } bool UnsafeFormatStringCheck::hasUnboundedStringSpecifier(StringRef Fmt, >From c5b8b1b2f186e86c7284540067c33d88694b4b45 Mon Sep 17 00:00:00 2001 From: Daniel Krupp <[email protected]> Date: Wed, 19 Nov 2025 12:14:41 +0100 Subject: [PATCH 3/6] Fixup. Fixing clang-tidy finding --- clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h index b61c7345a0484..829a0858b0dcd 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h @@ -25,7 +25,7 @@ class UnsafeFormatStringCheck : public ClangTidyCheck { void check(const ast_matchers::MatchFinder::MatchResult &Result) override; private: - bool hasUnboundedStringSpecifier(StringRef FormatString, bool IsScanfFamily); + bool hasUnboundedStringSpecifier(StringRef Fmt, bool IsScanfFamily); std::string getSafeAlternative(StringRef FunctionName); }; >From e88df90cd67392f3ea4ce591df5a88913ed8554e Mon Sep 17 00:00:00 2001 From: Daniel Krupp <[email protected]> Date: Wed, 26 Nov 2025 20:10:57 +0100 Subject: [PATCH 4/6] Review fixes --- .../bugprone/BugproneTidyModule.cpp | 4 +-- .../clang-tidy/bugprone/CMakeLists.txt | 2 +- .../bugprone/UnsafeFormatStringCheck.cpp | 34 +++++++++---------- .../bugprone/UnsafeFormatStringCheck.h | 4 +-- .../checks/bugprone/unsafe-format-string.rst | 4 +-- .../system-header-simulator.h | 2 +- 6 files changed, 24 insertions(+), 26 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 940d13bf60a18..367504623cee6 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -307,10 +307,10 @@ class BugproneModule : public ClangTidyModule { "bugprone-unique-ptr-array-mismatch"); CheckFactories.registerCheck<CrtpConstructorAccessibilityCheck>( "bugprone-crtp-constructor-accessibility"); - CheckFactories.registerCheck<UnsafeFunctionsCheck>( - "bugprone-unsafe-functions"); CheckFactories.registerCheck<UnsafeFormatStringCheck>( "bugprone-unsafe-format-string"); + CheckFactories.registerCheck<UnsafeFunctionsCheck>( + "bugprone-unsafe-functions"); CheckFactories.registerCheck<UnusedLocalNonTrivialVariableCheck>( "bugprone-unused-local-non-trivial-variable"); CheckFactories.registerCheck<UnusedRaiiCheck>("bugprone-unused-raii"); diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 0e9439423ce5a..88b776d9239d8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -107,8 +107,8 @@ add_clang_library(clangTidyBugproneModule STATIC UnhandledExceptionAtNewCheck.cpp UnhandledSelfAssignmentCheck.cpp UniquePtrArrayMismatchCheck.cpp - UnsafeFunctionsCheck.cpp UnsafeFormatStringCheck.cpp + UnsafeFunctionsCheck.cpp UnusedLocalNonTrivialVariableCheck.cpp UnusedRaiiCheck.cpp UnusedReturnValueCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp index 48a05aa27076c..72c34589956d8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp @@ -1,4 +1,4 @@ -//===--- UnsafeFormatStringCheck.cpp - clang-tidy -----------------------===// +//===----------------------------------------------------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -26,11 +26,12 @@ void UnsafeFormatStringCheck::registerMatchers(MatchFinder *Finder) { "vfscanf", "vsscanf", "wscanf", "fwscanf", "swscanf", "vwscanf", "vfwscanf", "vswscanf"); Finder->addMatcher( - callExpr(callee(functionDecl(VulnerableFunctions, - anyOf(isInStdNamespace(), - hasParent(translationUnitDecl())))), - anyOf(hasArgument(0, stringLiteral().bind("format")), - hasArgument(1, stringLiteral().bind("format")))) + callExpr( + callee(functionDecl(VulnerableFunctions, + anyOf(isInStdNamespace(), + hasDeclContext(translationUnitDecl())))), + anyOf(hasArgument(0, stringLiteral().bind("format")), + hasArgument(1, stringLiteral().bind("format")))) .bind("call"), this); } @@ -39,8 +40,7 @@ void UnsafeFormatStringCheck::check(const MatchFinder::MatchResult &Result) { const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call"); const auto *Format = Result.Nodes.getNodeAs<StringLiteral>("format"); - if (!Call || !Format) - return; + assert(Call && Format); std::string FormatString; if (Format->getCharByteWidth() == 1) { @@ -54,21 +54,19 @@ void UnsafeFormatStringCheck::check(const MatchFinder::MatchResult &Result) { } const auto *Callee = cast<FunctionDecl>(Call->getCalleeDecl()); - StringRef FunctionName = Callee->getName(); + const StringRef FunctionName = Callee->getName(); - bool IsScanfFamily = FunctionName.contains("scanf"); + const bool IsScanfFamily = FunctionName.contains("scanf"); if (!hasUnboundedStringSpecifier(FormatString, IsScanfFamily)) return; - auto Diag = - diag( - Call->getBeginLoc(), - IsScanfFamily - ? "format specifier '%%s' without field width may cause buffer " - "overflow; consider using '%%Ns' where N limits input length" - : "format specifier '%%s' without precision may cause buffer " - "overflow; consider using '%%.Ns' where N limits output length") + diag(Call->getBeginLoc(), + IsScanfFamily + ? "format specifier '%%s' without field width may cause buffer " + "overflow; consider using '%%Ns' where N limits input length" + : "format specifier '%%s' without precision may cause buffer " + "overflow; consider using '%%.Ns' where N limits output length") << Call->getSourceRange(); } diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h index 829a0858b0dcd..169ffcf13be8c 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h @@ -1,4 +1,4 @@ -//===--- UnsafeFormatStringCheck.h - clang-tidy ---------------*- C++ -*-===// +//===------------------------------------------------------------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -17,7 +17,7 @@ namespace clang::tidy::bugprone { /// specifiers that can cause buffer overflows. /// /// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/unsafe-format-string.html +/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/unsafe-format-string.html class UnsafeFormatStringCheck : public ClangTidyCheck { public: UnsafeFormatStringCheck(StringRef Name, ClangTidyContext *Context); diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst index 9f7ee63fb57f9..e9724d9709bdf 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst @@ -1,7 +1,7 @@ .. title:: clang-tidy - bugprone-unsafe-format-string bugprone-unsafe-format-string -============================== +============================= Detects usage of vulnerable format string functions with unbounded ``%s`` specifiers that can cause buffer overflows. @@ -12,7 +12,7 @@ This can lead to buffer overflow vulnerabilities when the input string is longer than the destination buffer. Format Specifier Behavior --------------------------- +------------------------- The check distinguishes between different function families: diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unsafe-format-string/system-header-simulator.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unsafe-format-string/system-header-simulator.h index fef2c10ae339a..6cc0bf799a930 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unsafe-format-string/system-header-simulator.h +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unsafe-format-string/system-header-simulator.h @@ -54,4 +54,4 @@ extern int snprintf( char* restrict buffer, size_t bufsz, const char* restrict format, ... ); #ifdef __cplusplus } //namespace std { -#endif \ No newline at end of file +#endif >From 272e66e5f78afc978eb7dce831a05cca52ad59ac Mon Sep 17 00:00:00 2001 From: Daniel Krupp <[email protected]> Date: Tue, 2 Dec 2025 22:10:34 +0100 Subject: [PATCH 5/6] Add configuration options to the checker CustomPrintfFunctions and CustomScanfFunctions are added to the checker where the user can define custom dangerous scanf/printf like functions with the function_name, stringliteralcount parameter pairs. --- .../bugprone/UnsafeFormatStringCheck.cpp | 136 ++++++++++++++++-- .../bugprone/UnsafeFormatStringCheck.h | 13 ++ .../checks/bugprone/unsafe-format-string.rst | 38 +++++ .../docs/clang-tidy/checks/list.rst | 1 + .../unsafe-format-string-custom-config.c | 33 +++++ 5 files changed, 208 insertions(+), 13 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-custom-config.c diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp index 72c34589956d8..ffa6f2bb4cd54 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp @@ -7,16 +7,61 @@ //===----------------------------------------------------------------------===// #include "UnsafeFormatStringCheck.h" +#include "../utils/OptionsUtils.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "llvm/Support/ConvertUTF.h" +#include <string> using namespace clang::ast_matchers; namespace clang::tidy::bugprone { +static constexpr llvm::StringLiteral OptionNameCustomPrintfFunctions = + "CustomPrintfFunctions"; +static constexpr llvm::StringLiteral OptionNameCustomScanfFunctions = + "CustomScanfFunctions"; + +static constexpr llvm::StringLiteral BuiltInFormatBind = "format"; +static constexpr llvm::StringLiteral BuiltInCallBind = "call"; +static constexpr llvm::StringLiteral PrintfCallBind = "printfcall"; +static constexpr llvm::StringLiteral ScanfCallBind = "scanfcall"; + +static std::vector<UnsafeFormatStringCheck::CheckedFunction> +parseCheckedFunctions(StringRef Option, ClangTidyContext *Context) { + const std::vector<StringRef> Functions = + utils::options::parseStringList(Option); + std::vector<UnsafeFormatStringCheck::CheckedFunction> Result; + Result.reserve(Functions.size()); + + for (const StringRef Function : Functions) { + if (Function.empty()) + continue; + const auto [Name, ParamCount] = Function.split(','); + unsigned long Count = 0; + if (Name.trim().empty() || ParamCount.trim().empty() || + ParamCount.trim().getAsInteger(10, Count)) { + Context->configurationDiag( + "invalid configuration value for option '%0'; " + "expected <functionname>, <paramcount>; pairs.") + << OptionNameCustomPrintfFunctions; + continue; + } + Result.push_back( + {Name.trim().str(), + matchers::MatchesAnyListedNameMatcher::NameMatcher(Name.trim()), + Count}); + } + + return Result; +} + UnsafeFormatStringCheck::UnsafeFormatStringCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + : ClangTidyCheck(Name, Context), + CustomPrintfFunctions(parseCheckedFunctions( + Options.get(OptionNameCustomPrintfFunctions, ""), Context)), + CustomScanfFunctions(parseCheckedFunctions( + Options.get(OptionNameCustomScanfFunctions, ""), Context)) {} void UnsafeFormatStringCheck::registerMatchers(MatchFinder *Finder) { // Matches sprintf and scanf family functions in std namespace in C++ and @@ -30,17 +75,87 @@ void UnsafeFormatStringCheck::registerMatchers(MatchFinder *Finder) { callee(functionDecl(VulnerableFunctions, anyOf(isInStdNamespace(), hasDeclContext(translationUnitDecl())))), - anyOf(hasArgument(0, stringLiteral().bind("format")), - hasArgument(1, stringLiteral().bind("format")))) - .bind("call"), + anyOf(hasArgument(0, stringLiteral().bind(BuiltInFormatBind)), + hasArgument(1, stringLiteral().bind(BuiltInFormatBind)))) + .bind(BuiltInCallBind), this); + + if (!CustomPrintfFunctions.empty()) { + std::vector<llvm::StringRef> FunctionNames; + FunctionNames.reserve(CustomPrintfFunctions.size()); + + for (const auto &Entry : CustomPrintfFunctions) + FunctionNames.emplace_back(Entry.Name); + + auto CustomFunctionsMatcher = matchers::matchesAnyListedName(FunctionNames); + + Finder->addMatcher(callExpr(callee((functionDecl(CustomFunctionsMatcher)))) + .bind(PrintfCallBind), + this); + } + + if (!CustomScanfFunctions.empty()) { + std::vector<llvm::StringRef> FunctionNames; + FunctionNames.reserve(CustomScanfFunctions.size()); + + for (const auto &Entry : CustomScanfFunctions) + FunctionNames.emplace_back(Entry.Name); + + auto CustomFunctionsMatcher = matchers::matchesAnyListedName(FunctionNames); + + Finder->addMatcher(callExpr(callee((functionDecl(CustomFunctionsMatcher)))) + .bind(ScanfCallBind), + this); + } +} + +void UnsafeFormatStringCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, OptionNameCustomPrintfFunctions, ""); + Options.store(Opts, OptionNameCustomScanfFunctions, ""); +} + +const StringLiteral *UnsafeFormatStringCheck::getFormatLiteral( + const CallExpr *Call, const std::vector<CheckedFunction> &CustomFunctions) { + const FunctionDecl *FD = cast<FunctionDecl>(Call->getDirectCallee()); + if (!FD) + return nullptr; + for (const auto &Entry : CustomFunctions) { + if (Entry.Pattern.match(*FD)) { + if (Entry.FormatStringLocation >= Call->getNumArgs()) + return nullptr; + const Expr *Arg = + Call->getArg(Entry.FormatStringLocation)->IgnoreImpCasts(); + return dyn_cast<StringLiteral>(Arg); + } + } + return nullptr; } void UnsafeFormatStringCheck::check(const MatchFinder::MatchResult &Result) { - const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call"); - const auto *Format = Result.Nodes.getNodeAs<StringLiteral>("format"); + const CallExpr *Call; + const StringLiteral *Format; + bool IsScanfFamily = false; + if (Result.Nodes.getNodeAs<CallExpr>(BuiltInCallBind)) { + Call = Result.Nodes.getNodeAs<CallExpr>(BuiltInCallBind); + Format = Result.Nodes.getNodeAs<StringLiteral>(BuiltInFormatBind); + const auto *Callee = cast<FunctionDecl>(Call->getCalleeDecl()); + const StringRef FunctionName = Callee->getName(); + IsScanfFamily = FunctionName.contains("scanf"); + } else if (Result.Nodes.getNodeAs<CallExpr>(PrintfCallBind)) { + Call = Result.Nodes.getNodeAs<CallExpr>(PrintfCallBind); + Format = getFormatLiteral(Call, CustomPrintfFunctions); + IsScanfFamily = false; + } else if (Result.Nodes.getNodeAs<CallExpr>(ScanfCallBind)) { + Call = Result.Nodes.getNodeAs<CallExpr>(ScanfCallBind); + Format = getFormatLiteral(Call, CustomScanfFunctions); + IsScanfFamily = true; + } else { + Call = nullptr; + Format = nullptr; + } - assert(Call && Format); + if (!Call || !Format) + return; std::string FormatString; if (Format->getCharByteWidth() == 1) { @@ -53,11 +168,6 @@ void UnsafeFormatStringCheck::check(const MatchFinder::MatchResult &Result) { convertUTF32ToUTF8String(Format->getBytes(), FormatString); } - const auto *Callee = cast<FunctionDecl>(Call->getCalleeDecl()); - const StringRef FunctionName = Callee->getName(); - - const bool IsScanfFamily = FunctionName.contains("scanf"); - if (!hasUnboundedStringSpecifier(FormatString, IsScanfFamily)) return; @@ -73,7 +183,7 @@ void UnsafeFormatStringCheck::check(const MatchFinder::MatchResult &Result) { bool UnsafeFormatStringCheck::hasUnboundedStringSpecifier(StringRef Fmt, bool IsScanfFamily) { size_t Pos = 0; - size_t N = Fmt.size(); + const size_t N = Fmt.size(); while ((Pos = Fmt.find('%', Pos)) != StringRef::npos) { if (Pos + 1 >= N) break; diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h index 169ffcf13be8c..98ada92b3ae36 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFEFORMATSTRINGCHECK_H #include "../ClangTidyCheck.h" +#include "../utils/Matchers.h" namespace clang::tidy::bugprone { @@ -21,10 +22,22 @@ namespace clang::tidy::bugprone { class UnsafeFormatStringCheck : public ClangTidyCheck { public: UnsafeFormatStringCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + struct CheckedFunction { + std::string Name; + matchers::MatchesAnyListedNameMatcher::NameMatcher Pattern; + unsigned long FormatStringLocation; + }; + private: + const std::vector<CheckedFunction> CustomPrintfFunctions; + const std::vector<CheckedFunction> CustomScanfFunctions; + const StringLiteral * + getFormatLiteral(const CallExpr *, + const std::vector<CheckedFunction> &CustomFunctions); bool hasUnboundedStringSpecifier(StringRef Fmt, bool IsScanfFamily); std::string getSafeAlternative(StringRef FunctionName); }; diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst index e9724d9709bdf..c0b8c0c5f06e8 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst @@ -51,6 +51,7 @@ Examples // Safe alternative: use safer functions snprintf(buffer, sizeof(buffer), "%s", input); + Checked Functions ----------------- @@ -65,6 +66,43 @@ The check detects unsafe format strings in these functions: * ``wscanf``, ``fwscanf``, ``swscanf`` * ``vwscanf``, ``vfwscanf``, ``vswscanf`` +Configuration +------------- + +The checker offers 2 configuration options. + +* `CustomPrintfFunctions` The user can specify own printf-like functions with dangerous format string parameter. +* `CustomScanfFunctions` The user can specify own scanf-like functions with dangerous format string parameter. + +Format: +Both options have the following format. +.. code:: + + bugprone-unsafe-functions.CustomPrintfFunctions=" + functionRegex1, format-string-position; + functionRegex2, format-string-position; + ... + " + +The first parameter in the pairs is a function regular expression matching the function name, +the second parameter is the count of the format string literal argument. + +The following configuration will give warning: + +.. code:: + + bugprone-unsafe-format-string.CustomPrintfFunctions: 'mysprintf, 0;' + bugprone-unsafe-format-string.CustomScanfFunctions: 'myscanf, 1;' + + extern int myscanf( const char* format, ... ); + extern int mysprintf( char* buffer, const char* format, ... ); + void test() { + char buffer[100]; + const char* input = "user input"; + mysprintf(buffer, "%s", input); // warning + myscanf("%s", buffer); //warning + } + Recommendations --------------- diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 8bb112f3d1832..f9a59694243c2 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -174,6 +174,7 @@ Clang-Tidy Checks :doc:`bugprone-unhandled-self-assignment <bugprone/unhandled-self-assignment>`, :doc:`bugprone-unintended-char-ostream-output <bugprone/unintended-char-ostream-output>`, "Yes" :doc:`bugprone-unique-ptr-array-mismatch <bugprone/unique-ptr-array-mismatch>`, "Yes" + :doc:`bugprone-unsafe-format-string <bugprone/unsafe-format-string>`, :doc:`bugprone-unsafe-functions <bugprone/unsafe-functions>`, :doc:`bugprone-unused-local-non-trivial-variable <bugprone/unused-local-non-trivial-variable>`, :doc:`bugprone-unused-raii <bugprone/unused-raii>`, "Yes" diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-custom-config.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-custom-config.c new file mode 100644 index 0000000000000..d817ac384fd37 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-custom-config.c @@ -0,0 +1,33 @@ +// RUN: %check_clang_tidy %s bugprone-unsafe-format-string %t --\ +// RUN: -config="{CheckOptions: {bugprone-unsafe-format-string.CustomPrintfFunctions: 'mysprintf, 1;', bugprone-unsafe-format-string.CustomScanfFunctions: 'myscanf, 0;' }}"\ +// RUN: -- -isystem %S/Inputs/unsafe-format-string + +#include <system-header-simulator.h> + +extern int myscanf( const char* format, ... ); +extern int mysprintf( char* buffer, const char* format, ... ); + +void test_sprintf() { + char buffer[100]; + const char* input = "user input"; + + /* Positive: unsafe %s without field width */ + mysprintf(buffer, "%s", input); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without precision may cause buffer overflow; consider using '%.Ns' where N limits output length [bugprone-unsafe-format-string] + + mysprintf(buffer, "%.99s", input); + /*no warning*/ +} + + +void test_scanf() { + char buffer[100]; + + /* Positive: unsafe %s without field width */ + myscanf("%s", buffer); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: format specifier '%s' without field width may cause buffer overflow; consider using '%Ns' where N limits input length [bugprone-unsafe-format-string] + + /*Negative: safe %s with field width */ + myscanf("%99s", buffer); + /* no-warning */ +} \ No newline at end of file >From 6fa63afcf82f0c99c58da9abf73308993f825f98 Mon Sep 17 00:00:00 2001 From: Daniel Krupp <[email protected]> Date: Tue, 2 Dec 2025 22:21:08 +0100 Subject: [PATCH 6/6] fixup --- .../clang-tidy/bugprone/UnsafeFormatStringCheck.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h index 98ada92b3ae36..e648b710421cd 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h @@ -38,8 +38,7 @@ class UnsafeFormatStringCheck : public ClangTidyCheck { const StringLiteral * getFormatLiteral(const CallExpr *, const std::vector<CheckedFunction> &CustomFunctions); - bool hasUnboundedStringSpecifier(StringRef Fmt, bool IsScanfFamily); - std::string getSafeAlternative(StringRef FunctionName); + bool hasUnboundedStringSpecifier(StringRef Fmt, bool IsScanfFamily); }; } // namespace clang::tidy::bugprone _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
