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 01/12] [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 02/12] 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 03/12] 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 04/12] 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 05/12] 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 06/12] 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 >From ab4bf4a5d61728df1b8c6e2c747aaf68eeca7131 Mon Sep 17 00:00:00 2001 From: Daniel Krupp <[email protected]> Date: Tue, 2 Dec 2025 22:27:16 +0100 Subject: [PATCH 07/12] fixup --- .../clang-tidy/bugprone/UnsafeFormatStringCheck.cpp | 9 ++++++--- .../clang-tidy/bugprone/UnsafeFormatStringCheck.h | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp index ffa6f2bb4cd54..afe5cfe6713cc 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp @@ -143,11 +143,13 @@ void UnsafeFormatStringCheck::check(const MatchFinder::MatchResult &Result) { IsScanfFamily = FunctionName.contains("scanf"); } else if (Result.Nodes.getNodeAs<CallExpr>(PrintfCallBind)) { Call = Result.Nodes.getNodeAs<CallExpr>(PrintfCallBind); - Format = getFormatLiteral(Call, CustomPrintfFunctions); + Format = + UnsafeFormatStringCheck::getFormatLiteral(Call, CustomPrintfFunctions); IsScanfFamily = false; } else if (Result.Nodes.getNodeAs<CallExpr>(ScanfCallBind)) { Call = Result.Nodes.getNodeAs<CallExpr>(ScanfCallBind); - Format = getFormatLiteral(Call, CustomScanfFunctions); + Format = + UnsafeFormatStringCheck::getFormatLiteral(Call, CustomScanfFunctions); IsScanfFamily = true; } else { Call = nullptr; @@ -168,7 +170,8 @@ void UnsafeFormatStringCheck::check(const MatchFinder::MatchResult &Result) { convertUTF32ToUTF8String(Format->getBytes(), FormatString); } - if (!hasUnboundedStringSpecifier(FormatString, IsScanfFamily)) + if (!UnsafeFormatStringCheck::hasUnboundedStringSpecifier(FormatString, + IsScanfFamily)) return; diag(Call->getBeginLoc(), diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h index e648b710421cd..9fed8f067c871 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h @@ -35,10 +35,10 @@ class UnsafeFormatStringCheck : public ClangTidyCheck { private: const std::vector<CheckedFunction> CustomPrintfFunctions; const std::vector<CheckedFunction> CustomScanfFunctions; - const StringLiteral * + static const StringLiteral * getFormatLiteral(const CallExpr *, const std::vector<CheckedFunction> &CustomFunctions); - bool hasUnboundedStringSpecifier(StringRef Fmt, bool IsScanfFamily); + static bool hasUnboundedStringSpecifier(StringRef Fmt, bool IsScanfFamily); }; } // namespace clang::tidy::bugprone >From 86aab9315a1cb01b6481d5640c0608dd6c6b638f Mon Sep 17 00:00:00 2001 From: Daniel Krupp <[email protected]> Date: Tue, 2 Dec 2025 22:53:53 +0100 Subject: [PATCH 08/12] Extend Release Notes --- clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index dc8abd88899a4..7543c07cdca5e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -202,6 +202,12 @@ New checks Detects default initialization (to 0) of variables with ``enum`` type where the enum has no enumerator with value of 0. +- New :doc:`bugprone-unsafe-format-string + <clang-tidy/checks/bugprone/unsafe-format-string>` check. + + Detects usage of vulnerable printf and scanf-like format + string functions with unbounded ``%s`` specifiers that can cause buffer overflows. + - New :doc:`cppcoreguidelines-pro-bounds-avoid-unchecked-container-access <clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-access>` check. >From 52b9826033fc1385379c0412fc3a883ad75c5032 Mon Sep 17 00:00:00 2001 From: Daniel Krupp <[email protected]> Date: Tue, 2 Dec 2025 23:11:55 +0100 Subject: [PATCH 09/12] Fix linter error --- .../clang-tidy/bugprone/UnsafeFormatStringCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp index afe5cfe6713cc..ad4b9756a53e9 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp @@ -116,7 +116,7 @@ void UnsafeFormatStringCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { const StringLiteral *UnsafeFormatStringCheck::getFormatLiteral( const CallExpr *Call, const std::vector<CheckedFunction> &CustomFunctions) { - const FunctionDecl *FD = cast<FunctionDecl>(Call->getDirectCallee()); + auto *FD = cast<FunctionDecl>(Call->getDirectCallee()); if (!FD) return nullptr; for (const auto &Entry : CustomFunctions) { >From 5626c34e00a841805ae74e84a0657e46369bef8e Mon Sep 17 00:00:00 2001 From: Daniel Krupp <[email protected]> Date: Wed, 3 Dec 2025 12:47:30 +0100 Subject: [PATCH 10/12] Fixups -document formatting fixes -const added -custom test extended to have 2 element config list --- .../clang-tidy/bugprone/UnsafeFormatStringCheck.cpp | 2 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-- .../checks/bugprone/unsafe-format-string.rst | 11 +++++++---- .../bugprone/unsafe-format-string-custom-config.c | 8 ++++++-- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp index ad4b9756a53e9..ff576548632f9 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp @@ -116,7 +116,7 @@ void UnsafeFormatStringCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { const StringLiteral *UnsafeFormatStringCheck::getFormatLiteral( const CallExpr *Call, const std::vector<CheckedFunction> &CustomFunctions) { - auto *FD = cast<FunctionDecl>(Call->getDirectCallee()); + const auto *FD = cast<FunctionDecl>(Call->getDirectCallee()); if (!FD) return nullptr; for (const auto &Entry : CustomFunctions) { diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 7543c07cdca5e..863b40867b4d5 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -205,8 +205,8 @@ New checks - New :doc:`bugprone-unsafe-format-string <clang-tidy/checks/bugprone/unsafe-format-string>` check. - Detects usage of vulnerable printf and scanf-like format - string functions with unbounded ``%s`` specifiers that can cause buffer overflows. + Detects usage of vulnerable ``printf`` and ``scanf``-like format string + functions with unbounded ``%s`` specifiers that can cause buffer overflows. - New :doc:`cppcoreguidelines-pro-bounds-avoid-unchecked-container-access <clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-access>` 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 c0b8c0c5f06e8..37cd3694a3b71 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 @@ -3,8 +3,8 @@ bugprone-unsafe-format-string ============================= -Detects usage of vulnerable format string functions with unbounded ``%s`` -specifiers that can cause buffer overflows. +Detects usage of vulnerable ``printf`` and ``scanf``-like 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. @@ -71,11 +71,14 @@ 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. +* `CustomPrintfFunctions` The user can specify own printf-like functions + with dangerous format string parameter. Default: ``""``. +* `CustomScanfFunctions` The user can specify own scanf-like functions + with dangerous format string parameter. Default: ``""``. Format: Both options have the following format. + .. code:: bugprone-unsafe-functions.CustomPrintfFunctions=" 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 index d817ac384fd37..c92e22a36b2ac 100644 --- 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 @@ -1,11 +1,12 @@ // 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: -config="{CheckOptions: {bugprone-unsafe-format-string.CustomPrintfFunctions: 'mysprintf, 1; mylogger, 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, ... ); +extern int mylogger( char* buffer, const char* format, ... ); void test_sprintf() { char buffer[100]; @@ -14,6 +15,9 @@ void test_sprintf() { /* 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] + + mylogger(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*/ @@ -30,4 +34,4 @@ void test_scanf() { /*Negative: safe %s with field width */ myscanf("%99s", buffer); /* no-warning */ -} \ No newline at end of file +} >From 3c1f63b6d1320ce55772132868e803014d1c307d Mon Sep 17 00:00:00 2001 From: Daniel Krupp <[email protected]> Date: Thu, 24 Sep 2026 17:54:08 +0200 Subject: [PATCH 11/12] Transform the docs to md. --- .../bugprone/UnsafeFormatStringCheck.cpp | 6 +- .../bugprone/UnsafeFormatStringCheck.h | 2 +- clang-tools-extra/docs/ReleaseNotes.md | 11 +- .../checks/bugprone/unsafe-format-string.md | 111 +++++++++++++++++ .../checks/bugprone/unsafe-format-string.rst | 114 ------------------ .../docs/clang-tidy/checks/list.md | 2 +- 6 files changed, 122 insertions(+), 124 deletions(-) create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.md delete mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp index ff576548632f9..c6208f430d7dc 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp @@ -48,7 +48,7 @@ parseCheckedFunctions(StringRef Option, ClangTidyContext *Context) { } Result.push_back( {Name.trim().str(), - matchers::MatchesAnyListedNameMatcher::NameMatcher(Name.trim()), + matchers::MatchesAnyListedRegexNameMatcher::NameMatcher(Name.trim()), Count}); } @@ -87,7 +87,7 @@ void UnsafeFormatStringCheck::registerMatchers(MatchFinder *Finder) { for (const auto &Entry : CustomPrintfFunctions) FunctionNames.emplace_back(Entry.Name); - auto CustomFunctionsMatcher = matchers::matchesAnyListedName(FunctionNames); + auto CustomFunctionsMatcher = matchers::matchesAnyListedRegexName(FunctionNames); Finder->addMatcher(callExpr(callee((functionDecl(CustomFunctionsMatcher)))) .bind(PrintfCallBind), @@ -101,7 +101,7 @@ void UnsafeFormatStringCheck::registerMatchers(MatchFinder *Finder) { for (const auto &Entry : CustomScanfFunctions) FunctionNames.emplace_back(Entry.Name); - auto CustomFunctionsMatcher = matchers::matchesAnyListedName(FunctionNames); + auto CustomFunctionsMatcher = matchers::matchesAnyListedRegexName(FunctionNames); Finder->addMatcher(callExpr(callee((functionDecl(CustomFunctionsMatcher)))) .bind(ScanfCallBind), diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h index 9fed8f067c871..0bca68bf0299f 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h @@ -28,7 +28,7 @@ class UnsafeFormatStringCheck : public ClangTidyCheck { struct CheckedFunction { std::string Name; - matchers::MatchesAnyListedNameMatcher::NameMatcher Pattern; + matchers::MatchesAnyListedRegexNameMatcher::NameMatcher Pattern; unsigned long FormatStringLocation; }; diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index cae6624268062..2aa8cd73ab5e9 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -129,6 +129,12 @@ infrastructure are described first, followed by tool-specific sections. #### New checks +- New {doc}`bugprone-unsafe-format-string + <clang-tidy/checks/bugprone/unsafe-format-string>` check. + + Detects usage of vulnerable `printf` and `scanf`-like format string + functions with unbounded `%s` specifiers that can cause buffer overflows. + - New {doc}`llvm-invalid-regex-pattern <clang-tidy/checks/llvm/invalid-regex-pattern>` check. @@ -158,11 +164,6 @@ infrastructure are described first, followed by tool-specific sections. Finds explicit zero initializers of arrays that can be replaced with empty braces. -- New {doc}`bugprone-unsafe-format-string - <clang-tidy/checks/bugprone/unsafe-format-string>` check. - - Detects usage of vulnerable `printf` and `scanf`-like format string - functions with unbounded `%s` specifiers that can cause buffer overflows. #### New check aliases diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.md b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.md new file mode 100644 index 0000000000000..edba685272dcd --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.md @@ -0,0 +1,111 @@ +```{title} clang-tidy - bugprone-unsafe-format-string +``` + +# bugprone-unsafe-format-string + +Detects usage of vulnerable `printf` and `scanf`-like 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 + +```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` + +## Options + +```{option} CustomPrintfFunctions +The user can specify own printf-like functions with dangerous format string +parameter. Default is `""`. +``` + +```{option} CustomScanfFunctions +The user can specify own scanf-like functions with dangerous format string +parameter. Default is `""`. +``` + +Both options have the following format: + +``` +bugprone-unsafe-format-string.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 a warning: + +```c +// 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 + +- 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/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst deleted file mode 100644 index 37cd3694a3b71..0000000000000 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.rst +++ /dev/null @@ -1,114 +0,0 @@ -.. title:: clang-tidy - bugprone-unsafe-format-string - -bugprone-unsafe-format-string -============================= - -Detects usage of vulnerable ``printf`` and ``scanf``-like 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`` - -Configuration -------------- - -The checker offers 2 configuration options. - -* `CustomPrintfFunctions` The user can specify own printf-like functions - with dangerous format string parameter. Default: ``""``. -* `CustomScanfFunctions` The user can specify own scanf-like functions - with dangerous format string parameter. Default: ``""``. - -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 ---------------- - -* 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/docs/clang-tidy/checks/list.md b/clang-tools-extra/docs/clang-tidy/checks/list.md index 0e256569ce357..a33a37c49ba76 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.md +++ b/clang-tools-extra/docs/clang-tidy/checks/list.md @@ -182,7 +182,7 @@ readability/* | {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>` | Yes | +| {doc}`bugprone-unsafe-format-string <bugprone/unsafe-format-string>` | | | {doc}`bugprone-unsafe-functions <bugprone/unsafe-functions>` | | | {doc}`bugprone-unsafe-to-allow-exceptions <bugprone/unsafe-to-allow-exceptions>` | | | {doc}`bugprone-unused-local-non-trivial-variable <bugprone/unused-local-non-trivial-variable>` | | >From 40f00bc267b736d8c572179cfd27dd63ac234fdc Mon Sep 17 00:00:00 2001 From: Daniel Krupp <[email protected]> Date: Fri, 25 Sep 2026 16:59:51 +0200 Subject: [PATCH 12/12] Fixup - Fix review comments - Add a test case for invalid configuration - Add new format string test cases - fix clang-format, clang-tidy issues --- .../bugprone/UnsafeFormatStringCheck.cpp | 78 ++++++++++--------- .../checks/bugprone/unsafe-format-string.md | 4 +- .../unsafe-format-string-invalid-config.c | 5 ++ .../checkers/bugprone/unsafe-format-string.c | 20 ++++- 4 files changed, 69 insertions(+), 38 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-invalid-config.c diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp index c6208f430d7dc..9acc110b1e253 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp @@ -10,21 +10,23 @@ #include "../utils/OptionsUtils.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "llvm/Support/ConvertUTF.h" +#include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/Regex.h" #include <string> using namespace clang::ast_matchers; namespace clang::tidy::bugprone { -static constexpr llvm::StringLiteral OptionNameCustomPrintfFunctions = +static constexpr llvm::StringRef OptionNameCustomPrintfFunctions = "CustomPrintfFunctions"; -static constexpr llvm::StringLiteral OptionNameCustomScanfFunctions = +static constexpr llvm::StringRef 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 constexpr llvm::StringRef BuiltInFormatBind = "format"; +static constexpr llvm::StringRef BuiltInCallBind = "call"; +static constexpr llvm::StringRef PrintfCallBind = "printfcall"; +static constexpr llvm::StringRef ScanfCallBind = "scanfcall"; static std::vector<UnsafeFormatStringCheck::CheckedFunction> parseCheckedFunctions(StringRef Option, ClangTidyContext *Context) { @@ -66,17 +68,26 @@ UnsafeFormatStringCheck::UnsafeFormatStringCheck(StringRef Name, 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"); + const auto VulnerableFunctionsArg0 = + hasAnyName("scanf", "vscanf", "wscanf", "vwscanf"); + const auto VulnerableFunctionsArg1 = + hasAnyName("sprintf", "vsprintf", "fscanf", "sscanf", "vscanf", "vfscanf", + "vsscanf", "fwscanf", "swscanf", "vfwscanf", "vswscanf"); Finder->addMatcher( callExpr( - callee(functionDecl(VulnerableFunctions, + callee(functionDecl(VulnerableFunctionsArg0, anyOf(isInStdNamespace(), hasDeclContext(translationUnitDecl())))), - anyOf(hasArgument(0, stringLiteral().bind(BuiltInFormatBind)), - hasArgument(1, stringLiteral().bind(BuiltInFormatBind)))) + hasArgument(0, stringLiteral().bind(BuiltInFormatBind))) + .bind(BuiltInCallBind), + this); + + Finder->addMatcher( + callExpr( + callee(functionDecl(VulnerableFunctionsArg1, + anyOf(isInStdNamespace(), + hasDeclContext(translationUnitDecl())))), + hasArgument(1, stringLiteral().bind(BuiltInFormatBind))) .bind(BuiltInCallBind), this); @@ -84,12 +95,13 @@ void UnsafeFormatStringCheck::registerMatchers(MatchFinder *Finder) { std::vector<llvm::StringRef> FunctionNames; FunctionNames.reserve(CustomPrintfFunctions.size()); - for (const auto &Entry : CustomPrintfFunctions) + for (const CheckedFunction &Entry : CustomPrintfFunctions) FunctionNames.emplace_back(Entry.Name); - auto CustomFunctionsMatcher = matchers::matchesAnyListedRegexName(FunctionNames); + auto CustomFunctionsMatcher = + matchers::matchesAnyListedRegexName(FunctionNames); - Finder->addMatcher(callExpr(callee((functionDecl(CustomFunctionsMatcher)))) + Finder->addMatcher(callExpr(callee(functionDecl(CustomFunctionsMatcher))) .bind(PrintfCallBind), this); } @@ -98,12 +110,13 @@ void UnsafeFormatStringCheck::registerMatchers(MatchFinder *Finder) { std::vector<llvm::StringRef> FunctionNames; FunctionNames.reserve(CustomScanfFunctions.size()); - for (const auto &Entry : CustomScanfFunctions) + for (const CheckedFunction &Entry : CustomScanfFunctions) FunctionNames.emplace_back(Entry.Name); - auto CustomFunctionsMatcher = matchers::matchesAnyListedRegexName(FunctionNames); + auto CustomFunctionsMatcher = + matchers::matchesAnyListedRegexName(FunctionNames); - Finder->addMatcher(callExpr(callee((functionDecl(CustomFunctionsMatcher)))) + Finder->addMatcher(callExpr(callee(functionDecl(CustomFunctionsMatcher))) .bind(ScanfCallBind), this); } @@ -116,10 +129,10 @@ void UnsafeFormatStringCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { const StringLiteral *UnsafeFormatStringCheck::getFormatLiteral( const CallExpr *Call, const std::vector<CheckedFunction> &CustomFunctions) { - const auto *FD = cast<FunctionDecl>(Call->getDirectCallee()); + const auto *FD = Call->getDirectCallee(); if (!FD) return nullptr; - for (const auto &Entry : CustomFunctions) { + for (const CheckedFunction &Entry : CustomFunctions) { if (Entry.Pattern.match(*FD)) { if (Entry.FormatStringLocation >= Call->getNumArgs()) return nullptr; @@ -132,28 +145,26 @@ const StringLiteral *UnsafeFormatStringCheck::getFormatLiteral( } void UnsafeFormatStringCheck::check(const MatchFinder::MatchResult &Result) { - const CallExpr *Call; - const StringLiteral *Format; + const CallExpr *Call = nullptr; + const StringLiteral *Format = nullptr; bool IsScanfFamily = false; - if (Result.Nodes.getNodeAs<CallExpr>(BuiltInCallBind)) { - Call = Result.Nodes.getNodeAs<CallExpr>(BuiltInCallBind); + if ((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); + } else if ((Call = Result.Nodes.getNodeAs<CallExpr>(PrintfCallBind))) { Format = UnsafeFormatStringCheck::getFormatLiteral(Call, CustomPrintfFunctions); IsScanfFamily = false; - } else if (Result.Nodes.getNodeAs<CallExpr>(ScanfCallBind)) { - Call = Result.Nodes.getNodeAs<CallExpr>(ScanfCallBind); + } else if ((Call = Result.Nodes.getNodeAs<CallExpr>(ScanfCallBind))) { Format = UnsafeFormatStringCheck::getFormatLiteral(Call, CustomScanfFunctions); IsScanfFamily = true; } else { Call = nullptr; Format = nullptr; + llvm_unreachable("No valid matched node in check()"); } if (!Call || !Format) @@ -170,8 +181,7 @@ void UnsafeFormatStringCheck::check(const MatchFinder::MatchResult &Result) { convertUTF32ToUTF8String(Format->getBytes(), FormatString); } - if (!UnsafeFormatStringCheck::hasUnboundedStringSpecifier(FormatString, - IsScanfFamily)) + if (!hasUnboundedStringSpecifier(FormatString, IsScanfFamily)) return; diag(Call->getBeginLoc(), @@ -244,14 +254,12 @@ bool UnsafeFormatStringCheck::hasUnboundedStringSpecifier(StringRef Fmt, if (SpecPos < N && Fmt[SpecPos] == 's') { if (IsScanfFamily) { // For scanf family, field width provides protection - if (!HasFieldWidth) { + if (!HasFieldWidth) return true; - } } else { // For sprintf family, only precision provides protection - if (!HasPrecision) { + if (!HasPrecision) return true; - } } } diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.md b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.md index edba685272dcd..0807557fc70a6 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.md +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-format-string.md @@ -66,12 +66,12 @@ The check detects unsafe format strings in these functions: ```{option} CustomPrintfFunctions The user can specify own printf-like functions with dangerous format string -parameter. Default is `""`. +parameter. Default is an empty string. ``` ```{option} CustomScanfFunctions The user can specify own scanf-like functions with dangerous format string -parameter. Default is `""`. +parameter. Default is an empty string. ``` Both options have the following format: diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-invalid-config.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-invalid-config.c new file mode 100644 index 0000000000000..3a535f5da01e9 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-invalid-config.c @@ -0,0 +1,5 @@ +// RUN: %check_clang_tidy %s bugprone-unsafe-format-string %t --\ +// RUN: -config="{CheckOptions: {bugprone-unsafe-format-string.CustomPrintfFunctions: 'mysprintf, wrong-value; mylogger, 1', bugprone-unsafe-format-string.CustomScanfFunctions: 'myscanf, 0;' }}"\ +// RUN: -- -isystem %S/Inputs/unsafe-format-string + +// CHECK-MESSAGES: warning: invalid configuration value for option \ 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 index 2e3077ecab35a..9627be789d734 100644 --- 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 @@ -78,14 +78,32 @@ void test_vsnprintf(int count, ...) { void test_scanf() { char buffer[100]; + char buffer2[100]; + int i; /* 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] + scanf("%99s %s", buffer, buffer2); + // 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] + + scanf("%*s %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] + + 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 */ + + /*Negative: safe %*s does not write into buffer */ + scanf("%*s %99s", buffer); + + scanf("%%%99s", buffer); + + scanf("%ds", &i); + } void test_fscanf() { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
