[analyzer] Add support for namespaces to GenericTaintChecker

This patch introduces the namespaces for the configured functions and
also enables the use of the member functions.

I added an optional Scope field for every configured function. Functions
without Scope match for every function regardless of the namespace.
Functions with Scope will match if the full name of the function starts
with the Scope.
Multiple functions can exist with the same name.

Differential Revision: https://reviews.llvm.org/D70878

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<NameScopeArgs> Filters;
-    std::vector<NameArgsPair> Sinks;
+    std::vector<NameScopeArgs> Sinks;
 
     TaintConfiguration() = default;
     TaintConfiguration(const TaintConfiguration &) = default;
@@ -97,18 +99,49 @@ 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. Returns true on matching.
-  bool checkPre(const CallExpr *CE, const FunctionDecl *FDecl, StringRef Name,
+  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 FunctionDecl *FDecl,
+  bool addSourcesPre(const CallExpr *CE, const FunctionData &FData,
-                     StringRef Name, CheckerContext &C) const;
+                     CheckerContext &C) const;
 
   /// Mark filter's arguments not tainted on a pre-visit. Returns true on
   /// matching.
-  bool addFiltersPre(const CallExpr *CE, StringRef Name,
+  bool addFiltersPre(const CallExpr *CE, const FunctionData &FData,
                      CheckerContext &C) const;
 
   /// Propagate taint generated at pre-visit. Returns true on matching.
@@ -149,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.
@@ -157,8 +190,17 @@ private:
                                CheckerContext &C) const;
 
   struct TaintPropagationRule;
-  using NameRuleMap = llvm::StringMap<TaintPropagationRule>;
+  template <typename T>
-  using NameArgMap = llvm::StringMap<ArgVector>;
+  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.
   ///
@@ -200,8 +242,7 @@ private:
     /// Get the propagation rule for a given function.
     static TaintPropagationRule
     getTaintPropagationRule(const NameRuleMap &CustomPropagations,
-                            const FunctionDecl *FDecl, StringRef Name,
+                            const FunctionData &FData, CheckerContext &C);
-                            CheckerContext &C);
 
     void addSrcArg(unsigned A) { SrcArgs.push_back(A); }
     void addDstArg(unsigned A) { DstArgs.push_back(A); }
@@ -236,14 +277,15 @@ private:
                            CheckerContext &C);
   };
 
-  /// Defines a map between the propagation function's name and
+  /// Defines a map between the propagation function's name, scope
-  /// TaintPropagationRule.
+  /// 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;
 };
 
@@ -260,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 {
@@ -275,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,
@@ -292,10 +335,11 @@ template <> struct ScalarEnumerationTraits<GenericTaintChecker::VariadicType> {
   }
 };
 
-template <> struct MappingTraits<TaintConfig::NameArgsPair> {
+template <> struct MappingTraits<TaintConfig::NameScopeArgs> {
-  static void mapping(IO &IO, TaintConfig::NameArgsPair &NameArg) {
+  static void mapping(IO &IO, TaintConfig::NameScopeArgs &NSA) {
-    IO.mapRequired("Name", NameArg.first);
+    IO.mapRequired("Name", std::get<0>(NSA));
-    IO.mapRequired("Args", NameArg.second);
+    IO.mapOptional("Scope", std::get<1>(NSA));
+    IO.mapRequired("Args", std::get<2>(NSA));
   }
 };
 } // namespace yaml
@@ -328,31 +372,51 @@ void GenericTaintChecker::parseConfiguration(CheckerManager &Mgr,
                                              const std::string &Option,
                                              TaintConfiguration &&Config) {
   for (auto &P : Config.Propagations) {
-    GenericTaintChecker::CustomPropagations.try_emplace(
+    GenericTaintChecker::CustomPropagations.emplace(
-        P.Name, std::move(P.SrcArgs),
+        P.Name,
-        convertToArgVector(Mgr, Option, P.DstArgs), P.VarType, P.VarIndex);
+        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,
+    GenericTaintChecker::CustomFilters.emplace(
-                                                   std::move(F.second));
+        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,
+    const NameRuleMap &CustomPropagations, const FunctionData &FData,
-    StringRef Name, CheckerContext &C) {
+    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}))
@@ -397,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) {
@@ -440,35 +505,32 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
   // or smart memory copy:
   // - memccpy - copying until hitting a special character.
 
-  auto It = CustomPropagations.find(Name);
+  auto It = findFunctionInConfig(CustomPropagations, FData);
-  if (It != CustomPropagations.end())
+  if (It != CustomPropagations.end()) {
-    return It->getValue();
+    const auto &Value = It->second;
+    return Value.second;
+  }
 
   return TaintPropagationRule();
 }
 
 void GenericTaintChecker::checkPreStmt(const CallExpr *CE,
                                        CheckerContext &C) const {
-  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+  Optional<FunctionData> FData = FunctionData::create(CE, C);
-  // Check for non-global functions.
+  if (!FData)
-  if (!FDecl || FDecl->getKind() != Decl::Function)
-    return;
-
-  StringRef Name = C.getCalleeName(FDecl);
-  if (Name.empty())
     return;
 
   // Check for taintedness related errors first: system call, uncontrolled
   // format string, tainted buffer size.
-  if (checkPre(CE, FDecl, Name, C))
+  if (checkPre(CE, *FData, C))
     return;
 
   // Marks the function's arguments and/or return value tainted if it present in
   // the list.
-  if (addSourcesPre(CE, FDecl, Name, C))
+  if (addSourcesPre(CE, *FData, C))
     return;
 
-  addFiltersPre(CE, Name, C);
+  addFiltersPre(CE, *FData, C);
 }
 
 void GenericTaintChecker::checkPostStmt(const CallExpr *CE,
@@ -484,12 +546,11 @@ void GenericTaintChecker::printState(raw_ostream &Out, ProgramStateRef State,
 }
 
 bool GenericTaintChecker::addSourcesPre(const CallExpr *CE,
-                                        const FunctionDecl *FDecl,
+                                        const FunctionData &FData,
-                                        StringRef Name,
                                         CheckerContext &C) const {
   // 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()) {
     ProgramStateRef State = Rule.process(CE, C);
     if (State) {
@@ -500,14 +561,16 @@ bool GenericTaintChecker::addSourcesPre(const CallExpr *CE,
   return false;
 }
 
-bool GenericTaintChecker::addFiltersPre(const CallExpr *CE, StringRef Name,
+bool GenericTaintChecker::addFiltersPre(const CallExpr *CE,
+                                        const FunctionData &FData,
                                         CheckerContext &C) const {
-  auto It = CustomFilters.find(Name);
+  auto It = findFunctionInConfig(CustomFilters, FData);
   if (It == CustomFilters.end())
     return false;
 
   ProgramStateRef State = C.getState();
-  const ArgVector &Args = It->getValue();
+  const auto &Value = It->second;
+  const ArgVector &Args = Value.second;
   for (unsigned ArgNum : Args) {
     if (ArgNum >= CE->getNumArgs())
       continue;
@@ -564,19 +627,19 @@ bool GenericTaintChecker::propagateFromPre(const CallExpr *CE,
 }
 
 bool GenericTaintChecker::checkPre(const CallExpr *CE,
-                                   const FunctionDecl *FDecl, StringRef Name,
+                                   const FunctionData &FData,
                                    CheckerContext &C) const {
 
   if (checkUncontrolledFormatString(CE, C))
     return true;
 
-  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;
@@ -595,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();
 
@@ -848,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;

clang/test/Analysis/Inputs/taint-generic-config.yaml

@@ -9,12 +9,29 @@ Propagations:
   - Name:     mySource2
     DstArgs:  [0]
 
+  # int x = myNamespace::mySource3(); // x is tainted
+  - Name:     mySource3
+    Scope:    "myNamespace::"
+    DstArgs:  [-1]
+
+  # int x = myAnotherNamespace::mySource3(); // x is tainted
+  - Name:     mySource3
+    Scope:    "myAnotherNamespace::"
+    DstArgs:  [-1]
+
   # int x, y;
   # myScanf("%d %d", &x, &y); // x and y are tainted
   - Name:          myScanf
     VariadicType:  Dst
     VariadicIndex: 1
 
+  # int x, y;
+  # Foo::myScanf("%d %d", &x, &y); // x and y are tainted
+  - Name:          myMemberScanf
+    Scope:         "Foo::"
+    VariadicType:  Dst
+    VariadicIndex: 1
+
   # int x; // x is tainted
   # int y;
   # myPropagator(x, &y); // y is tainted
@@ -40,6 +57,18 @@ Filters:
   - Name: isOutOfRange
     Args: [0]
 
+  # int x; // x is tainted
+  # myNamespace::isOutOfRange(&x); // x is not tainted anymore
+  - Name:  isOutOfRange2
+    Scope: "myNamespace::"
+    Args:  [0]
+
+  # int x; // x is tainted
+  # myAnotherNamespace::isOutOfRange(&x); // x is not tainted anymore
+  - Name:  isOutOfRange2
+    Scope: "myAnotherNamespace::"
+    Args:  [0]
+
 # A list of sink functions
 Sinks:
   # int x, y; // x and y are tainted
@@ -48,3 +77,15 @@ Sinks:
   # mySink(0, x, 1); // It won't warn
   - Name: mySink
     Args: [0, 2]
+
+  # int x; // x is tainted
+  # myNamespace::mySink(x); // It will warn
+  - Name:  mySink2
+    Scope: "myNamespace::"
+    Args:  [0]
+
+  # int x; // x is tainted
+  # myAnotherNamespace::mySink(x); // It will warn
+  - Name:  mySink2
+    Scope: "myAnotherNamespace::"
+    Args:  [0]

clang/test/Analysis/taint-generic.cpp

@@ -0,0 +1,126 @@
+// RUN: %clang_analyze_cc1  -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-config alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml -Wno-format-security -verify -std=c++11 %s
+
+#define BUFSIZE 10
+int Buffer[BUFSIZE];
+
+int scanf(const char*, ...);
+int mySource1();
+int mySource3();
+
+bool isOutOfRange2(const int*);
+
+void mySink2(int);
+
+// Test configuration
+namespace myNamespace {
+  void scanf(const char*, ...);
+  void myScanf(const char*, ...);
+  int mySource3();
+
+  bool isOutOfRange(const int*);
+  bool isOutOfRange2(const int*);
+
+  void mySink(int, int, int);
+  void mySink2(int);
+}
+
+namespace myAnotherNamespace {
+  int mySource3();
+
+  bool isOutOfRange2(const int*);
+
+  void mySink2(int);
+}
+
+void testConfigurationNamespacePropagation1() {
+  int x;
+  // The built-in functions should be matched only for functions in
+  // the global namespace
+  myNamespace::scanf("%d", &x);
+  Buffer[x] = 1; // no-warning
+
+  scanf("%d", &x);
+  Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationNamespacePropagation2() {
+  int x = mySource3();
+  Buffer[x] = 1; // no-warning
+
+  int y = myNamespace::mySource3();
+  Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationNamespacePropagation3() {
+  int x = myAnotherNamespace::mySource3();
+  Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationNamespacePropagation4() {
+  int x;
+  // Configured functions without scope should match for all function.
+  myNamespace::myScanf("%d", &x);
+  Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationNamespaceFilter1() {
+  int x = mySource1();
+  if (myNamespace::isOutOfRange2(&x))
+    return;
+  Buffer[x] = 1; // no-warning
+
+  int y = mySource1();
+  if (isOutOfRange2(&y))
+    return;
+  Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationNamespaceFilter2() {
+  int x = mySource1();
+  if (myAnotherNamespace::isOutOfRange2(&x))
+    return;
+  Buffer[x] = 1; // no-warning
+}
+
+void testConfigurationNamespaceFilter3() {
+  int x = mySource1();
+  if (myNamespace::isOutOfRange(&x))
+    return;
+  Buffer[x] = 1; // no-warning
+}
+
+void testConfigurationNamespaceSink1() {
+  int x = mySource1();
+  mySink2(x); // no-warning
+
+  int y = mySource1();
+  myNamespace::mySink2(y);
+  // expected-warning@-1 {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testConfigurationNamespaceSink2() {
+  int x = mySource1();
+  myAnotherNamespace::mySink2(x);
+  // expected-warning@-1 {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testConfigurationNamespaceSink3() {
+  int x = mySource1();
+  myNamespace::mySink(x, 0, 1);
+  // expected-warning@-1 {{Untrusted data is passed to a user-defined sink}}
+}
+
+struct Foo {
+    void scanf(const char*, int*);
+    void myMemberScanf(const char*, int*);
+};
+
+void testConfigurationMemberFunc() {
+  int x;
+  Foo foo;
+  foo.scanf("%d", &x);
+  Buffer[x] = 1; // no-warning
+
+  foo.myMemberScanf("%d", &x);
+  Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
+}