diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -71,6 +71,7 @@ #include "UndelegatedConstructorCheck.h" #include "UnhandledExceptionAtNewCheck.h" #include "UnhandledSelfAssignmentCheck.h" +#include "UnsafeFunctionsCheck.h" #include "UnusedRaiiCheck.h" #include "UnusedReturnValueCheck.h" #include "UseAfterMoveCheck.h" @@ -200,6 +201,8 @@ "bugprone-unhandled-self-assignment"); CheckFactories.registerCheck( "bugprone-unhandled-exception-at-new"); + CheckFactories.registerCheck( + "bugprone-unsafe-functions"); CheckFactories.registerCheck("bugprone-unused-raii"); CheckFactories.registerCheck( "bugprone-unused-return-value"); diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -67,6 +67,7 @@ UndelegatedConstructorCheck.cpp UnhandledExceptionAtNewCheck.cpp UnhandledSelfAssignmentCheck.cpp + UnsafeFunctionsCheck.cpp UnusedRaiiCheck.cpp UnusedReturnValueCheck.cpp UseAfterMoveCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h @@ -0,0 +1,52 @@ +//===--- UnsafeFunctionsCheck.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_UNSAFEFUNCTIONSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFEFUNCTIONSCHECK_H + +#include "../ClangTidyCheck.h" +#include + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Checks for functions that have safer, more secure replacements available, or +/// are considered deprecated due to design flaws. This check relies heavily on, +/// but is not exclusive to, the functions from the +/// Annex K. "Bounds-checking interfaces" of C11. +/// +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/unsafe-functions.html +class UnsafeFunctionsCheck : public ClangTidyCheck { +public: + UnsafeFunctionsCheck(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; + + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void onEndOfTranslationUnit() override; + +private: + /// If true, additional functions from widely used API-s (such as POSIX) are + /// added to the list of reported functions. + const bool ReportMoreUnsafeFunctions; + + Preprocessor *PP = nullptr; + /// Whether "Annex K" functions are available and should be + /// suggested in diagnostics. This is filled and cached internally. + std::optional IsAnnexKAvailable; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFEFUNCTIONSCHECK_H diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp @@ -0,0 +1,238 @@ +//===--- UnsafeFunctionsCheck.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 "UnsafeFunctionsCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" +#include + +using namespace clang::ast_matchers; +using namespace llvm; + +namespace clang { +namespace tidy { +namespace bugprone { + +static constexpr llvm::StringLiteral OptionNameReportMoreUnsafeFunctions = + "ReportMoreUnsafeFunctions"; + +static constexpr llvm::StringLiteral FunctionNamesWithAnnexKReplacementId = + "FunctionNamesWithAnnexKReplacement"; +static constexpr llvm::StringLiteral FunctionNamesId = "FunctionsNames"; +static constexpr llvm::StringLiteral AdditionalFunctionNamesId = + "AdditionalFunctionsNames"; +static constexpr llvm::StringLiteral DeclRefId = "DRE"; + +static std::optional +getAnnexKReplacementFor(StringRef FunctionName) { + return StringSwitch(FunctionName) + .Case("strlen", "strnlen_s") + .Case("wcslen", "wcsnlen_s") + .Default((Twine{FunctionName} + "_s").str()); +} + +static StringRef getReplacementFor(StringRef FunctionName, + bool IsAnnexKAvailable) { + if (IsAnnexKAvailable) { + // Try to find a better replacement from Annex K first. + StringRef AnnexKReplacementFunction = + StringSwitch(FunctionName) + .Cases("asctime", "asctime_r", "asctime_s") + .Case("gets", "gets_s") + .Default({}); + if (!AnnexKReplacementFunction.empty()) + return AnnexKReplacementFunction; + } + + // FIXME: Some of these functions are available in C++ under "std::", and + // should be matched and suggested. + return StringSwitch(FunctionName) + .Cases("asctime", "asctime_r", "strftime") + .Case("gets", "fgets") + .Case("rewind", "fseek") + .Case("setbuf", "setvbuf"); +} + +static StringRef getReplacementForAdditional(StringRef FunctionName, + bool IsAnnexKAvailable) { + if (IsAnnexKAvailable) { + // Try to find a better replacement from Annex K first. + StringRef AnnexKReplacementFunction = StringSwitch(FunctionName) + .Case("bcopy", "memcpy_s") + .Case("bzero", "memset_s") + .Default({}); + + if (!AnnexKReplacementFunction.empty()) + return AnnexKReplacementFunction; + } + + return StringSwitch(FunctionName) + .Case("bcmp", "memcmp") + .Case("bcopy", "memcpy") + .Case("bzero", "memset") + .Case("getpw", "getpwuid") + .Case("vfork", "posix_spawn"); +} + +/// \returns The rationale for replacing the function \p FunctionName with the +/// safer alternative. +static StringRef getRationaleFor(StringRef FunctionName) { + return StringSwitch(FunctionName) + .Cases("asctime", "asctime_r", "ctime", + "is not bounds-checking and non-reentrant") + .Cases("bcmp", "bcopy", "bzero", "is deprecated") + .Cases("fopen", "freopen", "has no exclusive access to the opened file") + .Case("gets", "is insecure, was deprecated and removed in C11 and C++14") + .Case("getpw", "is dangerous as it may overflow the provided buffer") + .Cases("rewind", "setbuf", "has no error detection") + .Case("vfork", "is insecure as it can lead to denial of service " + "situations in the parent process") + .Default("is not bounds-checking"); +} + +/// Calculates whether Annex K is available for the current translation unit +/// based on the macro definitions and the language options. +/// +/// The result is cached and saved in \p CacheVar. +static bool isAnnexKAvailable(std::optional &CacheVar, Preprocessor *PP, + const LangOptions &LO) { + if (CacheVar.has_value()) + return *CacheVar; + + if (!LO.C11) + // TODO: How is "Annex K" available in C++ mode? + return (CacheVar = false).value(); + + assert(PP && "No Preprocessor registered."); + + if (!PP->isMacroDefined("__STDC_LIB_EXT1__") || + !PP->isMacroDefined("__STDC_WANT_LIB_EXT1__")) + return (CacheVar = false).value(); + + const auto *MI = + PP->getMacroInfo(PP->getIdentifierInfo("__STDC_WANT_LIB_EXT1__")); + if (!MI || MI->tokens_empty()) + return (CacheVar = false).value(); + + const Token &T = MI->tokens().back(); + if (!T.isLiteral() || !T.getLiteralData()) + return (CacheVar = false).value(); + + CacheVar = StringRef(T.getLiteralData(), T.getLength()) == "1"; + return CacheVar.value(); +} + +UnsafeFunctionsCheck::UnsafeFunctionsCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + ReportMoreUnsafeFunctions( + Options.get(OptionNameReportMoreUnsafeFunctions, true)) {} + +void UnsafeFunctionsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, OptionNameReportMoreUnsafeFunctions, + ReportMoreUnsafeFunctions); +} + +void UnsafeFunctionsCheck::registerMatchers(MatchFinder *Finder) { + if (getLangOpts().C11) { + // Matching functions with safe replacements only in Annex K. + auto FunctionNamesWithAnnexKReplacementMatcher = hasAnyName( + "::bsearch", "::ctime", "::fopen", "::fprintf", "::freopen", "::fscanf", + "::fwprintf", "::fwscanf", "::getenv", "::gmtime", "::localtime", + "::mbsrtowcs", "::mbstowcs", "::memcpy", "::memmove", "::memset", + "::printf", "::qsort", "::scanf", "::snprintf", "::sprintf", "::sscanf", + "::strcat", "::strcpy", "::strerror", "::strlen", "::strncat", + "::strncpy", "::strtok", "::swprintf", "::swscanf", "::vfprintf", + "::vfscanf", "::vfwprintf", "::vfwscanf", "::vprintf", "::vscanf", + "::vsnprintf", "::vsprintf", "::vsscanf", "::vswprintf", "::vswscanf", + "::vwprintf", "::vwscanf", "::wcrtomb", "::wcscat", "::wcscpy", + "::wcslen", "::wcsncat", "::wcsncpy", "::wcsrtombs", "::wcstok", + "::wcstombs", "::wctomb", "::wmemcpy", "::wmemmove", "::wprintf", + "::wscanf"); + Finder->addMatcher( + declRefExpr(to(functionDecl(FunctionNamesWithAnnexKReplacementMatcher) + .bind(FunctionNamesWithAnnexKReplacementId))) + .bind(DeclRefId), + this); + } + + // Matching functions with replacements without Annex K. + auto FunctionNamesMatcher = + hasAnyName("::asctime", "asctime_r", "::gets", "::rewind", "::setbuf"); + Finder->addMatcher( + declRefExpr(to(functionDecl(FunctionNamesMatcher).bind(FunctionNamesId))) + .bind(DeclRefId), + this); + + if (ReportMoreUnsafeFunctions) { + // Matching functions with replacements without Annex K, at user request. + auto AdditionalFunctionNamesMatcher = + hasAnyName("::bcmp", "::bcopy", "::bzero", "::getpw", "::vfork"); + Finder->addMatcher( + declRefExpr(to(functionDecl(AdditionalFunctionNamesMatcher) + .bind(AdditionalFunctionNamesId))) + .bind(DeclRefId), + this); + } +} + +void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) { + const auto *DeclRef = Result.Nodes.getNodeAs(DeclRefId); + const auto *FuncDecl = cast(DeclRef->getDecl()); + assert(DeclRef && FuncDecl && "No valid matched node in check()"); + + const auto *AnnexK = Result.Nodes.getNodeAs( + FunctionNamesWithAnnexKReplacementId); + const auto *Normal = Result.Nodes.getNodeAs(FunctionNamesId); + const auto *Additional = + Result.Nodes.getNodeAs(AdditionalFunctionNamesId); + assert((AnnexK || Normal || Additional) && "No valid match category."); + + bool AnnexKIsAvailable = + isAnnexKAvailable(IsAnnexKAvailable, PP, getLangOpts()); + StringRef FunctionName = FuncDecl->getName(); + const std::optional ReplacementFunctionName = + [&]() -> std::optional { + if (AnnexK) { + if (AnnexKIsAvailable) + return getAnnexKReplacementFor(FunctionName); + return std::nullopt; + } + + if (Normal) + return getReplacementFor(FunctionName, AnnexKIsAvailable).str(); + + if (Additional) + return getReplacementForAdditional(FunctionName, AnnexKIsAvailable).str(); + + llvm_unreachable("Unhandled match category"); + }(); + if (!ReplacementFunctionName) + return; + + diag(DeclRef->getExprLoc(), "function %0 %1; '%2' should be used instead") + << FuncDecl << getRationaleFor(FunctionName) + << ReplacementFunctionName.value() << DeclRef->getSourceRange(); +} + +void UnsafeFunctionsCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, + Preprocessor * /*ModuleExpanderPP*/) { + this->PP = PP; +} + +void UnsafeFunctionsCheck::onEndOfTranslationUnit() { + this->PP = nullptr; + IsAnnexKAvailable.reset(); +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp --- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp @@ -16,6 +16,7 @@ #include "../bugprone/SpuriouslyWakeUpFunctionsCheck.h" #include "../bugprone/SuspiciousMemoryComparisonCheck.h" #include "../bugprone/UnhandledSelfAssignmentCheck.h" +#include "../bugprone/UnsafeFunctionsCheck.h" #include "../bugprone/UnusedReturnValueCheck.h" #include "../concurrency/ThreadCanceltypeAsynchronousCheck.h" #include "../google/UnnamedNamespaceInHeaderCheck.h" @@ -302,9 +303,13 @@ // FIO CheckFactories.registerCheck("cert-fio38-c"); // MSC + CheckFactories.registerCheck( + "cert-msc24-c"); CheckFactories.registerCheck("cert-msc30-c"); CheckFactories.registerCheck( "cert-msc32-c"); + CheckFactories.registerCheck( + "cert-msc33-c"); // POS CheckFactories.registerCheck( "cert-pos44-c"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -97,6 +97,14 @@ New checks ^^^^^^^^^^ +- New :doc:`bugprone-unsafe-functions + ` check. + + Checks for functions that have safer, more secure replacements available, or + are considered deprecated due to design flaws. + This check relies heavily on, but is not exclusive to, the functions from + the *Annex K. "Bounds-checking interfaces"* of C11. + - New :doc:`cppcoreguidelines-avoid-capture-default-when-capturing-this ` check. @@ -105,6 +113,14 @@ New check aliases ^^^^^^^^^^^^^^^^^ +- New alias :doc:`cert-msc24-c + ` to :doc:`bugprone-unsafe-functions + ` was added. + +- New alias :doc:`cert-msc33-c + ` to :doc:`bugprone-unsafe-functions + ` was added. + Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst @@ -0,0 +1,144 @@ +.. title:: clang-tidy - bugprone-unsafe-functions + +bugprone-unsafe-functions +========================= + +Checks for functions that have safer, more secure replacements available, or +are considered deprecated due to design flaws. +The check heavily relies on the functions from the +**Annex K.** "Bounds-checking interfaces" of C11. + +The check implements the following rules from the CERT C Coding Standard: + - Recommendation `MSC24-C. Do not use deprecated or obsolescent functions + `_. + - Rule `MSC33-C. Do not pass invalid data to the asctime() function + `_. + +`cert-msc24-c` and `cert-msc33-c` redirect here as aliases of this check. + +Unsafe functions +---------------- + +If *Annex K.* is available, a replacement from *Annex K.* is suggested for the +following functions: + +``asctime``, ``asctime_r``, ``bsearch``, ``ctime``, ``fopen``, ``fprintf``, +``freopen``, ``fscanf``, ``fwprintf``, ``fwscanf``, ``getenv``, ``gets``, +``gmtime``, ``localtime``, ``mbsrtowcs``, ``mbstowcs``, ``memcpy``, +``memmove``, ``memset``, ``printf``, ``qsort``, ``scanf``, ``snprintf``, +``sprintf``, ``sscanf``, ``strcat``, ``strcpy``, ``strerror``, ``strlen``, +``strncat``, ``strncpy``, ``strtok``, ``swprintf``, ``swscanf``, ``vfprintf``, +``vfscanf``, ``vfwprintf``, ``vfwscanf``, ``vprintf``, ``vscanf``, +``vsnprintf``, ``vsprintf``, ``vsscanf``, ``vswprintf``, ``vswscanf``, +``vwprintf``, ``vwscanf``, ``wcrtomb``, ``wcscat``, ``wcscpy``, +``wcslen``, ``wcsncat``, ``wcsncpy``, ``wcsrtombs``, ``wcstok``, ``wcstombs``, +``wctomb``, ``wmemcpy``, ``wmemmove``, ``wprintf``, ``wscanf``. + +If *Annex K.* is not available, replacements are suggested only for the +following functions from the previous list: + + - ``asctime``, ``asctime_r``, suggested replacement: ``strftime`` + - ``gets``, suggested replacement: ``fgets`` + +The following functions are always checked, regardless of *Annex K* availability: + + - ``rewind``, suggested replacement: ``fseek`` + - ``setbuf``, suggested replacement: ``setvbuf`` + +If `ReportMoreUnsafeFunctions +`_ is enabled, +the following functions are also checked: + + - ``bcmp``, suggested replacement: ``memcmp`` + - ``bcopy``, suggested replacement: ``memcpy_s`` if *Annex K* is available, + or ``memcpy`` + - ``bzero``, suggested replacement: ``memset_s`` if *Annex K* is available, + or ``memset`` + - ``getpw``, suggested replacement: ``getpwuid`` + - ``vfork``, suggested replacement: ``posix_spawn`` + +Although mentioned in the associated CERT rules, the following functions are +**ignored** by the check: + +``atof``, ``atoi``, ``atol``, ``atoll``, ``tmpfile``. + +The availability of *Annex K* is determined based on the following macros: + + - ``__STDC_LIB_EXT1__``: feature macro, which indicates the presence of + *Annex K. "Bounds-checking interfaces"* in the library implementation + - ``__STDC_WANT_LIB_EXT1__``: user-defined macro, which indicates that the + user requests the functions from *Annex K.* to be defined. + +Both macros have to be defined to suggest replacement functions from *Annex K.* +``__STDC_LIB_EXT1__`` is defined by the library implementation, and +``__STDC_WANT_LIB_EXT1__`` must be defined to ``1`` by the user **before** +including any system headers. + + +Options +------- + +.. option:: ReportMoreUnsafeFunctions + + When `true`, additional functions from widely used APIs (such as POSIX) are + added to the list of reported functions. + See the main documentation of the check for the complete list as to what + this option enables. + Default is `true`. + + +Examples +-------- + +.. code-block:: c++ + + #ifndef __STDC_LIB_EXT1__ + #error "Annex K is not supported by the current standard library implementation." + #endif + + #define __STDC_WANT_LIB_EXT1__ 1 + + #include // Defines functions from Annex K. + #include + + enum { BUFSIZE = 32 }; + + void Unsafe(const char *Msg) { + static const char Prefix[] = "Error: "; + static const char Suffix[] = "\n"; + char Buf[BUFSIZE] = {0}; + + strcpy(Buf, Prefix); // warning: function 'strcpy' is not bounds-checking; 'strcpy_s' should be used instead. + strcat(Buf, Msg); // warning: function 'strcat' is not bounds-checking; 'strcat_s' should be used instead. + strcat(Buf, Suffix); // warning: function 'strcat' is not bounds-checking; 'strcat_s' should be used instead. + if (fputs(buf, stderr) < 0) { + // error handling + return; + } + } + + void UsingSafeFunctions(const char *Msg) { + static const char Prefix[] = "Error: "; + static const char Suffix[] = "\n"; + char Buf[BUFSIZE] = {0}; + + if (strcpy_s(Buf, BUFSIZE, Prefix) != 0) { + // error handling + return; + } + + if (strcat_s(Buf, BUFSIZE, Msg) != 0) { + // error handling + return; + } + + if (strcat_s(Buf, BUFSIZE, Suffix) != 0) { + // error handling + return; + } + + if (fputs(Buf, stderr) < 0) { + // error handling + return; + } + } diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert/msc24-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert/msc24-c.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cert/msc24-c.rst @@ -0,0 +1,10 @@ +.. title:: clang-tidy - cert-msc24-c +.. meta:: + :http-equiv=refresh: 5;URL=../bugprone/unsafe-functions.html + +cert-msc24-c +============ + +The cert-msc24-c check is an alias, please see +`bugprone-unsafe-functions <../bugprone/unsafe-functions.html>`_ for more +information. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert/msc33-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert/msc33-c.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cert/msc33-c.rst @@ -0,0 +1,10 @@ +.. title:: clang-tidy - cert-msc33-c +.. meta:: + :http-equiv=refresh: 5;URL=../bugprone/unsafe-functions.html + +cert-msc33-c +============ + +The cert-msc33-c check is an alias, please see +`bugprone-unsafe-functions <../bugprone/unsafe-functions.html>`_ for more +information. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -137,6 +137,7 @@ `bugprone-undelegated-constructor `_, `bugprone-unhandled-exception-at-new `_, `bugprone-unhandled-self-assignment `_, + `bugprone-unsafe-functions `_, `bugprone-unused-raii `_, "Yes" `bugprone-unused-return-value `_, `bugprone-use-after-move `_, @@ -388,8 +389,10 @@ `cert-exp42-c `_, `bugprone-suspicious-memory-comparison `_, `cert-fio38-c `_, `misc-non-copyable-objects `_, `cert-flp37-c `_, `bugprone-suspicious-memory-comparison `_, + `cert-msc24-c `_, `bugprone-unsafe-functions `_, `cert-msc30-c `_, `cert-msc50-cpp `_, `cert-msc32-c `_, `cert-msc51-cpp `_, + `cert-msc33-c `_, `bugprone-unsafe-functions `_, `cert-msc54-cpp `_, `bugprone-signal-handler `_, `cert-oop11-cpp `_, `performance-move-constructor-init `_, `cert-oop54-cpp `_, `bugprone-unhandled-self-assignment `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c @@ -0,0 +1,154 @@ +// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K %s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 +// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__ -U__STDC_WANT_LIB_EXT1__ +// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -D__STDC_LIB_EXT1__=1 -U__STDC_WANT_LIB_EXT1__ +// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t -- -- -U__STDC_LIB_EXT1__ -D__STDC_WANT_LIB_EXT1__=1 +// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K-CERT-ONLY %s bugprone-unsafe-functions %t -- \ +// RUN: -config="{CheckOptions: [{key: bugprone-unsafe-functions.ReportMoreUnsafeFunctions, value: false}]}" \ +// RUN: -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 + +typedef __SIZE_TYPE__ size_t; +typedef __WCHAR_TYPE__ wchar_t; + +char *gets(char *S); +size_t strlen(const char *S); +size_t wcslen(const wchar_t *S); + +void f1(char *S) { + gets(S); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'gets' is insecure, was deprecated and removed in C11 and C++14; 'gets_s' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'gets' is insecure, was deprecated and removed in C11 and C++14; 'gets_s' should be used instea + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-3]]:3: warning: function 'gets' is insecure, was deprecated and removed in C11 and C++14; 'fgets' should be used instead + + strlen(S); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'strlen' is not bounds-checking; 'strnlen_s' should be used instead + // no-warning WITHOUT-ANNEX-K +} + +void f1w(wchar_t *S) { + wcslen(S); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'wcslen' is not bounds-checking; 'wcsnlen_s' should be used instead + // no-warning WITHOUT-ANNEX-K +} + +struct tm; +char *asctime(const struct tm *TimePtr); + +void f2(const struct tm *Time) { + asctime(Time); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-3]]:3: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead + + char *(*F1)(const struct tm *) = asctime; + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:36: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:36: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-3]]:36: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead + + char *(*F2)(const struct tm *) = &asctime; + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:37: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:37: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-3]]:37: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead +} + +typedef void *FILE; +FILE *fopen(const char *Filename, const char *Mode); +FILE *freopen(const char *Filename, const char *Mode, FILE *Stream); +int fscanf(FILE *Stream, const char *Format, ...); +void rewind(FILE *Stream); +void setbuf(FILE *Stream, char *Buf); + +void f3(char *S, FILE *F) { + fopen(S, S); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'fopen' has no exclusive access to the opened file; 'fopen_s' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'fopen' has no exclusive access to the opened file; 'fopen_s' should be used instead + // no-warning WITHOUT-ANNEX-K + + freopen(S, S, F); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'freopen' has no exclusive access to the opened file; 'freopen_s' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'freopen' has no exclusive access to the opened file; 'freopen_s' should be used instead + // no-warning WITHOUT-ANNEX-K + + int I; + fscanf(F, "%d", &I); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'fscanf' is not bounds-checking; 'fscanf_s' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'fscanf' is not bounds-checking; 'fscanf_s' should be used instead + // no-warning WITHOUT-ANNEX-K + + rewind(F); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'rewind' has no error detection; 'fseek' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'rewind' has no error detection; 'fseek' should be used instead + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-3]]:3: warning: function 'rewind' has no error detection; 'fseek' should be used instead + + setbuf(F, S); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'setbuf' has no error detection; 'setvbuf' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'setbuf' has no error detection; 'setvbuf' should be used instead + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-3]]:3: warning: function 'setbuf' has no error detection; 'setvbuf' should be used instead +} + +typedef int time_t; +char *ctime(const time_t *Timer); + +void f4(const time_t *Timer) { + ctime(Timer); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'ctime' is not bounds-checking and non-reentrant; 'ctime_s' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'ctime' is not bounds-checking and non-reentrant; 'ctime_s' should be used instead + // no-warning WITHOUT-ANNEX-K +} + +#define BUFSIZ 128 +typedef int uid_t; +typedef int pid_t; +int bcmp(const void *S1, const void *S2, size_t N); +void bcopy(const void *Src, void *Dest, size_t N); +void bzero(void *S, size_t N); +int getpw(uid_t UId, char *Buf); +pid_t vfork(void); + +void fOptional() { + char Buf1[BUFSIZ] = {0}; + char Buf2[BUFSIZ] = {0}; + + bcmp(Buf1, Buf2, BUFSIZ); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'bcmp' is deprecated; 'memcmp' should be used instead + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'bcmp' is deprecated; 'memcmp' should be used instead + // no-warning CERT-ONLY + + bcopy(Buf1, Buf2, BUFSIZ); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'bcopy' is deprecated; 'memcpy_s' should be used instead + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'bcopy' is deprecated; 'memcpy' should be used instead + // no-warning CERT-ONLY + + bzero(Buf1, BUFSIZ); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'bzero' is deprecated; 'memset_s' should be used instead + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'bzero' is deprecated; 'memset' should be used instead + // no-warning CERT-ONLY + + getpw(0, Buf1); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'getpw' is dangerous as it may overflow the provided buffer; 'getpwuid' should be used instead + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'getpw' is dangerous as it may overflow the provided buffer; 'getpwuid' should be used instead + // no-warning CERT-ONLY + + vfork(); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'vfork' is insecure as it can lead to denial of service situations in the parent process; 'posix_spawn' should be used instead + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'vfork' is insecure as it can lead to denial of service situations in the parent process; 'posix_spawn' should be used instead + // no-warning CERT-ONLY +} + +typedef int errno_t; +typedef size_t rsize_t; +errno_t asctime_s(char *S, rsize_t Maxsize, const struct tm *TimePtr); +errno_t strcat_s(char *S1, rsize_t S1Max, const char *S2); + +void fUsingSafeFunctions(const struct tm *Time, FILE *F) { + char Buf[BUFSIZ] = {0}; + + // no-warning, safe function from annex K is used + if (asctime_s(Buf, BUFSIZ, Time) != 0) + return; + + // no-warning, safe function from annex K is used + if (strcat_s(Buf, BUFSIZ, "something") != 0) + return; +}