futogergely updated this revision to Diff 440216.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91000/new/
https://reviews.llvm.org/D91000
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c
@@ -0,0 +1,153 @@
+// 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 char 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, and it is removed from C11; 'gets_s' should be used instead
+ // CHECK-MESSAGES-WITH-ANNEX-K-CERT-ONLY: :[[@LINE-2]]:3: warning: function 'gets' is insecure, and it is removed from C11; 'gets_s' should be used instead
+ // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-3]]:3: warning: function 'gets' is insecure, and it is removed from C11; '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
+
+ 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 *timeptr) {
+ asctime(timeptr);
+ // 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 *(*f_ptr1)(const struct tm *) = asctime;
+ // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:40: 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]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+ // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-3]]:40: warning: function 'asctime' is not bounds-checking and non-reentrant; 'strftime' should be used instead
+
+ char *(*f_ptr2)(const struct tm *) = &asctime;
+ // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:41: 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]]:41: warning: function 'asctime' is not bounds-checking and non-reentrant; 'asctime_s' should be used instead
+ // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-3]]:41: 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 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 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 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 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
+}
+
+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() {
+ const size_t BUFFSIZE = 32;
+ char buf1[BUFFSIZE] = {0};
+ char buf2[BUFFSIZE] = {0};
+
+ bcmp(buf1, buf2, BUFFSIZE);
+ // 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, BUFFSIZE);
+ // 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, BUFFSIZE);
+ // 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 *timeptr, FILE *f) {
+ const size_t BUFFSIZE = 32;
+ char buf[BUFFSIZE] = {0};
+
+ // no-warning, safe function from annex K is used
+ if (asctime_s(buf, BUFFSIZE, timeptr) != 0)
+ return;
+
+ // no-warning, safe function from annex K is used
+ if (strcat_s(buf, BUFFSIZE, "something") != 0)
+ return;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -134,6 +134,7 @@
`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>`_,
@@ -378,8 +379,10 @@
`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-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>`_,
`cert-pos44-c <cert/pos44-c.html>`_, `bugprone-bad-signal-to-kill-thread <bugprone/bad-signal-to-kill-thread.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst
@@ -0,0 +1,114 @@
+.. title:: clang-tidy - bugprone-unsafe-functions
+
+bugprone-unsafe-functions
+=========================
+
+Checks for functions that have safer, more secure replacements available.
+The functions checked are considered unsafe because for example they are
+deprecated or missing bounds checking. For the listed functions a
+replacement function is suggested. The check heavily relies on the
+functions from Annex K (Bounds-checking interfaces) of C11.
+
+The checker implements the following rules from the CERT C Coding Standard:
+ - Recommendation MSC24-C (`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 (`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>`_).
+
+If Annex K is available, a replacement from Annex K is suggested for the following functions:
+`asctime`, `asctime_r`, `bsearch`, `ctime`, `fopen`, `freopen`, `fprintf`, `fscanf`, `fwprintf`, `fwscanf`,
+`gets`, `getenv`, `gmtime`, `localtime`, `mbsrtowcs`, `mbstowcs`, `memcpy`, `memmove`, `memset`,
+`printf`, `qsort`, `scanf`, `snprintf`, `sprintf`, `sscanf`, `strcat`, `strcpy`, `strerror`,
+`strlen`, `strncat`, `strncpy`, `strtok`, `swprintf`, `swscanf`, `tmpfile`, `tmpnam`, `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 also checked (regardless of Annex K availability):
+ - `rewind`, suggested replacement: `fseek`
+ - `setbuf`, suggested replacement: `setvbuf`
+
+Based on the ReportMoreUnsafeFunctions option, the following functions are also checked:
+ - `bcmp`, suggested replacement: `memcmp`
+ - `bcopy`, suggested replacement: `memcpy_s` if Annex K is available, or `memcopy` if Annex K is not available
+ - `bzero`, suggested replacement: `memset_s` if Annex K is available. or `memset`, if Annex K is not available
+ - `getpw`, suggested replacement: `getpwuid`
+ - `vfork`, suggested replacement: `posix_spawn`
+
+The following functions are **ignored** by the checker: `atof`, `atoi`, `atol`, `atoll`,
+`tmpfile`.
+
+The availability of Annex K is checked 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 if the user wants 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`, the following functions are added to the checker: `bcmp`, `bcopy`, `bzero`, `getpw`, `vfork`.
+ Default is `true`.
+
+
+Examples:
+
+.. code-block:: c++
+
+ // __STDC_LIB_EXT1__ is defined by the library implementation
+ #define __STDC_WANT_LIB_EXT1__ 1
+
+ #include <string.h> // defines the functions from Annex K
+ #include <stdio.h>
+
+ enum { BUFSIZE = 32 };
+
+ void fWarning(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 fUsingSafeFunctions(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;
+ }
+ }
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -145,6 +145,14 @@
Future libc++ will remove the extension (`D120996
<https://reviews.llvm.org/D120996>`).
+- New :doc:`bugprone-unsafe-functions <clang-tidy/checks/bugprone/unsafe-functions>` check.
+
+ Checks for functions that have safer, more secure replacements available.
+ The functions checked are considered unsafe because for example they are
+ deprecated or missing bounds checking. For the listed functions a
+ replacement function is suggested. The check heavily relies on the
+ functions from Annex K (Bounds-checking interfaces) of C11.
+
New check aliases
^^^^^^^^^^^^^^^^^
@@ -152,6 +160,14 @@
<clang-tidy/checks/cppcoreguidelines/macro-to-enum>` to :doc:`modernize-macro-to-enum
<clang-tidy/checks/modernize/macro-to-enum>` was added.
+- 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
^^^^^^^^^^^^^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ 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"
@@ -301,9 +302,13 @@
// 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");
Index: clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
@@ -0,0 +1,54 @@
+//===--- 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"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Checks for functions that have safer, more secure replacements available.
+/// The functions checked are considered unsafe because for example they are
+/// deprecated or missing bounds checking. For the listed functions a
+/// replacement function is suggested. The checker heavily relies on the
+/// functions from 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;
+
+private:
+ bool useSafeFunctionsFromAnnexK() const;
+
+ Optional<std::string> getAnnexKReplacementFor(StringRef FunctionName) const;
+ StringRef getReplacementFor(StringRef FunctionName) const;
+ StringRef getReplacementForOptional(StringRef FunctionName) const;
+
+ Preprocessor *PP = nullptr;
+
+ // Used for caching the result of useSafeFunctionsFromAnnexK.
+ mutable Optional<bool> IsAnnexKAvailable;
+
+ const bool ReportMoreUnsafeFunctions;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFEFUNCTIONSCHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
===================================================================
--- /dev/null
+++ 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 =
+ "FunctionNamesWithAnnexKReplacementExpr";
+static constexpr llvm::StringLiteral FunctionNamesId = "FunctionsNamesExpr";
+static constexpr llvm::StringLiteral OptionalFunctionNamesId =
+ "OptionalFunctionsNamesExpr";
+
+static constexpr llvm::StringLiteral DeclRefId = "DeclRefId";
+
+static StringRef getRationaleFor(StringRef FunctionName) {
+ return StringSwitch<StringRef>(FunctionName)
+ .Cases("asctime", "asctime_r", "ctime",
+ "is not bounds-checking and non-reentrant")
+ .Cases("fopen", "freopen", "has no exclusive access to file")
+ .Case("gets", "is insecure, and it is removed from C11")
+ .Cases("rewind", "setbuf", "has no error detection")
+ .Default("is not bounds-checking");
+}
+
+static StringRef getRationaleForOptional(StringRef FunctionName) {
+ return StringSwitch<StringRef>(FunctionName)
+ .Cases("bcmp", "bcopy", "bzero", "is deprecated")
+ .Case("getpw", "is dangerous as it may overflow the provided buffer")
+ .Case("vfork", "is insecure as it can lead to denial of service "
+ "situations in the parent process");
+}
+
+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.
+ auto OptionalFunctionNamesMatcher =
+ hasAnyName("::bcmp", "::bcopy", "::bzero", "::getpw", "::vfork");
+ Finder->addMatcher(declRefExpr(to(functionDecl(OptionalFunctionNamesMatcher)
+ .bind(OptionalFunctionNamesId)))
+ .bind(DeclRefId),
+ this);
+ }
+}
+
+void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>(DeclRefId);
+ assert(DeclRef && "No matching declaration reference found.");
+
+ SourceRange Range = DeclRef->getSourceRange();
+ // FIXME: I'm not sure if this can ever happen, but to be on the safe side,
+ // validity is checked.
+ if (Range.isInvalid())
+ return;
+
+ const auto *AnnexK = Result.Nodes.getNodeAs<FunctionDecl>(
+ FunctionNamesWithAnnexKReplacementId);
+ const auto *Normal = Result.Nodes.getNodeAs<FunctionDecl>(FunctionNamesId);
+ const auto *OptionalFunctions =
+ Result.Nodes.getNodeAs<FunctionDecl>(OptionalFunctionNamesId);
+
+ const auto *FuncDecl = cast<FunctionDecl>(DeclRef->getDecl());
+ StringRef FunctionName = FuncDecl->getName();
+
+ const Optional<std::string> ReplacementFunctionName =
+ [&]() -> Optional<std::string> {
+ if (AnnexK)
+ return getAnnexKReplacementFor(FunctionName);
+
+ if (Normal)
+ return getReplacementFor(FunctionName).str();
+
+ if (OptionalFunctions)
+ return getReplacementForOptional(FunctionName).str();
+
+ llvm_unreachable("Unhandled match");
+ }();
+
+ if (!ReplacementFunctionName)
+ return;
+
+ diag(Range.getBegin(), "function %0 %1; '%2' should be used instead")
+ << FuncDecl
+ << (OptionalFunctions ? getRationaleForOptional(FunctionName)
+ : getRationaleFor(FunctionName))
+ << ReplacementFunctionName.getValue() << Range;
+}
+
+void UnsafeFunctionsCheck::registerPPCallbacks(const SourceManager &SM,
+ Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) {
+ this->PP = PP;
+}
+
+bool UnsafeFunctionsCheck::useSafeFunctionsFromAnnexK() const {
+ if (IsAnnexKAvailable)
+ return IsAnnexKAvailable.getValue();
+
+ // Check for Annex K's availability only once,
+ // and cache the result.
+
+ if (!getLangOpts().C11)
+ return (IsAnnexKAvailable = false).getValue();
+
+ assert(PP && "No Preprocessor registered.");
+
+ if (!PP->isMacroDefined("__STDC_LIB_EXT1__") ||
+ !PP->isMacroDefined("__STDC_WANT_LIB_EXT1__"))
+ return (IsAnnexKAvailable = false).getValue();
+
+ const auto *MI =
+ PP->getMacroInfo(PP->getIdentifierInfo("__STDC_WANT_LIB_EXT1__"));
+ if (!MI || MI->tokens_empty())
+ return (IsAnnexKAvailable = false).getValue();
+
+ const Token &T = MI->tokens().back();
+ if (!T.isLiteral() || !T.getLiteralData())
+ return (IsAnnexKAvailable = false).getValue();
+
+ IsAnnexKAvailable = StringRef(T.getLiteralData(), T.getLength()) == "1";
+ return IsAnnexKAvailable.getValue();
+}
+
+Optional<std::string>
+UnsafeFunctionsCheck::getAnnexKReplacementFor(StringRef FunctionName) const {
+ if (!useSafeFunctionsFromAnnexK())
+ return None;
+
+ return StringSwitch<std::string>(FunctionName)
+ .Case("strlen", "strnlen_s")
+ .Case("wcslen", "wcsnlen_s")
+ .Default((Twine{FunctionName} + "_s").str());
+}
+
+StringRef
+UnsafeFunctionsCheck::getReplacementFor(StringRef FunctionName) const {
+ // Try to find a replacement from Annex K first.
+ StringRef AnnexKReplacementFunction =
+ useSafeFunctionsFromAnnexK()
+ ? StringSwitch<StringRef>(FunctionName)
+ .Cases("asctime", "asctime_r", "asctime_s")
+ .Case("gets", "gets_s")
+ .Default({})
+ : StringRef{};
+
+ if (!AnnexKReplacementFunction.empty())
+ return AnnexKReplacementFunction;
+
+ return StringSwitch<StringRef>(FunctionName)
+ .Cases("asctime", "asctime_r", "strftime")
+ .Case("gets", "fgets")
+ .Case("rewind", "fseek")
+ .Case("setbuf", "setvbuf");
+}
+
+StringRef
+UnsafeFunctionsCheck::getReplacementForOptional(StringRef FunctionName) const {
+ // Try to find a replacement from Annex K first.
+ StringRef AnnexKReplacementFunction =
+ useSafeFunctionsFromAnnexK() ? StringSwitch<StringRef>(FunctionName)
+ .Case("bcopy", "memcpy_s")
+ .Case("bzero", "memset_s")
+ .Default({})
+ : StringRef{};
+
+ 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");
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -64,6 +64,7 @@
UndelegatedConstructorCheck.cpp
UnhandledExceptionAtNewCheck.cpp
UnhandledSelfAssignmentCheck.cpp
+ UnsafeFunctionsCheck.cpp
UnusedRaiiCheck.cpp
UnusedReturnValueCheck.cpp
UseAfterMoveCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -68,6 +68,7 @@
#include "UndelegatedConstructorCheck.h"
#include "UnhandledExceptionAtNewCheck.h"
#include "UnhandledSelfAssignmentCheck.h"
+#include "UnsafeFunctionsCheck.h"
#include "UnusedRaiiCheck.h"
#include "UnusedReturnValueCheck.h"
#include "UseAfterMoveCheck.h"
@@ -196,6 +197,8 @@
"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>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
