[clang-tools-extra] f27c8ac - [clang-tidy] Add the `bugprone-unsafe-functions` check
via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 2 05:12:16 PST 2023
Author: Gergely Fűtő
Date: 2023-02-02T14:11:42+01:00
New Revision: f27c8ac83e7cb945c8b3f9bf0092f8cf93278b5c
URL: https://github.com/llvm/llvm-project/commit/f27c8ac83e7cb945c8b3f9bf0092f8cf93278b5c
DIFF: https://github.com/llvm/llvm-project/commit/f27c8ac83e7cb945c8b3f9bf0092f8cf93278b5c.diff
LOG: [clang-tidy] Add the `bugprone-unsafe-functions` check
Checks for unsafe functions, mostly those listed in the
SEI CERT C Coding Standard Recommendation `MSC24-C` and Rule `MSC33-C`.
For the listed functions, an alternative, more secure replacement is
suggested, if such is available. The checker heavily relies on the
functions from "Annex K" (Bounds-checking interfaces) from C11, but
there are several other recommendations not directly from Annex K.
Differential Revision: http://reviews.llvm.org/D91000
Reviewed-By: aaron.ballman, dkrupp, steakhal, whisperity
Co-Authored-By: Tamás Koller <koller.tamas1996 at gmail.com>
Co-Authored-By: Balázs Benics <balazs.benics at sigmatechnology.se>
Co-Authored-By: Whisperity <whisperity at gmail.com>
Added:
clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst
clang-tools-extra/docs/clang-tidy/checks/cert/msc24-c.rst
clang-tools-extra/docs/clang-tidy/checks/cert/msc33-c.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
Modified:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index a11db603d5ff0..11b4051668cef 100644
--- 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 @@ class BugproneModule : public ClangTidyModule {
"bugprone-unhandled-self-assignment");
CheckFactories.registerCheck<UnhandledExceptionAtNewCheck>(
"bugprone-unhandled-exception-at-new");
+ CheckFactories.registerCheck<UnsafeFunctionsCheck>(
+ "bugprone-unsafe-functions");
CheckFactories.registerCheck<UnusedRaiiCheck>("bugprone-unused-raii");
CheckFactories.registerCheck<UnusedReturnValueCheck>(
"bugprone-unused-return-value");
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 643c62132d875..a975c86fc3970 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -67,6 +67,7 @@ add_clang_library(clangTidyBugproneModule
UndelegatedConstructorCheck.cpp
UnhandledExceptionAtNewCheck.cpp
UnhandledSelfAssignmentCheck.cpp
+ UnsafeFunctionsCheck.cpp
UnusedRaiiCheck.cpp
UnusedReturnValueCheck.cpp
UseAfterMoveCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
new file mode 100644
index 0000000000000..ebb49645b5904
--- /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 <cassert>
+
+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<std::string>
+getAnnexKReplacementFor(StringRef FunctionName) {
+ return StringSwitch<std::string>(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<StringRef>(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<StringRef>(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<StringRef>(FunctionName)
+ .Case("bcopy", "memcpy_s")
+ .Case("bzero", "memset_s")
+ .Default({});
+
+ if (!AnnexKReplacementFunction.empty())
+ return AnnexKReplacementFunction;
+ }
+
+ return StringSwitch<StringRef>(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<StringRef>(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<bool> &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<DeclRefExpr>(DeclRefId);
+ const auto *FuncDecl = cast<FunctionDecl>(DeclRef->getDecl());
+ assert(DeclRef && FuncDecl && "No valid matched node in check()");
+
+ const auto *AnnexK = Result.Nodes.getNodeAs<FunctionDecl>(
+ FunctionNamesWithAnnexKReplacementId);
+ const auto *Normal = Result.Nodes.getNodeAs<FunctionDecl>(FunctionNamesId);
+ const auto *Additional =
+ Result.Nodes.getNodeAs<FunctionDecl>(AdditionalFunctionNamesId);
+ assert((AnnexK || Normal || Additional) && "No valid match category.");
+
+ bool AnnexKIsAvailable =
+ isAnnexKAvailable(IsAnnexKAvailable, PP, getLangOpts());
+ StringRef FunctionName = FuncDecl->getName();
+ const std::optional<std::string> ReplacementFunctionName =
+ [&]() -> std::optional<std::string> {
+ 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/bugprone/UnsafeFunctionsCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
new file mode 100644
index 0000000000000..369ea25f693cc
--- /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 <optional>
+
+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<bool> 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/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index c3be9d66d090a..d448d9ba61454 100644
--- 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 @@ class CERTModule : public ClangTidyModule {
// FIO
CheckFactories.registerCheck<misc::NonCopyableObjectsCheck>("cert-fio38-c");
// MSC
+ CheckFactories.registerCheck<bugprone::UnsafeFunctionsCheck>(
+ "cert-msc24-c");
CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc30-c");
CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
"cert-msc32-c");
+ CheckFactories.registerCheck<bugprone::UnsafeFunctionsCheck>(
+ "cert-msc33-c");
// POS
CheckFactories.registerCheck<bugprone::BadSignalToKillThreadCheck>(
"cert-pos44-c");
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6fe457b61f620..8c8868f2f7b57 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -97,6 +97,14 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- New :doc:`bugprone-unsafe-functions
+ <clang-tidy/checks/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
<clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this>` check.
@@ -105,6 +113,14 @@ New checks
New check aliases
^^^^^^^^^^^^^^^^^
+- New alias :doc:`cert-msc24-c
+ <clang-tidy/checks/cert/msc24-c>` to :doc:`bugprone-unsafe-functions
+ <clang-tidy/checks/bugprone/unsafe-functions>` was added.
+
+- New alias :doc:`cert-msc33-c
+ <clang-tidy/checks/cert/msc33-c>` to :doc:`bugprone-unsafe-functions
+ <clang-tidy/checks/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
index 0000000000000..a0a267883b6fe
--- /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
+ <https://wiki.sei.cmu.edu/confluence/display/c/MSC24-C.+Do+not+use+deprecated+or+obsolescent+functions>`_.
+ - Rule `MSC33-C. Do not pass invalid data to the asctime() function
+ <https://wiki.sei.cmu.edu/confluence/display/c/MSC33-C.+Do+not+pass+invalid+data+to+the+asctime%28%29+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
+<unsafe-functions.html#cmdoption-arg-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 <string.h> // Defines functions from Annex K.
+ #include <stdio.h>
+
+ 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
index 0000000000000..f4b3f3f663921
--- /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
index 0000000000000..5c61556edab00
--- /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
index 90169dc72ef0a..b23fdc1dee56b 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -137,6 +137,7 @@ Clang-Tidy Checks
`bugprone-undelegated-constructor <bugprone/undelegated-constructor.html>`_,
`bugprone-unhandled-exception-at-new <bugprone/unhandled-exception-at-new.html>`_,
`bugprone-unhandled-self-assignment <bugprone/unhandled-self-assignment.html>`_,
+ `bugprone-unsafe-functions <bugprone/unsafe-functions.html>`_,
`bugprone-unused-raii <bugprone/unused-raii.html>`_, "Yes"
`bugprone-unused-return-value <bugprone/unused-return-value.html>`_,
`bugprone-use-after-move <bugprone/use-after-move.html>`_,
@@ -388,8 +389,10 @@ Clang-Tidy Checks
`cert-exp42-c <cert/exp42-c.html>`_, `bugprone-suspicious-memory-comparison <bugprone/suspicious-memory-comparison.html>`_,
`cert-fio38-c <cert/fio38-c.html>`_, `misc-non-copyable-objects <misc/non-copyable-objects.html>`_,
`cert-flp37-c <cert/flp37-c.html>`_, `bugprone-suspicious-memory-comparison <bugprone/suspicious-memory-comparison.html>`_,
+ `cert-msc24-c <cert/msc24-c.html>`_, `bugprone-unsafe-functions <bugprone/unsafe-functions.html>`_,
`cert-msc30-c <cert/msc30-c.html>`_, `cert-msc50-cpp <cert/msc50-cpp.html>`_,
`cert-msc32-c <cert/msc32-c.html>`_, `cert-msc51-cpp <cert/msc51-cpp.html>`_,
+ `cert-msc33-c <cert/msc33-c.html>`_, `bugprone-unsafe-functions <bugprone/unsafe-functions.html>`_,
`cert-msc54-cpp <cert/msc54-cpp.html>`_, `bugprone-signal-handler <bugprone/signal-handler.html>`_,
`cert-oop11-cpp <cert/oop11-cpp.html>`_, `performance-move-constructor-init <performance/move-constructor-init.html>`_,
`cert-oop54-cpp <cert/oop54-cpp.html>`_, `bugprone-unhandled-self-assignment <bugprone/unhandled-self-assignment.html>`_,
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
index 0000000000000..e1f8238fd4f3f
--- /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;
+}
More information about the cfe-commits
mailing list