Index: clang-tidy/cert/CERTTidyModule.cpp =================================================================== --- clang-tidy/cert/CERTTidyModule.cpp +++ 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 @@ // DCL CheckFactories.registerCheck( "cert-dcl03-c"); + // ENV + CheckFactories.registerCheck( + "cert-env33-c"); // FLP CheckFactories.registerCheck( "cert-flp30-c"); Index: clang-tidy/cert/CMakeLists.txt =================================================================== --- clang-tidy/cert/CMakeLists.txt +++ clang-tidy/cert/CMakeLists.txt @@ -2,6 +2,7 @@ add_clang_library(clangTidyCERTModule CERTTidyModule.cpp + CommandProcessorCheck.cpp FloatLoopCounter.cpp SetLongJmpCheck.cpp StaticObjectExceptionCheck.cpp Index: clang-tidy/cert/CommandProcessorCheck.h =================================================================== --- clang-tidy/cert/CommandProcessorCheck.h +++ 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 Index: clang-tidy/cert/CommandProcessorCheck.cpp =================================================================== --- clang-tidy/cert/CommandProcessorCheck.cpp +++ clang-tidy/cert/CommandProcessorCheck.cpp @@ -0,0 +1,46 @@ +//===--- 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"))) + .bind("expr"), + this); +} + +void CommandProcessorCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Fn = Result.Nodes.getNodeAs("func"); + const auto *E = Result.Nodes.getNodeAs("expr"); + + // 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. + if (Fn->getName() == "system" && E->getNumArgs() == 1 && + E->getArg(0)->isNullPointerConstant(*Result.Context, + Expr::NPC_ValueDependentIsNotNull)) + return; + + diag(E->getExprLoc(), "calling %0 uses a command processor") << Fn; +} + +} // namespace cert +} // namespace tidy +} // namespace clang Index: docs/clang-tidy/checks/cert-env33-c.rst =================================================================== --- docs/clang-tidy/checks/cert-env33-c.rst +++ 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() +`_. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -8,6 +8,7 @@ cert-dcl50-cpp cert-dcl54-cpp (redirects to misc-new-delete-overloads) cert-dcl59-cpp (redirects to google-build-namespaces) + cert-env33-c cert-err52-cpp cert-err58-cpp cert-err60-cpp Index: test/clang-tidy/cert-env33-c.c =================================================================== --- test/clang-tidy/cert-env33-c.c +++ 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 +}