[clang-tools-extra] r261530 - 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

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 22 08:01:08 PST 2016


Author: aaronballman
Date: Mon Feb 22 10:01:06 2016
New Revision: 261530

URL: http://llvm.org/viewvc/llvm-project?rev=261530&view=rev
Log:
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

Added:
    clang-tools-extra/trunk/clang-tidy/cert/CommandProcessorCheck.cpp
    clang-tools-extra/trunk/clang-tidy/cert/CommandProcessorCheck.h
    clang-tools-extra/trunk/docs/clang-tidy/checks/cert-env33-c.rst
    clang-tools-extra/trunk/test/clang-tidy/cert-env33-c.c
Modified:
    clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
    clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
    clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp?rev=261530&r1=261529&r2=261530&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp Mon Feb 22 10:01:06 2016
@@ -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");

Modified: clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt?rev=261530&r1=261529&r2=261530&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt Mon Feb 22 10:01:06 2016
@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyCERTModule
   CERTTidyModule.cpp
+  CommandProcessorCheck.cpp
   FloatLoopCounter.cpp
   SetLongJmpCheck.cpp
   StaticObjectExceptionCheck.cpp

Added: clang-tools-extra/trunk/clang-tidy/cert/CommandProcessorCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/CommandProcessorCheck.cpp?rev=261530&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cert/CommandProcessorCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/cert/CommandProcessorCheck.cpp Mon Feb 22 10:01:06 2016
@@ -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

Added: clang-tools-extra/trunk/clang-tidy/cert/CommandProcessorCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/CommandProcessorCheck.h?rev=261530&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/cert/CommandProcessorCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/cert/CommandProcessorCheck.h Mon Feb 22 10:01:06 2016
@@ -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

Added: clang-tools-extra/trunk/docs/clang-tidy/checks/cert-env33-c.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cert-env33-c.rst?rev=261530&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/cert-env33-c.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/cert-env33-c.rst Mon Feb 22 10:01:06 2016
@@ -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>`_.

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=261530&r1=261529&r2=261530&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Mon Feb 22 10:01:06 2016
@@ -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

Added: clang-tools-extra/trunk/test/clang-tidy/cert-env33-c.c
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cert-env33-c.c?rev=261530&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/cert-env33-c.c (added)
+++ clang-tools-extra/trunk/test/clang-tidy/cert-env33-c.c Mon Feb 22 10:01:06 2016
@@ -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
+}




More information about the cfe-commits mailing list