diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 3aa39d10ceb5d..6704d951760da 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -110,6 +110,7 @@ #include "UnhandledSelfAssignmentCheck.h" #include "UnintendedCharOstreamOutputCheck.h" #include "UniquePtrArrayMismatchCheck.h" +#include "UnsafeFormatStringCheck.h" #include "UnsafeFunctionsCheck.h" #include "UnsafeToAllowExceptionsCheck.h" #include "UnusedLocalNonTrivialVariableCheck.h" @@ -321,6 +322,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-unique-ptr-array-mismatch"); CheckFactories.registerCheck( "bugprone-crtp-constructor-accessibility"); + CheckFactories.registerCheck( + "bugprone-unsafe-format-string"); CheckFactories.registerCheck( "bugprone-unsafe-functions"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 43e85b1407f21..f85e461d7a670 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -112,6 +112,7 @@ add_clang_library(clangTidyBugproneModule STATIC UnhandledExceptionAtNewCheck.cpp UnhandledSelfAssignmentCheck.cpp UniquePtrArrayMismatchCheck.cpp + UnsafeFormatStringCheck.cpp UnsafeFunctionsCheck.cpp UnsafeToAllowExceptionsCheck.cpp UnusedLocalNonTrivialVariableCheck.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..35863286b127f --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.cpp @@ -0,0 +1,273 @@ +//===----------------------------------------------------------------------===// +// +// 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 "../utils/OptionsUtils.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/ConvertUTF.h" +#include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/Regex.h" +#include + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +static constexpr StringRef OptionNameCustomPrintfFunctions = + "CustomPrintfFunctions"; +static constexpr StringRef OptionNameCustomScanfFunctions = + "CustomScanfFunctions"; + +static constexpr StringRef BuiltInFormatBind = "format"; +static constexpr StringRef BuiltInCallBind = "call"; +static constexpr StringRef PrintfCallBind = "printfcall"; +static constexpr StringRef ScanfCallBind = "scanfcall"; + +static std::vector +parseCheckedFunctions(StringRef OptionName, StringRef Option, + ClangTidyContext *Context) { + const std::vector Functions = + utils::options::parseStringList(Option); + std::vector 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 ', ;' pairs") + << OptionName; + continue; + } + Result.push_back( + {Name.trim().str(), + matchers::MatchesAnyListedRegexNameMatcher::NameMatcher(Name.trim()), + Count}); + } + + return Result; +} + +void UnsafeFormatStringCheck::registerFunctionMatchers( + MatchFinder *Finder, const std::vector &Funcs, + StringRef BindTo) { + std::vector FunctionNames; + FunctionNames.reserve(Funcs.size()); + for (const CheckedFunction &Entry : Funcs) + FunctionNames.emplace_back(Entry.Name); + const auto CustomFunctionsMatcher = + matchers::matchesAnyListedRegexName(FunctionNames); + Finder->addMatcher( + callExpr(callee(functionDecl(CustomFunctionsMatcher))).bind(BindTo), + this); +} + +UnsafeFormatStringCheck::UnsafeFormatStringCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + CustomPrintfFunctions(parseCheckedFunctions( + OptionNameCustomPrintfFunctions, + Options.get(OptionNameCustomPrintfFunctions, ""), Context)), + CustomScanfFunctions(parseCheckedFunctions( + OptionNameCustomScanfFunctions, + Options.get(OptionNameCustomScanfFunctions, ""), Context)) {} + +void UnsafeFormatStringCheck::registerMatchers(MatchFinder *Finder) { + // Matches sprintf and scanf family functions in std namespace in C++ and + // globally in C. + const auto VulnerableFunctionsArg0 = + hasAnyName("scanf", "vscanf", "wscanf", "vwscanf"); + const auto VulnerableFunctionsArg1 = + hasAnyName("sprintf", "vsprintf", "fscanf", "sscanf", "vfscanf", + "vsscanf", "fwscanf", "swscanf", "vfwscanf", "vswscanf"); + Finder->addMatcher( + callExpr( + callee(functionDecl(VulnerableFunctionsArg0, + anyOf(isInStdNamespace(), + hasDeclContext(translationUnitDecl())))), + 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); + + if (!CustomPrintfFunctions.empty()) + registerFunctionMatchers(Finder, CustomPrintfFunctions, PrintfCallBind); + if (!CustomScanfFunctions.empty()) + registerFunctionMatchers(Finder, CustomScanfFunctions, ScanfCallBind); +} + +std::string UnsafeFormatStringCheck::serializeConfig( + const std::vector &Functions) { + std::vector Result; + Result.reserve(Functions.size()); + for (const auto &Entry : Functions) + Result.push_back(Entry.Name + "," + + std::to_string(Entry.FormatStringLocation)); + return llvm::join(Result, ";"); +} + +void UnsafeFormatStringCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, OptionNameCustomPrintfFunctions, + serializeConfig(CustomPrintfFunctions)); + Options.store(Opts, OptionNameCustomScanfFunctions, + serializeConfig(CustomScanfFunctions)); +} + +const StringLiteral *UnsafeFormatStringCheck::getFormatLiteral( + const CallExpr *Call, const std::vector &CustomFunctions) { + const FunctionDecl *FD = Call->getDirectCallee(); + if (!FD) + return nullptr; + for (const CheckedFunction &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(Arg); + } + } + llvm_unreachable( + "The Call must match one of the functions in CustomFunctions."); +} + +static bool hasUnboundedStringSpecifier(StringRef Fmt, bool IsScanfFamily) { + size_t Pos = 0; + const 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] == '.') { + HasPrecision = true; + SpecPos++; + if (SpecPos < N && Fmt[SpecPos] == '*') { + // precision in argument + SpecPos++; + } else { + while (SpecPos < N && isdigit(Fmt[SpecPos])) + 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; +} + +void UnsafeFormatStringCheck::check(const MatchFinder::MatchResult &Result) { + const CallExpr *Call = nullptr; + const StringLiteral *Format = nullptr; + bool IsScanfFamily = false; + if ((Call = Result.Nodes.getNodeAs(BuiltInCallBind))) { + Format = Result.Nodes.getNodeAs(BuiltInFormatBind); + const auto *Callee = cast(Call->getCalleeDecl()); + IsScanfFamily = Callee->getName().contains("scanf"); + } else if ((Call = Result.Nodes.getNodeAs(PrintfCallBind))) { + Format = + UnsafeFormatStringCheck::getFormatLiteral(Call, CustomPrintfFunctions); + IsScanfFamily = false; + } else if ((Call = Result.Nodes.getNodeAs(ScanfCallBind))) { + Format = + UnsafeFormatStringCheck::getFormatLiteral(Call, CustomScanfFunctions); + IsScanfFamily = true; + } else { + llvm_unreachable("No valid matched node in check()"); + } + + // Format can be null with incorrect format string position configuration + if (!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); + } + + if (!hasUnboundedStringSpecifier(FormatString, IsScanfFamily)) + return; + diag(Call->getBeginLoc(), + "format specifier '%%s' without %select{precision|field width}0 may " + "cause buffer overflow; " + "consider using '%%%select{.|}0Ns' where N limits " + "%select{output|input}0 length") + << IsScanfFamily << Call->getSourceRange(); +} + +} // 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..7b7709bf4eec3 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFormatStringCheck.h @@ -0,0 +1,49 @@ +//===------------------------------------------------------------*- 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" +#include "../utils/Matchers.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: +/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/unsafe-format-string.html +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::MatchesAnyListedRegexNameMatcher::NameMatcher Pattern; + unsigned long FormatStringLocation; + }; + +private: + const std::vector CustomPrintfFunctions; + const std::vector CustomScanfFunctions; + void registerFunctionMatchers(ast_matchers::MatchFinder *Finder, + const std::vector &Funcs, + StringRef BindTo); + std::string serializeConfig(const std::vector &Functions); + static const StringLiteral * + getFormatLiteral(const CallExpr *, + const std::vector &CustomFunctions); +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFEFORMATSTRINGCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 8b32c22542aeb..9fa854eb76b32 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -138,6 +138,12 @@ infrastructure are described first, followed by tool-specific sections. #### New checks +- New {doc}`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 ` check. 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..88e8dd26718a1 --- /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. + +## 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 + +## 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 +List of printf-like functions with dangerous format string +parameter. Default is an empty string. +``` + +```{option} CustomScanfFunctions +List of scanf-like functions with dangerous format string +parameter. Default is an empty string. +``` + +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 (zero-based) index of the +format string literal argument. + +The following configuration will give a warning: + +```c +// bugprone-unsafe-format-string.CustomPrintfFunctions: 'mysprintf, 1;' +// bugprone-unsafe-format-string.CustomScanfFunctions: 'myscanf, 0;' + +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 +} +``` diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.md b/clang-tools-extra/docs/clang-tidy/checks/list.md index a74a26e691053..a33a37c49ba76 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.md +++ b/clang-tools-extra/docs/clang-tidy/checks/list.md @@ -182,6 +182,7 @@ readability/* | {doc}`bugprone-unhandled-self-assignment ` | | | {doc}`bugprone-unintended-char-ostream-output ` | Yes | | {doc}`bugprone-unique-ptr-array-mismatch ` | Yes | +| {doc}`bugprone-unsafe-format-string ` | | | {doc}`bugprone-unsafe-functions ` | | | {doc}`bugprone-unsafe-to-allow-exceptions ` | | | {doc}`bugprone-unused-local-non-trivial-variable ` | | diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/system-header-simulator.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/system-header-simulator.h new file mode 100644 index 0000000000000..bb256778bbb0b --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/std/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 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..75498341aea9c --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-custom-config.c @@ -0,0 +1,36 @@ +// RUN: %check_clang_tidy %s bugprone-unsafe-format-string %t --\ +// RUN: -config="{CheckOptions: {bugprone-unsafe-format-string.CustomPrintfFunctions: 'mysprintf, 1; mylogger, 1', bugprone-unsafe-format-string.CustomScanfFunctions: 'myscanf, 0;' }}"\ +// RUN: -- -I %S/../Inputs/Headers/std + +#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]; + const char* input = "user input"; + + /* 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); + +} + + +void test_scanf() { + char buffer[100]; + + /* 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] + + /*safe %s with field width */ + myscanf("%99s", buffer); +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-invalid-config-scanf.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-invalid-config-scanf.c new file mode 100644 index 0000000000000..7cfdf40f73d3e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-invalid-config-scanf.c @@ -0,0 +1,4 @@ +// RUN: %check_clang_tidy %s bugprone-unsafe-format-string %t --\ +// RUN: -config="{CheckOptions: {bugprone-unsafe-format-string.CustomScanfFunctions: 'myscanf, wrong;' }}" + +// CHECK-MESSAGES: warning: invalid configuration value for option 'CustomScanfFunctions' 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..df19722114892 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-invalid-config.c @@ -0,0 +1,4 @@ +// 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;' }}" + +// CHECK-MESSAGES: warning: invalid configuration value for option 'CustomPrintfFunctions' diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-wrong-custom-config.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-wrong-custom-config.c new file mode 100644 index 0000000000000..8794f07b87e16 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string-wrong-custom-config.c @@ -0,0 +1,34 @@ +// RUN: %check_clang_tidy %s bugprone-unsafe-format-string %t --\ +// RUN: -config="{CheckOptions: {bugprone-unsafe-format-string.CustomPrintfFunctions: 'mysprintf, 0; mylogger, 1', bugprone-unsafe-format-string.CustomScanfFunctions: 'myscanf, 0;' }}"\ +// RUN: -- -I %S/../Inputs/Headers/std + +#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]; + const char* input = "user input"; + + /* unsafe %s without field width */ + mysprintf(buffer, "%s", input); + // no warning as mysprintf config is wrong + + 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); +} + +void test_scanf() { + char buffer[100]; + + /* 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] + + /*safe %s with field width */ + myscanf("%99s", buffer); +} 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..996d0756cb82a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string.c @@ -0,0 +1,285 @@ +// RUN: %check_clang_tidy %s bugprone-unsafe-format-string %t -- -- -I %S/../Inputs/Headers/std + +#include "system-header-simulator.h" + +void test_sprintf() { + char buffer[100]; + const char* input = "user input"; + const wchar_t* winput = L"wide user input"; + + + /* 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] + + /* 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] + + /* 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] + + /*precision limits string length */ + sprintf(buffer, "%.99s", input); + + /*precision with field width */ + sprintf(buffer, "%1.99s", input); + + /*dynamic precision */ + sprintf(buffer, "%.*s", 99, input); + + /*field width with dynamic precision */ + sprintf(buffer, "%1.*s", 99, input); + + /*dynamic field width with fixed precision */ + sprintf(buffer, "%*.99s", 10, input); + + /*dynamic field width and precision */ + sprintf(buffer, "%*.*s", 10, 99, input); + + /*limit is taken as 0*/ + sprintf(buffer, "%.s", 99, input); + + /*limit is taken as 0*/ + sprintf(buffer, "%.ls", winput); + + /*other format specifiers are safe */ + sprintf(buffer, "%ds %fs", 42, 3.14); + + //Tests taken from warn-format-overflow-truncation.c + sprintf(buffer, "hell\0 boy"); + sprintf(buffer, "hello b\0y"); + sprintf(buffer, "hello"); + sprintf(buffer, "hello!"); + sprintf(buffer, "1234%%"); + sprintf(buffer, "12345%%"); + sprintf(buffer, "1234%c", '9'); + sprintf(buffer, "12345%c", '9'); + sprintf(buffer, "1234%d", 9); + sprintf(buffer, "12345%d", 9); + sprintf(buffer, "1234%lld", 9ll); + sprintf(buffer, "12345%lld", 9ll); + sprintf(buffer, "12%#x", 9); + sprintf(buffer, "123%#x", 9); + sprintf(buffer, "12%p", (void *)9); + sprintf(buffer, "123%p", (void *)9); + sprintf(buffer, "123%+d", 9); + sprintf(buffer, "1234%+d", 9); + sprintf(buffer, "123% i", 9); + sprintf(buffer, "1234% i", 9); + sprintf(buffer, "%5d", 9); + sprintf(buffer, "1%5d", 9); + sprintf(buffer, "%.3f", 9.f); + sprintf(buffer, "5%.3f", 9.f); + sprintf(buffer, "%+.2f", 9.f); + sprintf(buffer, "%+.3f", 9.f); + sprintf(buffer, "%.0e", 9.f); + sprintf(buffer, "5%.1e", 9.f); + sprintf(buffer, "%5.1f", 9.f); + sprintf(buffer, "%+5.1f", 9.f); + sprintf(buffer, "% 5.1f", 9.f); + sprintf(buffer, "% 5.1f", 9.f); + sprintf(buffer, "%+6.1f", 9.f); + sprintf(buffer, "% 6.1f", 9.f); + sprintf(buffer, "% 6.1f", 9.f); + +} + +void test_vsprintf() { + char buffer[100]; + va_list args; + + /* 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] + + /* 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] + + /* precision limits string length */ + vsprintf(buffer, "%.99s", args); + +} + +void test_vsnprintf(int count, ...) { + va_list args; + va_start(args, count); + char buffer[100]; + + /*vsnprintf is safe */ + vsnprintf(buffer, sizeof(buffer), "%99s", args); + + va_end(args); +} + +void test_scanf() { + char buffer[100]; + char buffer2[100]; + int i; + + /* 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] + + /*safe %s with field width */ + scanf("%99s", buffer); + + /*safe %*s does not write into buffer */ + scanf("%*s %99s", buffer); + + scanf("%%%99s", buffer); + + scanf("%ds", &i); + +} + +void test_fscanf() { + char buffer[100]; + FILE* file = 0; + + /* 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] + + /*safe %s with field width */ + fscanf(file, "%99s", buffer); +} + +void test_sscanf(char *source) { + char buffer[100]; + + /* 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] + + /*safe %s with field width */ + sscanf(source, "%99s", buffer); +} + +void test_vfscanf() { + FILE* file = 0; + va_list args; + + /* 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] + + /*safe %s with field width */ + vfscanf(file, "%99s", args); +} + +void test_vsscanf(char * source) { + va_list args; + + /* 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] + + /*safe %s with field width */ + vsscanf(source, "%99s", args); +} + +void test_vscanf() { + va_list args; + + /* 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] + + /*safe %s with field width */ + vscanf("%99s", args); +} + +void test_wscanf() { + wchar_t buffer[100]; + + /* 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] + + /*safe %s with field width */ + wscanf(L"%99s", buffer); +} + +void test_fwscanf() { + wchar_t buffer[100]; + FILE* file = 0; + + /* 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] + + /*safe %s with field width */ + fwscanf(file, L"%99s", buffer); +} + +void test_swscanf(wchar_t *source) { + wchar_t buffer[100]; + + /* 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] + + /*safe %s with field width */ + swscanf(source, L"%99s", buffer); +} + +void test_vwscanf() { + va_list args; + + /* 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] + + /*safe %s with field width */ + vwscanf(L"%99s", args); +} + +void test_vfwscanf() { + FILE* file = 0; + va_list args; + + /* 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] + + /*safe %s with field width */ + vfwscanf(file, L"%99s", args); +} + +void test_vswscanf() { + const wchar_t* source = L"input"; + va_list args; + + /* 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] + + /*safe %s with field width */ + vswscanf(source, L"%99s", args); +} + +void test_safe_alternatives() { + char buffer[100]; + const char* input = "user input"; + + /*snprintf is inherently safe */ + snprintf(buffer, sizeof(buffer), "%s", input); + + /*printf family doesn't write to buffers */ + printf("%s", input); + + /*fprintf doesn't write to user buffers */ + fprintf(stderr, "%s", input); +} 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..7dbc91218973d --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-format-string.cpp @@ -0,0 +1,52 @@ +// RUN: %check_clang_tidy %s bugprone-unsafe-format-string %t -- -- -I %S/../Inputs/Headers/std + +#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"; + /* no warning for calling member functions */ + sprintf(buffer, "%s", input); + } +}; + +void test_sprintf() { + char buffer[100]; + const char* input = "user input"; + + /* 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] + + /* 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] + + /* 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] + + /*precision limits string length */ + std::sprintf(buffer, "%.99s", input); + + /*precision with field width */ + std::sprintf(buffer, "%1.99s", input); + + /*dynamic precision */ + std::sprintf(buffer, "%.*s", 99, input); + + /*field width with dynamic precision */ + std::sprintf(buffer, "%1.*s", 99, input); + + /*dynamic field width with fixed precision */ + std::sprintf(buffer, "%*.99s", 10, input); + + /*dynamic field width and precision */ + std::sprintf(buffer, "%*.*s", 10, 99, input); + + + /*other format specifiers are safe */ + std::sprintf(buffer, "%d %f", 42, 3.14); +}