diff options
| author | Dimitry Andric <dim@FreeBSD.org> | 2020-01-17 20:45:01 +0000 | 
|---|---|---|
| committer | Dimitry Andric <dim@FreeBSD.org> | 2020-01-17 20:45:01 +0000 | 
| commit | 706b4fc47bbc608932d3b491ae19a3b9cde9497b (patch) | |
| tree | 4adf86a776049cbf7f69a1929c4babcbbef925eb /clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp | |
| parent | 7cc9cf2bf09f069cb2dd947ead05d0b54301fb71 (diff) | |
Notes
Diffstat (limited to 'clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp')
| -rw-r--r-- | clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp | 234 | 
1 files changed, 163 insertions, 71 deletions
diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index d442b26b39594..302d5bb1bea86 100644 --- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -24,9 +24,10 @@  #include "clang/StaticAnalyzer/Core/CheckerManager.h"  #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"  #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" -#include "llvm/ADT/StringMap.h"  #include "llvm/Support/YAMLTraits.h" +#include <algorithm>  #include <limits> +#include <unordered_map>  #include <utility>  using namespace clang; @@ -56,10 +57,11 @@ public:    /// Used to parse the configuration file.    struct TaintConfiguration { -    using NameArgsPair = std::pair<std::string, ArgVector>; +    using NameScopeArgs = std::tuple<std::string, std::string, ArgVector>;      struct Propagation {        std::string Name; +      std::string Scope;        ArgVector SrcArgs;        SignedArgVector DstArgs;        VariadicType VarType; @@ -67,8 +69,8 @@ public:      };      std::vector<Propagation> Propagations; -    std::vector<NameArgsPair> Filters; -    std::vector<NameArgsPair> Sinks; +    std::vector<NameScopeArgs> Filters; +    std::vector<NameScopeArgs> Sinks;      TaintConfiguration() = default;      TaintConfiguration(const TaintConfiguration &) = default; @@ -97,14 +99,52 @@ private:        BT.reset(new BugType(this, "Use of Untrusted Data", "Untrusted Data"));    } +  struct FunctionData { +    FunctionData() = delete; +    FunctionData(const FunctionData &) = default; +    FunctionData(FunctionData &&) = default; +    FunctionData &operator=(const FunctionData &) = delete; +    FunctionData &operator=(FunctionData &&) = delete; + +    static Optional<FunctionData> create(const CallExpr *CE, +                                         const CheckerContext &C) { +      const FunctionDecl *FDecl = C.getCalleeDecl(CE); +      if (!FDecl || (FDecl->getKind() != Decl::Function && +                     FDecl->getKind() != Decl::CXXMethod)) +        return None; + +      StringRef Name = C.getCalleeName(FDecl); +      std::string FullName = FDecl->getQualifiedNameAsString(); +      if (Name.empty() || FullName.empty()) +        return None; + +      return FunctionData{FDecl, Name, FullName}; +    } + +    bool isInScope(StringRef Scope) const { +      return StringRef(FullName).startswith(Scope); +    } + +    const FunctionDecl *const FDecl; +    const StringRef Name; +    const std::string FullName; +  }; +    /// Catch taint related bugs. Check if tainted data is passed to a -  /// system call etc. -  bool checkPre(const CallExpr *CE, CheckerContext &C) const; +  /// system call etc. Returns true on matching. +  bool checkPre(const CallExpr *CE, const FunctionData &FData, +                CheckerContext &C) const; + +  /// Add taint sources on a pre-visit. Returns true on matching. +  bool addSourcesPre(const CallExpr *CE, const FunctionData &FData, +                     CheckerContext &C) const; -  /// Add taint sources on a pre-visit. -  void addSourcesPre(const CallExpr *CE, CheckerContext &C) const; +  /// Mark filter's arguments not tainted on a pre-visit. Returns true on +  /// matching. +  bool addFiltersPre(const CallExpr *CE, const FunctionData &FData, +                     CheckerContext &C) const; -  /// Propagate taint generated at pre-visit. +  /// Propagate taint generated at pre-visit. Returns true on matching.    bool propagateFromPre(const CallExpr *CE, CheckerContext &C) const;    /// Check if the region the expression evaluates to is the standard input, @@ -142,7 +182,7 @@ private:    /// Check if tainted data is used as a custom sink's parameter.    static constexpr llvm::StringLiteral MsgCustomSink =        "Untrusted data is passed to a user-defined sink"; -  bool checkCustomSinks(const CallExpr *CE, StringRef Name, +  bool checkCustomSinks(const CallExpr *CE, const FunctionData &FData,                          CheckerContext &C) const;    /// Generate a report if the expression is tainted or points to tainted data. @@ -150,8 +190,17 @@ private:                                 CheckerContext &C) const;    struct TaintPropagationRule; -  using NameRuleMap = llvm::StringMap<TaintPropagationRule>; -  using NameArgMap = llvm::StringMap<ArgVector>; +  template <typename T> +  using ConfigDataMap = +      std::unordered_multimap<std::string, std::pair<std::string, T>>; +  using NameRuleMap = ConfigDataMap<TaintPropagationRule>; +  using NameArgMap = ConfigDataMap<ArgVector>; + +  /// Find a function with the given name and scope. Returns the first match +  /// or the end of the map. +  template <typename T> +  static auto findFunctionInConfig(const ConfigDataMap<T> &Map, +                                   const FunctionData &FData);    /// A struct used to specify taint propagation rules for a function.    /// @@ -193,8 +242,7 @@ private:      /// Get the propagation rule for a given function.      static TaintPropagationRule      getTaintPropagationRule(const NameRuleMap &CustomPropagations, -                            const FunctionDecl *FDecl, StringRef Name, -                            CheckerContext &C); +                            const FunctionData &FData, CheckerContext &C);      void addSrcArg(unsigned A) { SrcArgs.push_back(A); }      void addDstArg(unsigned A) { DstArgs.push_back(A); } @@ -229,14 +277,15 @@ private:                             CheckerContext &C);    }; -  /// Defines a map between the propagation function's name and -  /// TaintPropagationRule. +  /// Defines a map between the propagation function's name, scope +  /// and TaintPropagationRule.    NameRuleMap CustomPropagations; -  /// Defines a map between the filter function's name and filtering args. +  /// Defines a map between the filter function's name, scope and filtering +  /// args.    NameArgMap CustomFilters; -  /// Defines a map between the sink function's name and sinking args. +  /// Defines a map between the sink function's name, scope and sinking args.    NameArgMap CustomSinks;  }; @@ -253,7 +302,7 @@ constexpr llvm::StringLiteral GenericTaintChecker::MsgCustomSink;  using TaintConfig = GenericTaintChecker::TaintConfiguration;  LLVM_YAML_IS_SEQUENCE_VECTOR(TaintConfig::Propagation) -LLVM_YAML_IS_SEQUENCE_VECTOR(TaintConfig::NameArgsPair) +LLVM_YAML_IS_SEQUENCE_VECTOR(TaintConfig::NameScopeArgs)  namespace llvm {  namespace yaml { @@ -268,6 +317,7 @@ template <> struct MappingTraits<TaintConfig> {  template <> struct MappingTraits<TaintConfig::Propagation> {    static void mapping(IO &IO, TaintConfig::Propagation &Propagation) {      IO.mapRequired("Name", Propagation.Name); +    IO.mapOptional("Scope", Propagation.Scope);      IO.mapOptional("SrcArgs", Propagation.SrcArgs);      IO.mapOptional("DstArgs", Propagation.DstArgs);      IO.mapOptional("VariadicType", Propagation.VarType, @@ -285,10 +335,11 @@ template <> struct ScalarEnumerationTraits<GenericTaintChecker::VariadicType> {    }  }; -template <> struct MappingTraits<TaintConfig::NameArgsPair> { -  static void mapping(IO &IO, TaintConfig::NameArgsPair &NameArg) { -    IO.mapRequired("Name", NameArg.first); -    IO.mapRequired("Args", NameArg.second); +template <> struct MappingTraits<TaintConfig::NameScopeArgs> { +  static void mapping(IO &IO, TaintConfig::NameScopeArgs &NSA) { +    IO.mapRequired("Name", std::get<0>(NSA)); +    IO.mapOptional("Scope", std::get<1>(NSA)); +    IO.mapRequired("Args", std::get<2>(NSA));    }  };  } // namespace yaml @@ -321,31 +372,51 @@ void GenericTaintChecker::parseConfiguration(CheckerManager &Mgr,                                               const std::string &Option,                                               TaintConfiguration &&Config) {    for (auto &P : Config.Propagations) { -    GenericTaintChecker::CustomPropagations.try_emplace( -        P.Name, std::move(P.SrcArgs), -        convertToArgVector(Mgr, Option, P.DstArgs), P.VarType, P.VarIndex); +    GenericTaintChecker::CustomPropagations.emplace( +        P.Name, +        std::make_pair(P.Scope, TaintPropagationRule{ +                                    std::move(P.SrcArgs), +                                    convertToArgVector(Mgr, Option, P.DstArgs), +                                    P.VarType, P.VarIndex}));    }    for (auto &F : Config.Filters) { -    GenericTaintChecker::CustomFilters.try_emplace(F.first, -                                                   std::move(F.second)); +    GenericTaintChecker::CustomFilters.emplace( +        std::get<0>(F), +        std::make_pair(std::move(std::get<1>(F)), std::move(std::get<2>(F))));    }    for (auto &S : Config.Sinks) { -    GenericTaintChecker::CustomSinks.try_emplace(S.first, std::move(S.second)); +    GenericTaintChecker::CustomSinks.emplace( +        std::get<0>(S), +        std::make_pair(std::move(std::get<1>(S)), std::move(std::get<2>(S))));    }  } +template <typename T> +auto GenericTaintChecker::findFunctionInConfig(const ConfigDataMap<T> &Map, +                                               const FunctionData &FData) { +  auto Range = Map.equal_range(FData.Name); +  auto It = +      std::find_if(Range.first, Range.second, [&FData](const auto &Entry) { +        const auto &Value = Entry.second; +        StringRef Scope = Value.first; +        return Scope.empty() || FData.isInScope(Scope); +      }); +  return It != Range.second ? It : Map.end(); +} +  GenericTaintChecker::TaintPropagationRule  GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule( -    const NameRuleMap &CustomPropagations, const FunctionDecl *FDecl, -    StringRef Name, CheckerContext &C) { +    const NameRuleMap &CustomPropagations, const FunctionData &FData, +    CheckerContext &C) {    // TODO: Currently, we might lose precision here: we always mark a return    // value as tainted even if it's just a pointer, pointing to tainted data.    // Check for exact name match for functions without builtin substitutes. +  // Use qualified name, because these are C functions without namespace.    TaintPropagationRule Rule = -      llvm::StringSwitch<TaintPropagationRule>(Name) +      llvm::StringSwitch<TaintPropagationRule>(FData.FullName)            // Source functions            // TODO: Add support for vfscanf & family.            .Case("fdopen", TaintPropagationRule({}, {ReturnValueIndex})) @@ -390,6 +461,7 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(    // Check if it's one of the memory setting/copying functions.    // This check is specialized but faster then calling isCLibraryFunction. +  const FunctionDecl *FDecl = FData.FDecl;    unsigned BId = 0;    if ((BId = FDecl->getMemoryFunctionKind()))      switch (BId) { @@ -433,23 +505,32 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(    // or smart memory copy:    // - memccpy - copying until hitting a special character. -  auto It = CustomPropagations.find(Name); -  if (It != CustomPropagations.end()) -    return It->getValue(); +  auto It = findFunctionInConfig(CustomPropagations, FData); +  if (It != CustomPropagations.end()) { +    const auto &Value = It->second; +    return Value.second; +  }    return TaintPropagationRule();  }  void GenericTaintChecker::checkPreStmt(const CallExpr *CE,                                         CheckerContext &C) const { +  Optional<FunctionData> FData = FunctionData::create(CE, C); +  if (!FData) +    return; +    // Check for taintedness related errors first: system call, uncontrolled    // format string, tainted buffer size. -  if (checkPre(CE, C)) +  if (checkPre(CE, *FData, C))      return;    // Marks the function's arguments and/or return value tainted if it present in    // the list. -  addSourcesPre(CE, C); +  if (addSourcesPre(CE, *FData, C)) +    return; + +  addFiltersPre(CE, *FData, C);  }  void GenericTaintChecker::checkPostStmt(const CallExpr *CE, @@ -464,31 +545,47 @@ void GenericTaintChecker::printState(raw_ostream &Out, ProgramStateRef State,    printTaint(State, Out, NL, Sep);  } -void GenericTaintChecker::addSourcesPre(const CallExpr *CE, +bool GenericTaintChecker::addSourcesPre(const CallExpr *CE, +                                        const FunctionData &FData,                                          CheckerContext &C) const { -  ProgramStateRef State = nullptr; -  const FunctionDecl *FDecl = C.getCalleeDecl(CE); -  if (!FDecl || FDecl->getKind() != Decl::Function) -    return; - -  StringRef Name = C.getCalleeName(FDecl); -  if (Name.empty()) -    return; -    // First, try generating a propagation rule for this function.    TaintPropagationRule Rule = TaintPropagationRule::getTaintPropagationRule( -      this->CustomPropagations, FDecl, Name, C); +      this->CustomPropagations, FData, C);    if (!Rule.isNull()) { -    State = Rule.process(CE, C); -    if (!State) -      return; -    C.addTransition(State); -    return; +    ProgramStateRef State = Rule.process(CE, C); +    if (State) { +      C.addTransition(State); +      return true; +    } +  } +  return false; +} + +bool GenericTaintChecker::addFiltersPre(const CallExpr *CE, +                                        const FunctionData &FData, +                                        CheckerContext &C) const { +  auto It = findFunctionInConfig(CustomFilters, FData); +  if (It == CustomFilters.end()) +    return false; + +  ProgramStateRef State = C.getState(); +  const auto &Value = It->second; +  const ArgVector &Args = Value.second; +  for (unsigned ArgNum : Args) { +    if (ArgNum >= CE->getNumArgs()) +      continue; + +    const Expr *Arg = CE->getArg(ArgNum); +    Optional<SVal> V = getPointedToSVal(C, Arg); +    if (V) +      State = removeTaint(State, *V);    } -  if (!State) -    return; -  C.addTransition(State); +  if (State != C.getState()) { +    C.addTransition(State); +    return true; +  } +  return false;  }  bool GenericTaintChecker::propagateFromPre(const CallExpr *CE, @@ -530,26 +627,19 @@ bool GenericTaintChecker::propagateFromPre(const CallExpr *CE,  }  bool GenericTaintChecker::checkPre(const CallExpr *CE, +                                   const FunctionData &FData,                                     CheckerContext &C) const {    if (checkUncontrolledFormatString(CE, C))      return true; -  const FunctionDecl *FDecl = C.getCalleeDecl(CE); -  if (!FDecl || FDecl->getKind() != Decl::Function) -    return false; - -  StringRef Name = C.getCalleeName(FDecl); -  if (Name.empty()) -    return false; - -  if (checkSystemCall(CE, Name, C)) +  if (checkSystemCall(CE, FData.Name, C))      return true; -  if (checkTaintedBufferSize(CE, FDecl, C)) +  if (checkTaintedBufferSize(CE, FData.FDecl, C))      return true; -  if (checkCustomSinks(CE, Name, C)) +  if (checkCustomSinks(CE, FData, C))      return true;    return false; @@ -568,7 +658,7 @@ Optional<SVal> GenericTaintChecker::getPointedToSVal(CheckerContext &C,    QualType ArgTy = Arg->getType().getCanonicalType();    if (!ArgTy->isPointerType()) -    return None; +    return State->getSVal(*AddrLoc);    QualType ValTy = ArgTy->getPointeeType(); @@ -821,13 +911,15 @@ bool GenericTaintChecker::checkTaintedBufferSize(const CallExpr *CE,           generateReportIfTainted(CE->getArg(ArgNum), MsgTaintedBufferSize, C);  } -bool GenericTaintChecker::checkCustomSinks(const CallExpr *CE, StringRef Name, +bool GenericTaintChecker::checkCustomSinks(const CallExpr *CE, +                                           const FunctionData &FData,                                             CheckerContext &C) const { -  auto It = CustomSinks.find(Name); +  auto It = findFunctionInConfig(CustomSinks, FData);    if (It == CustomSinks.end())      return false; -  const GenericTaintChecker::ArgVector &Args = It->getValue(); +  const auto &Value = It->second; +  const GenericTaintChecker::ArgVector &Args = Value.second;    for (unsigned ArgNum : Args) {      if (ArgNum >= CE->getNumArgs())        continue;  | 
