From 1c9e6b1bad53486efd12564f76e960efd0d9dd61 Mon Sep 17 00:00:00 2001 From: Zhongxing Xu Date: Sun, 10 Oct 2010 05:45:30 +0000 Subject: [PATCH] Add experimental chroot check which checks improper use of chroot(). Patch by Lei Zhang. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@116163 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Checker/ChrootChecker.cpp | 161 ++++++++++++++++++ .../GRExprEngineExperimentalChecks.cpp | 1 + lib/Checker/GRExprEngineExperimentalChecks.h | 1 + test/Analysis/chroot.c | 24 +++ 4 files changed, 187 insertions(+) create mode 100644 lib/Checker/ChrootChecker.cpp create mode 100644 test/Analysis/chroot.c diff --git a/lib/Checker/ChrootChecker.cpp b/lib/Checker/ChrootChecker.cpp new file mode 100644 index 0000000000..2b29049c07 --- /dev/null +++ b/lib/Checker/ChrootChecker.cpp @@ -0,0 +1,161 @@ +//===- Chrootchecker.cpp -------- Basic security 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 chroot checker, which checks improper use of chroot. +// +//===----------------------------------------------------------------------===// + +#include "GRExprEngineExperimentalChecks.h" +#include "clang/Checker/BugReporter/BugType.h" +#include "clang/Checker/PathSensitive/CheckerVisitor.h" +#include "clang/Checker/PathSensitive/GRState.h" +#include "clang/Checker/PathSensitive/GRStateTrait.h" +#include "clang/Checker/PathSensitive/SymbolManager.h" +#include "llvm/ADT/ImmutableMap.h" +using namespace clang; + +namespace { + +// enum value that represent the jail state +enum Kind { NO_CHROOT, ROOT_CHANGED, JAIL_ENTERED }; + +bool isRootChanged(intptr_t k) { return k == ROOT_CHANGED; } +bool isJailEntered(intptr_t k) { return k == JAIL_ENTERED; } + +// This checker checks improper use of chroot. +// The state transition: +// NO_CHROOT ---chroot(path)--> ROOT_CHANGED ---chdir(/) --> JAIL_ENTERED +// | | +// ROOT_CHANGED<--chdir(..)-- JAIL_ENTERED<--chdir(..)-- +// | | +// bug<--foo()-- JAIL_ENTERED<--foo()-- +class ChrootChecker : public CheckerVisitor { + IdentifierInfo *II_chroot, *II_chdir; + // This bug refers to possibly break out of a chroot() jail. + BuiltinBug *BT_BreakJail; + +public: + ChrootChecker() : II_chroot(0), II_chdir(0), BT_BreakJail(0) {} + + static void *getTag() { + static int x; + return &x; + } + + virtual bool EvalCallExpr(CheckerContext &C, const CallExpr *CE); + virtual void PreVisitCallExpr(CheckerContext &C, const CallExpr *CE); + +private: + void Chroot(CheckerContext &C, const CallExpr *CE); + void Chdir(CheckerContext &C, const CallExpr *CE); +}; + +} // end anonymous namespace + +void clang::RegisterChrootChecker(GRExprEngine &Eng) { + Eng.registerCheck(new ChrootChecker()); +} + +bool ChrootChecker::EvalCallExpr(CheckerContext &C, const CallExpr *CE) { + const GRState *state = C.getState(); + const Expr *Callee = CE->getCallee(); + SVal L = state->getSVal(Callee); + const FunctionDecl *FD = L.getAsFunctionDecl(); + if (!FD) + return false; + + ASTContext &Ctx = C.getASTContext(); + if (!II_chroot) + II_chroot = &Ctx.Idents.get("chroot"); + if (!II_chdir) + II_chdir = &Ctx.Idents.get("chdir"); + + if (FD->getIdentifier() == II_chroot) { + Chroot(C, CE); + return true; + } + if (FD->getIdentifier() == II_chdir) { + Chdir(C, CE); + return true; + } + + return false; +} + +void ChrootChecker::Chroot(CheckerContext &C, const CallExpr *CE) { + const GRState *state = C.getState(); + GRStateManager &Mgr = state->getStateManager(); + + // Once encouter a chroot(), set the enum value ROOT_CHANGED directly in + // the GDM. + state = Mgr.addGDM(state, ChrootChecker::getTag(), (void*) ROOT_CHANGED); + C.addTransition(state); +} + +void ChrootChecker::Chdir(CheckerContext &C, const CallExpr *CE) { + const GRState *state = C.getState(); + GRStateManager &Mgr = state->getStateManager(); + + // If there are no jail state in the GDM, just return. + const void* k = state->FindGDM(ChrootChecker::getTag()); + if (!k) + return; + + // After chdir("/"), enter the jail, set the enum value JAIL_ENTERED. + const Expr *ArgExpr = CE->getArg(0); + SVal ArgVal = state->getSVal(ArgExpr); + + if (const MemRegion *R = ArgVal.getAsRegion()) { + R = R->StripCasts(); + if (const StringRegion* StrRegion= dyn_cast(R)) { + const StringLiteral* Str = StrRegion->getStringLiteral(); + if (Str->getString() == "/") + state = Mgr.addGDM(state, ChrootChecker::getTag(), + (void*) JAIL_ENTERED); + } + } + + C.addTransition(state); +} + +// Check the jail state before any function call except chroot and chdir(). +void ChrootChecker::PreVisitCallExpr(CheckerContext &C, const CallExpr *CE) { + const GRState *state = C.getState(); + const Expr *Callee = CE->getCallee(); + SVal L = state->getSVal(Callee); + const FunctionDecl *FD = L.getAsFunctionDecl(); + if (!FD) + return; + + ASTContext &Ctx = C.getASTContext(); + if (!II_chroot) + II_chroot = &Ctx.Idents.get("chroot"); + if (!II_chdir) + II_chdir = &Ctx.Idents.get("chdir"); + + // Ingnore chroot and chdir. + if (FD->getIdentifier() == II_chroot || FD->getIdentifier() == II_chdir) + return; + + // If jail state is ROOT_CHANGED, generate BugReport. + void* const* k = state->FindGDM(ChrootChecker::getTag()); + if (k) + if (isRootChanged((intptr_t) *k)) + if (ExplodedNode *N = C.GenerateNode()) { + if (!BT_BreakJail) + BT_BreakJail = new BuiltinBug("Break out of jail", + "No call of chdir(\"/\") immediately " + "after chroot"); + BugReport *R = new BugReport(*BT_BreakJail, + BT_BreakJail->getDescription(), N); + C.EmitReport(R); + } + + return; +} diff --git a/lib/Checker/GRExprEngineExperimentalChecks.cpp b/lib/Checker/GRExprEngineExperimentalChecks.cpp index 84262b0043..6f70286e9b 100644 --- a/lib/Checker/GRExprEngineExperimentalChecks.cpp +++ b/lib/Checker/GRExprEngineExperimentalChecks.cpp @@ -22,6 +22,7 @@ void clang::RegisterExperimentalChecks(GRExprEngine &Eng) { // These are checks that never belong as internal checks // within GRExprEngine. RegisterCStringChecker(Eng); + RegisterChrootChecker(Eng); RegisterMallocChecker(Eng); RegisterPthreadLockChecker(Eng); RegisterStreamChecker(Eng); diff --git a/lib/Checker/GRExprEngineExperimentalChecks.h b/lib/Checker/GRExprEngineExperimentalChecks.h index 461318ea74..3380031e02 100644 --- a/lib/Checker/GRExprEngineExperimentalChecks.h +++ b/lib/Checker/GRExprEngineExperimentalChecks.h @@ -20,6 +20,7 @@ namespace clang { class GRExprEngine; void RegisterAnalyzerStatsChecker(GRExprEngine &Eng); +void RegisterChrootChecker(GRExprEngine &Eng); void RegisterCStringChecker(GRExprEngine &Eng); void RegisterIdempotentOperationChecker(GRExprEngine &Eng); void RegisterMallocChecker(GRExprEngine &Eng); diff --git a/test/Analysis/chroot.c b/test/Analysis/chroot.c new file mode 100644 index 0000000000..a0ee450496 --- /dev/null +++ b/test/Analysis/chroot.c @@ -0,0 +1,24 @@ +// RUN: %clang_cc1 -analyze -analyzer-check-objc-mem -analyzer-experimental-checks -analyzer-store region -verify %s + +extern int chroot(const char* path); +extern int chdir(const char* path); + +void foo(void) { +} + +void f1(void) { + chroot("/usr/local"); // root changed. + foo(); // expected-warning {{No call of chdir("/") immediately after chroot}} +} + +void f2(void) { + chroot("/usr/local"); // root changed. + chdir("/"); // enter the jail. + foo(); // no-warning +} + +void f3(void) { + chroot("/usr/local"); // root changed. + chdir("../"); // change working directory, still out of jail. + foo(); // expected-warning {{No call of chdir("/") immediately after chroot}} +}