r252285 - [analyzer] Add VforkChecker to find unsafe code in vforked process.

Yury Gribov via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 6 03:16:32 PST 2015


Author: ygribov
Date: Fri Nov  6 05:16:31 2015
New Revision: 252285

URL: http://llvm.org/viewvc/llvm-project?rev=252285&view=rev
Log:
[analyzer] Add VforkChecker to find unsafe code in vforked process.

This checker looks for unsafe constructs in vforked process:
function calls (excluding whitelist), memory write and returns.
This was originally motivated by a vfork-related bug in xtables package.

Patch by Yury Gribov.

Differential revision: http://reviews.llvm.org/D14014

Added:
    cfe/trunk/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
    cfe/trunk/test/Analysis/vfork.c
Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
    cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
    cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
    cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
    cfe/trunk/test/Analysis/Inputs/system-header-simulator.h

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h?rev=252285&r1=252284&r2=252285&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h Fri Nov  6 05:16:31 2015
@@ -15,9 +15,13 @@
 #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
 
 #include "clang/AST/Stmt.h"
+#include <tuple>
 
 namespace clang {
 
+class Expr;
+class VarDecl;
+
 namespace ento {
 
 bool containsMacro(const Stmt *S);
@@ -35,6 +39,9 @@ template <class T> bool containsStmt(con
   return false;
 }
 
+std::pair<const clang::VarDecl *, const clang::Expr *>
+parseAssignment(const Stmt *S);
+
 } // end GR namespace
 
 } // end clang namespace

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=252285&r1=252284&r2=252285&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Fri Nov  6 05:16:31 2015
@@ -76,6 +76,7 @@ add_clang_library(clangStaticAnalyzerChe
   UndefinedAssignmentChecker.cpp
   UnixAPIChecker.cpp
   UnreachableCodeChecker.cpp
+  VforkChecker.cpp
   VLASizeChecker.cpp
   VirtualCallChecker.cpp
 

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td?rev=252285&r1=252284&r2=252285&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td Fri Nov  6 05:16:31 2015
@@ -362,6 +362,10 @@ def MismatchedDeallocatorChecker : Check
   HelpText<"Check for mismatched deallocators.">,
   DescFile<"MallocChecker.cpp">;
 
+def VforkChecker : Checker<"Vfork">,
+  HelpText<"Check for proper usage of vfork">,
+  DescFile<"VforkChecker.cpp">;
+
 } // end "unix"
 
 let ParentPackage = UnixAlpha in {

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp?rev=252285&r1=252284&r2=252285&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp Fri Nov  6 05:16:31 2015
@@ -19,6 +19,7 @@
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -111,15 +112,11 @@ void DereferenceChecker::reportBug(Progr
     S = expr->IgnoreParenLValueCasts();
 
   if (IsBind) {
-    if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(S)) {
-      if (BO->isAssignmentOp())
-        S = BO->getRHS();
-    } else if (const DeclStmt *DS = dyn_cast<DeclStmt>(S)) {
-      assert(DS->isSingleDecl() && "We process decls one by one");
-      if (const VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl()))
-        if (const Expr *Init = VD->getAnyInitializer())
-          S = Init;
-    }
+    const VarDecl *VD;
+    const Expr *Init;
+    std::tie(VD, Init) = parseAssignment(S);
+    if (VD && Init)
+      S = Init;
   }
 
   switch (S->getStmtClass()) {

Added: cfe/trunk/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/VforkChecker.cpp?rev=252285&view=auto
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/VforkChecker.cpp (added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/VforkChecker.cpp Fri Nov  6 05:16:31 2015
@@ -0,0 +1,218 @@
+//===- VforkChecker.cpp -------- Vfork usage checks --------------*- C++ -*-==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines vfork checker which checks for dangerous uses of vfork.
+//  Vforked process shares memory (including stack) with parent so it's
+//  range of actions is significantly limited: can't write variables,
+//  can't call functions not in whitelist, etc. For more details, see
+//  http://man7.org/linux/man-pages/man2/vfork.2.html
+//
+//  This checker checks for prohibited constructs in vforked process.
+//  The state transition diagram:
+//  PARENT ---(vfork() == 0)--> CHILD
+//                                   |
+//                                   --(*p = ...)--> bug
+//                                   |
+//                                   --foo()--> bug
+//                                   |
+//                                   --return--> bug
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/AST/ParentMap.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class VforkChecker : public Checker<check::PreCall, check::PostCall,
+                                    check::Bind, check::PreStmt<ReturnStmt>> {
+  mutable std::unique_ptr<BuiltinBug> BT;
+  mutable llvm::SmallSet<const IdentifierInfo *, 10> VforkWhitelist;
+  mutable const IdentifierInfo *II_vfork;
+
+  static bool isChildProcess(const ProgramStateRef State);
+
+  bool isVforkCall(const Decl *D, CheckerContext &C) const;
+  bool isCallWhitelisted(const IdentifierInfo *II, CheckerContext &C) const;
+
+  void reportBug(const char *What, CheckerContext &C,
+                 const char *Details = 0) const;
+
+public:
+  VforkChecker() : II_vfork(0) {}
+
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+  void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const;
+  void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
+};
+
+} // end anonymous namespace
+
+// This trait holds region of variable that is assigned with vfork's
+// return value (this is the only region child is allowed to write).
+// VFORK_RESULT_INVALID means that we are in parent process.
+// VFORK_RESULT_NONE means that vfork's return value hasn't been assigned.
+// Other values point to valid regions.
+REGISTER_TRAIT_WITH_PROGRAMSTATE(VforkResultRegion, const void *)
+#define VFORK_RESULT_INVALID 0
+#define VFORK_RESULT_NONE ((void *)(uintptr_t)1)
+
+bool VforkChecker::isChildProcess(const ProgramStateRef State) {
+  return State->get<VforkResultRegion>() != VFORK_RESULT_INVALID;
+}
+
+bool VforkChecker::isVforkCall(const Decl *D, CheckerContext &C) const {
+  auto FD = dyn_cast_or_null<FunctionDecl>(D);
+  if (!FD || !C.isCLibraryFunction(FD))
+    return false;
+
+  if (!II_vfork) {
+    ASTContext &AC = C.getASTContext();
+    II_vfork = &AC.Idents.get("vfork");
+  }
+
+  return FD->getIdentifier() == II_vfork;
+}
+
+// Returns true iff ok to call function after successful vfork.
+bool VforkChecker::isCallWhitelisted(const IdentifierInfo *II,
+                                 CheckerContext &C) const {
+  if (VforkWhitelist.empty()) {
+    // According to manpage.
+    const char *ids[] = {
+      "_exit",
+      "_Exit",
+      "execl",
+      "execlp",
+      "execle",
+      "execv",
+      "execvp",
+      "execvpe",
+      0,
+    };
+
+    ASTContext &AC = C.getASTContext();
+    for (const char **id = ids; *id; ++id)
+      VforkWhitelist.insert(&AC.Idents.get(*id));
+  }
+
+  return VforkWhitelist.count(II);
+}
+
+void VforkChecker::reportBug(const char *What, CheckerContext &C,
+                             const char *Details) const {
+  if (ExplodedNode *N = C.generateErrorNode(C.getState())) {
+    if (!BT)
+      BT.reset(new BuiltinBug(this,
+                              "Dangerous construct in a vforked process"));
+
+    SmallString<256> buf;
+    llvm::raw_svector_ostream os(buf);
+
+    os << What << " is prohibited after a successful vfork";
+
+    if (Details)
+      os << "; " << Details;
+
+    auto Report = llvm::make_unique<BugReport>(*BT, os.str(), N);
+    // TODO: mark vfork call in BugReportVisitor
+    C.emitReport(std::move(Report));
+  }
+}
+
+// Detect calls to vfork and split execution appropriately.
+void VforkChecker::checkPostCall(const CallEvent &Call,
+                                 CheckerContext &C) const {
+  // We can't call vfork in child so don't bother
+  // (corresponding warning has already been emitted in checkPreCall).
+  ProgramStateRef State = C.getState();
+  if (isChildProcess(State))
+    return;
+
+  if (!isVforkCall(Call.getDecl(), C))
+    return;
+
+  // Get return value of vfork.
+  SVal VforkRetVal = Call.getReturnValue();
+  Optional<DefinedOrUnknownSVal> DVal =
+    VforkRetVal.getAs<DefinedOrUnknownSVal>();
+  if (!DVal)
+    return;
+
+  // Get assigned variable.
+  const ParentMap &PM = C.getLocationContext()->getParentMap();
+  const Stmt *P = PM.getParentIgnoreParenCasts(Call.getOriginExpr());
+  const VarDecl *LhsDecl;
+  std::tie(LhsDecl, std::ignore) = parseAssignment(P);
+
+  // Get assigned memory region.
+  MemRegionManager &M = C.getStoreManager().getRegionManager();
+  const MemRegion *LhsDeclReg =
+    LhsDecl
+      ? M.getVarRegion(LhsDecl, C.getLocationContext())
+      : (const MemRegion *)VFORK_RESULT_NONE;
+
+  // Parent branch gets nonzero return value (according to manpage).
+  ProgramStateRef ParentState, ChildState;
+  std::tie(ParentState, ChildState) = C.getState()->assume(*DVal);
+  C.addTransition(ParentState);
+  ChildState = ChildState->set<VforkResultRegion>(LhsDeclReg);
+  C.addTransition(ChildState);
+}
+
+// Prohibit calls to non-whitelist functions in child process.
+void VforkChecker::checkPreCall(const CallEvent &Call,
+                                CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  if (isChildProcess(State)
+      && !isCallWhitelisted(Call.getCalleeIdentifier(), C))
+    reportBug("This function call", C);
+}
+
+// Prohibit writes in child process (except for vfork's lhs).
+void VforkChecker::checkBind(SVal L, SVal V, const Stmt *S,
+                             CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  if (!isChildProcess(State))
+    return;
+
+  const MemRegion *VforkLhs =
+    static_cast<const MemRegion *>(State->get<VforkResultRegion>());
+  const MemRegion *MR = L.getAsRegion();
+
+  // Child is allowed to modify only vfork's lhs.
+  if (!MR || MR == VforkLhs)
+    return;
+
+  reportBug("This assignment", C);
+}
+
+// Prohibit return from function in child process.
+void VforkChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+  if (isChildProcess(State))
+    reportBug("Return", C, "call _exit() instead");
+}
+
+void ento::registerVforkChecker(CheckerManager &mgr) {
+  mgr.registerChecker<VforkChecker>();
+}

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp?rev=252285&r1=252284&r2=252285&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp Fri Nov  6 05:16:31 2015
@@ -12,6 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
 
 // Recursively find any substatements containing macros
@@ -70,3 +71,26 @@ bool clang::ento::containsBuiltinOffsetO
 
   return false;
 }
+
+// Extract lhs and rhs from assignment statement
+std::pair<const clang::VarDecl *, const clang::Expr *>
+clang::ento::parseAssignment(const Stmt *S) {
+  const VarDecl *VD = 0;
+  const Expr *RHS = 0;
+
+  if (auto Assign = dyn_cast_or_null<BinaryOperator>(S)) {
+    if (Assign->isAssignmentOp()) {
+      // Ordinary assignment
+      RHS = Assign->getRHS();
+      if (auto DE = dyn_cast_or_null<DeclRefExpr>(Assign->getLHS()))
+        VD = dyn_cast_or_null<VarDecl>(DE->getDecl());
+    }
+  } else if (auto PD = dyn_cast_or_null<DeclStmt>(S)) {
+    // Initialization
+    assert(PD->isSingleDecl() && "We process decls one by one");
+    VD = dyn_cast_or_null<VarDecl>(PD->getSingleDecl());
+    RHS = VD->getAnyInitializer();
+  }
+
+  return std::make_pair(VD, RHS);
+}

Modified: cfe/trunk/test/Analysis/Inputs/system-header-simulator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator.h?rev=252285&r1=252284&r2=252285&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator.h (original)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator.h Fri Nov  6 05:16:31 2015
@@ -92,3 +92,13 @@ typedef struct __SomeStruct {
   char * p;
 } SomeStruct;
 void fakeSystemHeaderCall(SomeStruct *);
+
+typedef int pid_t;
+pid_t fork(void);
+pid_t vfork(void);
+int execl(const char *path, const char *arg, ...);
+
+void exit(int status) __attribute__ ((__noreturn__));
+void _exit(int status) __attribute__ ((__noreturn__));
+void _Exit(int status) __attribute__ ((__noreturn__));
+

Added: cfe/trunk/test/Analysis/vfork.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/vfork.c?rev=252285&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/vfork.c (added)
+++ cfe/trunk/test/Analysis/vfork.c Fri Nov  6 05:16:31 2015
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,security.insecureAPI.vfork,unix.Vfork -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,security.insecureAPI.vfork,unix.Vfork -verify -x c++ %s
+
+#include "Inputs/system-header-simulator.h"
+
+void foo();
+
+// Ensure that child process is properly checked.
+int f1(int x) {
+  pid_t pid = vfork(); // expected-warning{{Call to function 'vfork' is insecure}}
+  if (pid != 0)
+    return 0;
+
+  switch (x) {
+  case 0:
+    // Ensure that modifying pid is ok.
+    pid = 1; // no-warning
+    // Ensure that calling whitelisted routines is ok.
+    execl("", "", 0); // no-warning
+    _exit(1); // no-warning
+    break;
+  case 1:
+    // Ensure that writing variables is prohibited.
+    x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+    break;
+  case 2:
+    // Ensure that calling functions is prohibited.
+    foo(); // expected-warning{{This function call is prohibited after a successful vfork}}
+    break;
+  default:
+    // Ensure that returning from function is prohibited.
+    return 0; // expected-warning{{Return is prohibited after a successful vfork; call _exit() instead}}
+  }
+
+  while(1);
+}
+
+// Same as previous but without explicit pid variable.
+int f2(int x) {
+  pid_t pid = vfork(); // expected-warning{{Call to function 'vfork' is insecure}}
+
+  switch (x) {
+  case 0:
+    // Ensure that writing pid is ok.
+    pid = 1; // no-warning
+    // Ensure that calling whitelisted routines is ok.
+    execl("", "", 0); // no-warning
+    _exit(1); // no-warning
+    break;
+  case 1:
+    // Ensure that writing variables is prohibited.
+    x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+    break;
+  case 2:
+    // Ensure that calling functions is prohibited.
+    foo(); // expected-warning{{This function call is prohibited after a successful vfork}}
+    break;
+  default:
+    // Ensure that returning from function is prohibited.
+    return 0; // expected-warning{{Return is prohibited after a successful vfork; call _exit() instead}}
+  }
+
+  while(1);
+}
+
+// Ensure that parent process isn't restricted.
+int f3(int x) {
+  if (vfork() == 0) // expected-warning{{Call to function 'vfork' is insecure}}
+    _exit(1);
+  x = 0; // no-warning
+  foo(); // no-warning
+  return 0;
+} // no-warning
+
+// Unbound pids are special so test them separately.
+void f4(int x) {
+  switch (x) {
+  case 0:
+    vfork(); // expected-warning{{Call to function 'vfork' is insecure}}
+    x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+    break;
+
+  case 1:
+    {
+      char args[2];
+      switch (vfork()) { // expected-warning{{Call to function 'vfork' is insecure}}
+      case 0:
+        args[0] = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+        exit(1);
+      }
+      break;
+    }
+
+  case 2:
+    {
+      pid_t pid;
+      if ((pid = vfork()) == 0) // expected-warning{{Call to function 'vfork' is insecure}}
+        while(1); // no-warning
+      break;
+    }
+  }
+  while(1);
+} //no-warning
+
+
+void f5() {
+  // See "libxtables: move some code to avoid cautions in vfork man page"
+  // (http://lists.netfilter.org/pipermail/netfilter-buglog/2014-October/003280.html).
+  if (vfork() == 0) { // expected-warning{{Call to function 'vfork' is insecure}}
+    execl("prog", "arg1", 0); // no-warning
+    exit(1);  // expected-warning{{This function call is prohibited after a successful vfork}}
+  }
+}
+




More information about the cfe-commits mailing list