Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h @@ -15,9 +15,13 @@ #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H #include "clang/AST/Stmt.h" +#include namespace clang { +class Expr; +class VarDecl; + namespace ento { bool containsMacro(const Stmt *S); @@ -35,6 +39,9 @@ return false; } +std::pair +parseAssignment(const Stmt *S); + } // end GR namespace } // end clang namespace Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -76,6 +76,7 @@ UndefinedAssignmentChecker.cpp UnixAPIChecker.cpp UnreachableCodeChecker.cpp + VforkChecker.cpp VLASizeChecker.cpp VirtualCallChecker.cpp Index: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td +++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td @@ -362,6 +362,10 @@ 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 { Index: cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -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 @@ S = expr->IgnoreParenLValueCasts(); if (IsBind) { - if (const BinaryOperator *BO = dyn_cast(S)) { - if (BO->isAssignmentOp()) - S = BO->getRHS(); - } else if (const DeclStmt *DS = dyn_cast(S)) { - assert(DS->isSingleDecl() && "We process decls one by one"); - if (const VarDecl *VD = dyn_cast(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()) { Index: cfe/trunk/lib/StaticAnalyzer/Checkers/VforkChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/VforkChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/VforkChecker.cpp @@ -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> { + mutable std::unique_ptr BT; + mutable llvm::SmallSet 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() != VFORK_RESULT_INVALID; +} + +bool VforkChecker::isVforkCall(const Decl *D, CheckerContext &C) const { + auto FD = dyn_cast_or_null(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(*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 DVal = + VforkRetVal.getAs(); + 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(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(State->get()); + 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(); +} Index: cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp @@ -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 @@ return false; } + +// Extract lhs and rhs from assignment statement +std::pair +clang::ento::parseAssignment(const Stmt *S) { + const VarDecl *VD = 0; + const Expr *RHS = 0; + + if (auto Assign = dyn_cast_or_null(S)) { + if (Assign->isAssignmentOp()) { + // Ordinary assignment + RHS = Assign->getRHS(); + if (auto DE = dyn_cast_or_null(Assign->getLHS())) + VD = dyn_cast_or_null(DE->getDecl()); + } + } else if (auto PD = dyn_cast_or_null(S)) { + // Initialization + assert(PD->isSingleDecl() && "We process decls one by one"); + VD = dyn_cast_or_null(PD->getSingleDecl()); + RHS = VD->getAnyInitializer(); + } + + return std::make_pair(VD, RHS); +} Index: cfe/trunk/test/Analysis/Inputs/system-header-simulator.h =================================================================== --- cfe/trunk/test/Analysis/Inputs/system-header-simulator.h +++ cfe/trunk/test/Analysis/Inputs/system-header-simulator.h @@ -92,3 +92,13 @@ 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__)); + Index: cfe/trunk/test/Analysis/vfork.c =================================================================== --- cfe/trunk/test/Analysis/vfork.c +++ cfe/trunk/test/Analysis/vfork.c @@ -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}} + } +} +