diff options
author | Aaron Ballman <aaron@aaronballman.com> | 2016-02-22 16:01:06 +0000 |
---|---|---|
committer | Aaron Ballman <aaron@aaronballman.com> | 2016-02-22 16:01:06 +0000 |
commit | 527a4205505155503de1fb1daecea472ba95358b (patch) | |
tree | 188fda052a14c7f40a22c4ae9629cfba4d9a2958 | |
parent | d665a66b0f094f3f85fd30e9d7888481e3035ca0 (diff) | |
download | bcm5719-llvm-527a4205505155503de1fb1daecea472ba95358b.tar.gz bcm5719-llvm-527a4205505155503de1fb1daecea472ba95358b.zip |
Add a new check, cert-env33-c, that diagnoses uses of system(), popen(), and _popen() to execute a command processor. This check corresponds to the CERT secure coding rule: https://www.securecoding.cert.org/confluence/pages/viewpage.action?pageId=2130132
llvm-svn: 261530
7 files changed, 122 insertions, 0 deletions
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp index 05a631b75c7..4fbcc198956 100644 --- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp @@ -16,6 +16,7 @@ #include "../misc/NonCopyableObjects.h" #include "../misc/StaticAssertCheck.h" #include "../misc/ThrowByValueCatchByReferenceCheck.h" +#include "CommandProcessorCheck.h" #include "FloatLoopCounter.h" #include "SetLongJmpCheck.h" #include "StaticObjectExceptionCheck.h" @@ -54,6 +55,9 @@ public: // DCL CheckFactories.registerCheck<StaticAssertCheck>( "cert-dcl03-c"); + // ENV + CheckFactories.registerCheck<CommandProcessorCheck>( + "cert-env33-c"); // FLP CheckFactories.registerCheck<FloatLoopCounter>( "cert-flp30-c"); diff --git a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt index 8c097aae8df..6273159dbb5 100644 --- a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt @@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyCERTModule CERTTidyModule.cpp + CommandProcessorCheck.cpp FloatLoopCounter.cpp SetLongJmpCheck.cpp StaticObjectExceptionCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cert/CommandProcessorCheck.cpp b/clang-tools-extra/clang-tidy/cert/CommandProcessorCheck.cpp new file mode 100644 index 00000000000..e2dbeca20c1 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cert/CommandProcessorCheck.cpp @@ -0,0 +1,45 @@ +//===--- Env33CCheck.cpp - clang-tidy--------------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "CommandProcessorCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cert { + +void CommandProcessorCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + callExpr( + callee(functionDecl(anyOf(hasName("::system"), hasName("::popen"), + hasName("::_popen"))) + .bind("func")), + // Do not diagnose when the call expression passes a null pointer + // constant to system(); that only checks for the presence of a + // command processor, which is not a security risk by itself. + unless(callExpr(callee(functionDecl(hasName("::system"))), + argumentCountIs(1), + hasArgument(0, nullPointerConstant())))) + .bind("expr"), + this); +} + +void CommandProcessorCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Fn = Result.Nodes.getNodeAs<FunctionDecl>("func"); + const auto *E = Result.Nodes.getNodeAs<CallExpr>("expr"); + + diag(E->getExprLoc(), "calling %0 uses a command processor") << Fn; +} + +} // namespace cert +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/cert/CommandProcessorCheck.h b/clang-tools-extra/clang-tidy/cert/CommandProcessorCheck.h new file mode 100644 index 00000000000..a85a7edae5e --- /dev/null +++ b/clang-tools-extra/clang-tidy/cert/CommandProcessorCheck.h @@ -0,0 +1,38 @@ +//===--- CommandInterpreterCheck.h - clang-tidy------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_COMMAND_PROCESSOR_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_COMMAND_PROCESSOR_CHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace cert { + +/// Execution of a command processor can lead to security vulnerabilities, +/// and is generally not required. Instead, prefer to launch executables +/// directly via mechanisms that give you more control over what executable is +/// actually launched. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cert-env33-c.html +class CommandProcessorCheck : public ClangTidyCheck { +public: + CommandProcessorCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace cert +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_COMMAND_PROCESSOR_CHECK_H diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert-env33-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-env33-c.rst new file mode 100644 index 00000000000..c5321b07f7f --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cert-env33-c.rst @@ -0,0 +1,13 @@ +.. title:: clang-tidy - cert-env33-c + +cert-env33-c +============ + +This check flags calls to ``system()``, ``popen()``, and ``_popen()``, which +execute a command processor. It does not flag calls to ``system()`` with a null +pointer argument, as such a call checks for the presence of a command processor +but does not actually attempt to execute a command. + +This check corresponds to the CERT C Coding Standard rule +`ENV33-C. Do not call system() +<https://www.securecoding.cert.org/confluence/pages/viewpage.action?pageId=2130132>`_. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index b19a7eb7558..b0ae6d8797c 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -8,6 +8,7 @@ Clang-Tidy Checks cert-dcl50-cpp cert-dcl54-cpp (redirects to misc-new-delete-overloads) <cert-dcl54-cpp> cert-dcl59-cpp (redirects to google-build-namespaces) <cert-dcl59-cpp> + cert-env33-c cert-err52-cpp cert-err58-cpp cert-err60-cpp diff --git a/clang-tools-extra/test/clang-tidy/cert-env33-c.c b/clang-tools-extra/test/clang-tidy/cert-env33-c.c new file mode 100644 index 00000000000..5846b496242 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/cert-env33-c.c @@ -0,0 +1,20 @@ +// RUN: %check_clang_tidy %s cert-env33-c %t + +typedef struct FILE {} FILE; + +extern int system(const char *); +extern FILE *popen(const char *, const char *); +extern FILE *_popen(const char *, const char *); + +void f(void) { + // It is permissible to check for the presence of a command processor. + system(0); + + system("test"); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'system' uses a command processor [cert-env33-c] + + popen("test", "test"); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'popen' uses a command processor + _popen("test", "test"); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling '_popen' uses a command processor +} |