[analyzer] Add yaml parser to GenericTaintChecker

[boga95]

Summary

Parse the yaml configuration file and store the results in static variables. The user can define taint propagation rules, custom sinks, and filter functions.

[Szelethus]

I think the idea is awesome, thanks!

We really need to not use UINT_MAX, and I think static fields and member functions are a bit overused in this patch -- can't we just make getConfiguration and parseConfiguration static non-member functions, and make them take GenericTaintChecker as an argument?

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

73–75 ↗

(On Diff #191369)

We should definitely change these, not only is the large integer number impossible to remember, but this value could differ on different platforms.

[boga95]

Why is it better not to use static functions/variables? Has it any performance impact?

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

73–75 ↗

(On Diff #191369)

I tried to use int, but I got a lot of warnings because of the getNumArgs() returns an unsigned value.

[Szelethus]

In D59555#1442331, @boga95 wrote:

Why is it better not to use static functions/variables? Has it any performance impact?

I don't think so, I just think that it's easier to follow what's happening with a non-static field. Also, I just don't see why it needs to be static, you only use GenericTaintChecker::Custom* where you could easily access it. Especially GenericTaintChecker::getConfiguration -- if it wasn't static, you wouldn't even need to take the checker as a parameter.

[Szelethus]

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

73–75 ↗

(On Diff #191369)

What warnings? I thought we have -Wsign-conversion disabled.

[boga95]

Rebase after https://reviews.llvm.org/D59861.

[Szelethus]

We agreed to disagree on the static function stuff -- it doesn't really matter, and I don't insist. I have no objections against this patch, great job! I won't formally accept to make it stand out a little more. Thanks!

[Szelethus]

Sorry, I rushed my earlier comment -- I definitely think that we should get rid of the UINT_MAX thing before landing this.

[NoQ]

This new approach is clearly useful to other checkers as well, not only the Taint checker. I believe we should strongly consider generalizing it somehow, it's just too awesome to restrict to a single checker.

There's also a closely related technology called "API Notes" that allows you to inject annotations into system headers by shipping .yaml files with the compiler. It was actively pushed for for the Analyzer in particular but ended up never getting upstreamed:

• http://lists.llvm.org/pipermail/cfe-dev/2015-December/046335.html
• http://lists.llvm.org/pipermail/cfe-dev/2017-May/053860.html

I suspect that it'll make perfect sense for your use case to define a clang attribute for taint sources and sinks and then define an API notes definition that'll inject this attribute into headers. If only API notes were available, this would have been a perfect use case for them. I think it might be a good idea to ping the mailing lists about API notes one more time, announce that you *are* going for a similar technology anyway, and see if anybody suggests something.

Pros/cons:

• The attributes/API notes solution is very comfy because it both allows users to address their false positives / false negatives by adding annotations in their source *and* allows us to annotate system headers via yaml files, and (if i understand correctly) both sorts of annotations are accessible uniformly via Decl->getAttr<...>().
  • If we decide to go for our own yaml format that doesn't work on top of attributes, we'll either get only one of these, or double up our implementation burden in checkers.
    • Implementation burden should not be that high, but it will be annoying.
  • If we don't want users to annotate their headers with source-level annotations, it sounds easier to go for a custom yaml parser, because defining attributes is annoying.
    • But i believe we do want them to in this case.

Does any of this make any sense within the bigger plans that you have for this checker?

[boga95]

Sorry for the late answer, I was working on my thesis which is about taint analysis. During that, I implemented several cool features (namespaces, std::cin, std::string, etc.) for the checker. I will share them soon.

I thought about the API notes and I think it will fit very well into the checker. If my understanding is clear, the checker would be configured with attributes and/or a yaml file which contains the attributes. Therefore, the implementation will become simpler, because the checker will only read the attributes. I made a draft for the possible usage of the attributes:

[[taint::dst(-1)]]
int mySource(); // The return value will become tainted

[[taint::src(0, 1)]] [[taint::dst(2)]]
void myPropagator(int*, int*, int*);

[[taint::src(0)]] [[taint::varDst(2)]]
int myFscanf(FILE*, const char*, ...); // The variadic arguments will become tainted, if the first argument is tainted

[[taint::dst(0, -1)]] [[taint::varSrc(2)]]
int mySprintf(char*, const char*, ... ); // The first argument and the return value will become tainted, if any of the variadic arguments is tainted

I think we can use the current yaml configuration in order to not block my progress. I think most of the current implementation can be reused for the API notes. I will be able to easily change the interface after the API notes are ready.

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

73–75 ↗

(On Diff #191369)

I got -Wsign-compare warnings, but it compiles. I will change it in the next review because that's contains the yaml file and the related tests.

[boga95]

I changed the parsing, therefore the return value index is represented by -1.
I added a test configuration file and parse it when testing to prove the configuration doesn't break the code.

[NoQ]

Ok! This looks like there isn't that much code, so it should be fine to duplicate. Do you have plans to eliminate the existing switch and instead use yaml for *all* supported functions while shipping a default .yaml file with the analyzer?

I'm still in doubts on how to connect your work with the CallDescription effort. I'll think more about that.

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

210–218 ↗

(On Diff #199990)

I think you don't need to make them static. By the time you need them, you already have access to the instance of the checker. Static variables of non-trivial types are frowned upon (https://llvm.org/docs/CodingStandards.html#do-not-use-static-constructors).

294 ↗

(On Diff #199990)

Let's re-use at least this function. I.e., abstract it out from GenericTaintChecker and put it into a header accessible by all checkers (dunno, lib/StaticAnalyzer/Checkers/Yaml.h).

316 ↗

(On Diff #199990)

Do we ever need to copy the config? Maybe make it a move-only type?

[boga95]

I already thought about it. It would make the code much cleaner, but it would have a little performance impact (Does it matter?).
It's straightforward to read the supported functions from another yaml file. Besides that, it can support multiple config files too.

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

73–75 ↗

(On Diff #191369)

Now, this is just for internal representation. The -1 value is mapped to this.

[xazax.hun]

Generally looks good, some nits inline. Please mark comments you already solved as done.

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

52 ↗

(On Diff #199990)

Is there a way to have only one type for argument vectors?

312 ↗

(On Diff #199990)

We tend to not write the braces in small cases like this.

73–75 ↗

(On Diff #191369)

What about the C++ way using numeric limits?

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

16 ↗

(On Diff #199990)

Here Var stands for variadic I guess. I would prefer to avoid abbreviations as they might be misleading.

[NoQ]

In D59555#1514602, @NoQ wrote:

I'm still in doubts on how to connect your work with the CallDescription effort. I'll think more about that.

I guess i'll just make a yaml reader for CallDescriptions as soon as the interface settles down a bit, and then propose you to switch to using it.

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

804–805 ↗

(On Diff #203339)

I think i'll softly advocate for a more centralized format that doesn't require every checker to implement an option for just that purpose.

Will you be happy with a global analyzer flag, eg. -analyzer-config api-yaml=/home/foo/analyzer.yaml and then:

Checker:
    Name: alpha.security.taint.TaintPropagation
    Config:
        Propagations:
        ...

with possibly multiple checkers in the same file? I guess we can change it later if you don't mind breaking flag compatibility.

lib/StaticAnalyzer/Checkers/Yaml.h

16–17 ↗

(On Diff #203339)

I believe we should emit a compile error-like diagnostic here. One of the good things about compile errors would be that GUIs like scan-build would notify their users about compile errors in a friendly manner, while dumps to llvm::errs() will be completely ignored.

[boga95]

Report diagnostic error in case of an invalid filename or an ill-formed yaml file.

[Szelethus]

Starting to look real good!

include/clang/StaticAnalyzer/Checkers/Checkers.td

807 ↗

(On Diff #210073)

We mark options that are not yet ready for used with InAlpha, rather then Released. I think it would be more fitting in this case!

Mind that if you'd like to list this option after that, you'd have to use

clang -cc1 -analyzer-checker-option-help-alpha

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

58 ↗

(On Diff #210073)

Just simply Used to parse the configuration file.
https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments

302–303 ↗

(On Diff #210073)

Do we emit an error for this case?

lib/StaticAnalyzer/Checkers/Yaml.h

1 ↗

(On Diff #210073)

Header blurb (licence stuff etc)!

test/Analysis/taint-generic.c

1–2 ↗

(On Diff #210073)

Could you please use line breaks here? I know it isn't your making, but if we're touching it anyways :^)

// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
// RUN:   -DFILE_IS_STRUCT \
// RUN:   -analyzer-checker=alpha.security.taint \
// RUN:   -analyzer-checker=core \
// RUN:   -analyzer-checker=alpha.security.ArrayBoundV2 \
// RUN:   -analyzer-config \
// RUN:     alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml

And also a testcase for an incorrect checker option:

// RUN: not %clang_analyze_cc1 -verify %s \
// RUN:   -analyzer-checker=alpha.security.taint \
// RUN:   -analyzer-config \
// RUN:     alpha.security.taint.TaintPropagation:Config=justguessit \
// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-FILE

// CHECK-INVALID-FILE: (frontend): invalid input for checker option
// CHECK-INVALID-FILE-SAME:        'alpha.security.taint.TaintPropagation:Config',
// CHECK-INVALID-FILE-SAME:        that expects a valid yaml file

something like that.

[Szelethus]

Hmm, okay, so we convert -1 from the config file to UINT_MAX in the code, I like it!

I wrote a couple nits but they really are just that. In general, for each different error message, a different test case would be great.

include/clang/StaticAnalyzer/Checkers/Checkers.td

805 ↗

(On Diff #210260)

Okay, so how do I know what the format of that file is? How about we create another option (in a different revision) that dumps an example configuration with comments?

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

27 ↗

(On Diff #210260)

Do we need this include?

89–92 ↗

(On Diff #210260)

Leave this as is for now, but maaybe in the future we should just use an enum. What do you think?

304–306 ↗

(On Diff #210260)

Could we have a test for this too? :)

lib/StaticAnalyzer/Checkers/Yaml.h

1 ↗

(On Diff #210260)

Have with with the same length as the rest :)

9 ↗

(On Diff #210260)

Is this actually the reason why we have this file? We already have a YAML parser in LLVM, what's does this file do that the "default" parser doesn't?

14 ↗

(On Diff #210260)

Hmmm, do we need this include?

16 ↗

(On Diff #210260)

namespace clang {
namespace ento {

42–43 ↗

(On Diff #210260)

And for this too?

48 ↗

(On Diff #210260)

} // end of namespace clang
} // end of namespace ento

test/Analysis/taint-generic.c

24 ↗

(On Diff #210260)

Could you please add the rest of the error message?

[NoQ]

Looks great to me once @Szelethus's comments are addressed. Thanks!!

lib/StaticAnalyzer/Checkers/Yaml.h

34 ↗

(On Diff #210260)

I suggest None for being more explicit and readable.

test/Analysis/taint-generic.c

24 ↗

(On Diff #210260)

I'd rather remove the rest of the error message. There's no need to duplicate something that the user has already written on the command line.

Or do we think like \escapes?

[Szelethus]

lib/StaticAnalyzer/Checkers/Yaml.h

48 ↗

(On Diff #210260)

I mean, the other way around >.<

test/Analysis/taint-generic.c

24 ↗

(On Diff #210260)

I'm not sure what \escapes mean -- however, if we emit the filename in the error message, we should test it. Also, how many filenames has the user written in the comand line? Include paths for this and that, output file, inputfile, files to link against... I would very much prefer preserving the current error message.

[NoQ]

test/Analysis/taint-generic.c

24 ↗

(On Diff #210260)

I'm thinking, like, the user is using zsh and writing file#1.txt instead of file\#1.txt.

[Szelethus]

LGTM, thanks! Please mark inlines as done as you fix them. If you like the file description I proposed, I'm happy to commit this on your behalf (or you might as well apply for a commit access, since you have a track record of high quality patches already).

lib/StaticAnalyzer/Checkers/Yaml.h

9 ↗

(On Diff #210260)

I would appreciate some comments, it still isn't immediately obvious why we need yet another yaml file. How about

"This file defines convenience functions for handling YAML configuration files for checkers/packages".

[xiaobai]

cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

74

Is there a particular reason this is marked as delete? This present an issue because the Optional constructed in ento::registerGenericTaintChecker will use this constructor. I don't hit this issue with any recent version of clang (tried with 6, 7, and 8) but I am hitting it with clang 3.8 on the swift-ci ubuntu bots.

cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td

 //===----------------------------------------------------------------------===//
 // Taint checkers.
 //===----------------------------------------------------------------------===//
 
 let ParentPackage = Taint in {
 
 def GenericTaintChecker : Checker<"TaintPropagation">,
   HelpText<"Generate taint information used by other checkers">,
+  CheckerOptions<[
+    CmdLineOption<String,
+                  "Config",
+                  "Specifies the name of the configuration file.",
+                  "",
+                  InAlpha>,
+  ]>,
   Documentation<HasAlphaDocumentation>;
 
 } // end "alpha.security.taint"
 
 //===----------------------------------------------------------------------===//
 // Mac OS X, Cocoa, and Core Foundation checkers.
 //===----------------------------------------------------------------------===//
 

cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

 //
 // The taint information produced by it might be useful to other checkers. For
 // example, checkers should report errors which involve tainted data more
 // aggressively, even if the involved symbols are under constrained.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Taint.h"
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "Yaml.h"
 #include "clang/AST/Attr.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
-#include <climits>
-#include <initializer_list>
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/YAMLTraits.h"
+#include <limits>
 #include <utility>
 
 using namespace clang;
 using namespace ento;
 using namespace taint;
 
 namespace {
 class GenericTaintChecker
     : public Checker<check::PostStmt<CallExpr>, check::PreStmt<CallExpr>> {
 public:
   static void *getTag() {
     static int Tag;
     return &Tag;
   }
 
   void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
 
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
 
-  void printState(raw_ostream &Out, ProgramStateRef State,
-                  const char *NL, const char *Sep) const override;
+  void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
+                  const char *Sep) const override;
 
-private:
-  static const unsigned InvalidArgIndex = UINT_MAX;
+  using ArgVector = SmallVector<unsigned, 2>;
+  using SignedArgVector = SmallVector<int, 2>;
+
+  enum class VariadicType { None, Src, Dst };
+
+  /// Used to parse the configuration file.
+  struct TaintConfiguration {
+    using NameArgsPair = std::pair<std::string, ArgVector>;
+
+    struct Propagation {
+      std::string Name;
+      ArgVector SrcArgs;
+      SignedArgVector DstArgs;
+      VariadicType VarType;
+      unsigned VarIndex;
+    };
+
+    std::vector<Propagation> Propagations;
+    std::vector<NameArgsPair> Filters;
+    std::vector<NameArgsPair> Sinks;
+
+    TaintConfiguration() = default;
+    TaintConfiguration(const TaintConfiguration &) = delete;
+    TaintConfiguration(TaintConfiguration &&) = default;
+    TaintConfiguration &operator=(const TaintConfiguration &) = delete;
+    TaintConfiguration &operator=(TaintConfiguration &&) = default;
+  };
+
+  /// Convert SignedArgVector to ArgVector.
+  ArgVector convertToArgVector(CheckerManager &Mgr, const std::string &Option,
+                               SignedArgVector Args);
+
+  /// Parse the config.
+  void parseConfiguration(CheckerManager &Mgr, const std::string &Option,
+                          TaintConfiguration &&Config);
+
+  static const unsigned InvalidArgIndex{std::numeric_limits<unsigned>::max()};
   /// Denotes the return vale.
-  static const unsigned ReturnValueIndex = UINT_MAX - 1;
+  static const unsigned ReturnValueIndex{std::numeric_limits<unsigned>::max() -
+                                         1};
 
+private:
   mutable std::unique_ptr<BugType> BT;
   void initBugType() const {
     if (!BT)
       BT.reset(new BugType(this, "Use of Untrusted Data", "Untrusted Data"));
   }
 
   /// Catch taint related bugs. Check if tainted data is passed to a
   /// system call etc.
private:
   static const char MsgTaintedBufferSize[];
   bool checkTaintedBufferSize(const CallExpr *CE, const FunctionDecl *FDecl,
                               CheckerContext &C) const;
 
   /// Generate a report if the expression is tainted or points to tainted data.
   bool generateReportIfTainted(const Expr *E, const char Msg[],
                                CheckerContext &C) const;
 
-  using ArgVector = SmallVector<unsigned, 2>;
-
   /// A struct used to specify taint propagation rules for a function.
   ///
   /// If any of the possible taint source arguments is tainted, all of the
   /// destination arguments should also be tainted. Use InvalidArgIndex in the
   /// src list to specify that all of the arguments can introduce taint. Use
   /// InvalidArgIndex in the dst arguments to signify that all the non-const
   /// pointer and reference arguments might be tainted on return. If
   /// ReturnValueIndex is added to the dst list, the return value will be
   /// tainted.
   struct TaintPropagationRule {
-    enum class VariadicType { None, Src, Dst };
-
     using PropagationFuncType = bool (*)(bool IsTainted, const CallExpr *,
                                          CheckerContext &C);
 
     /// List of arguments which can be taint sources and should be checked.
     ArgVector SrcArgs;
     /// List of arguments which should be tainted on function return.
     ArgVector DstArgs;
     /// Index for the first variadic parameter if exist.
     unsigned VariadicIndex;
     /// Show when a function has variadic parameters. If it has, it marks all
     /// of them as source or destination.
     VariadicType VarType;
     /// Special function for tainted source determination. If defined, it can
     /// override the default behavior.
     PropagationFuncType PropagationFunc;
 
     TaintPropagationRule()
         : VariadicIndex(InvalidArgIndex), VarType(VariadicType::None),
           PropagationFunc(nullptr) {}
 
-    TaintPropagationRule(std::initializer_list<unsigned> &&Src,
-                         std::initializer_list<unsigned> &&Dst,
+    TaintPropagationRule(ArgVector &&Src, ArgVector &&Dst,
                          VariadicType Var = VariadicType::None,
                          unsigned VarIndex = InvalidArgIndex,
                          PropagationFuncType Func = nullptr)
         : SrcArgs(std::move(Src)), DstArgs(std::move(Dst)),
           VariadicIndex(VarIndex), VarType(Var), PropagationFunc(Func) {}
 
     /// Get the propagation rule for a given function.
     static TaintPropagationRule
struct TaintPropagationRule {
     /// Pre-process a function which propagates taint according to the
     /// taint rule.
     ProgramStateRef process(const CallExpr *CE, CheckerContext &C) const;
 
     // Functions for custom taintedness propagation.
     static bool postSocket(bool IsTainted, const CallExpr *CE,
                            CheckerContext &C);
   };
+
+  using NameRuleMap = llvm::StringMap<TaintPropagationRule>;
+  using NameArgMap = llvm::StringMap<ArgVector>;
+
+  /// Defines a map between the propagation function's name and
+  /// TaintPropagationRule.
+  NameRuleMap CustomPropagations;
+
+  /// Defines a map between the filter function's name and filtering args.
+  NameArgMap CustomFilters;
+
+  /// Defines a map between the sink function's name and sinking args.
+  NameArgMap CustomSinks;
 };
 
 const unsigned GenericTaintChecker::ReturnValueIndex;
 const unsigned GenericTaintChecker::InvalidArgIndex;
 
 const char GenericTaintChecker::MsgUncontrolledFormatString[] =
     "Untrusted data is used as a format string "
     "(CWE-134: Uncontrolled Format String)";
 
 const char GenericTaintChecker::MsgSanitizeSystemArgs[] =
     "Untrusted data is passed to a system call "
     "(CERT/STR02-C. Sanitize data passed to complex subsystems)";
 
 const char GenericTaintChecker::MsgTaintedBufferSize[] =
     "Untrusted data is used to specify the buffer size "
     "(CERT/STR31-C. Guarantee that storage for strings has sufficient space "
     "for character data and the null terminator)";
-
 } // end of anonymous namespace
 
+using TaintConfig = GenericTaintChecker::TaintConfiguration;
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(TaintConfig::Propagation)
+LLVM_YAML_IS_SEQUENCE_VECTOR(TaintConfig::NameArgsPair)
+
+namespace llvm {
+namespace yaml {
+template <> struct MappingTraits<TaintConfig> {
+  static void mapping(IO &IO, TaintConfig &Config) {
+    IO.mapOptional("Propagations", Config.Propagations);
+    IO.mapOptional("Filters", Config.Filters);
+    IO.mapOptional("Sinks", Config.Sinks);
+  }
+};
+
+template <> struct MappingTraits<TaintConfig::Propagation> {
+  static void mapping(IO &IO, TaintConfig::Propagation &Propagation) {
+    IO.mapRequired("Name", Propagation.Name);
+    IO.mapOptional("SrcArgs", Propagation.SrcArgs);
+    IO.mapOptional("DstArgs", Propagation.DstArgs);
+    IO.mapOptional("VariadicType", Propagation.VarType,
+                   GenericTaintChecker::VariadicType::None);
+    IO.mapOptional("VariadicIndex", Propagation.VarIndex,
+                   GenericTaintChecker::InvalidArgIndex);
+  }
+};
+
+template <> struct ScalarEnumerationTraits<GenericTaintChecker::VariadicType> {
+  static void enumeration(IO &IO, GenericTaintChecker::VariadicType &Value) {
+    IO.enumCase(Value, "None", GenericTaintChecker::VariadicType::None);
+    IO.enumCase(Value, "Src", GenericTaintChecker::VariadicType::Src);
+    IO.enumCase(Value, "Dst", GenericTaintChecker::VariadicType::Dst);
+  }
+};
+
+template <> struct MappingTraits<TaintConfig::NameArgsPair> {
+  static void mapping(IO &IO, TaintConfig::NameArgsPair &NameArg) {
+    IO.mapRequired("Name", NameArg.first);
+    IO.mapRequired("Args", NameArg.second);
+  }
+};
+} // namespace yaml
+} // namespace llvm
+
 /// A set which is used to pass information from call pre-visit instruction
 /// to the call post-visit. The values are unsigned integers, which are either
 /// ReturnValueIndex, or indexes of the pointer/reference argument, which
 /// points to data, which should be tainted on return.
 REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, unsigned)
 
+GenericTaintChecker::ArgVector GenericTaintChecker::convertToArgVector(
+    CheckerManager &Mgr, const std::string &Option, SignedArgVector Args) {
+  ArgVector Result;
+  for (int Arg : Args) {
+    if (Arg == -1)
+      Result.push_back(ReturnValueIndex);
+    else if (Arg < -1) {
+      Result.push_back(InvalidArgIndex);
+      Mgr.reportInvalidCheckerOptionValue(
+          this, Option,
+          "an argument number for propagation rules greater or equal to -1");
+    } else
+      Result.push_back(static_cast<unsigned>(Arg));
+  }
+  return Result;
+}
+
+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);
+  }
+
+  for (auto &F : Config.Filters) {
+    GenericTaintChecker::CustomFilters.try_emplace(F.first,
+                                                   std::move(F.second));
+  }
+
+  for (auto &S : Config.Sinks) {
+    GenericTaintChecker::CustomSinks.try_emplace(S.first, std::move(S.second));
+  }
+}
+
 GenericTaintChecker::TaintPropagationRule
 GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
     const FunctionDecl *FDecl, StringRef Name, 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.
   TaintPropagationRule Rule =
       llvm::StringSwitch<TaintPropagationRule>(Name)
           // Source functions
           // TODO: Add support for vfscanf & family.
           .Case("fdopen", TaintPropagationRule({}, {ReturnValueIndex}))
           .Case("fopen", TaintPropagationRule({}, {ReturnValueIndex}))
           .Case("freopen", TaintPropagationRule({}, {ReturnValueIndex}))
           .Case("getch", TaintPropagationRule({}, {ReturnValueIndex}))
           .Case("getchar", TaintPropagationRule({}, {ReturnValueIndex}))
-          .Case("getchar_unlocked", TaintPropagationRule({}, {ReturnValueIndex}))
+          .Case("getchar_unlocked",
+                TaintPropagationRule({}, {ReturnValueIndex}))
           .Case("getenv", TaintPropagationRule({}, {ReturnValueIndex}))
           .Case("gets", TaintPropagationRule({}, {0, ReturnValueIndex}))
           .Case("scanf", TaintPropagationRule({}, {}, VariadicType::Dst, 1))
           .Case("socket",
                 TaintPropagationRule({}, {ReturnValueIndex}, VariadicType::None,
                                      InvalidArgIndex,
                                      &TaintPropagationRule::postSocket))
           .Case("wgetch", TaintPropagationRule({}, {ReturnValueIndex}))
if (ArgNum >= CE->getNumArgs())
       return State;
     if ((IsTainted = isTaintedOrPointsToTainted(CE->getArg(ArgNum), State, C)))
       break;
   }
 
   // Check for taint in variadic arguments.
   if (!IsTainted && VariadicType::Src == VarType) {
     // Check if any of the arguments is tainted
-    for (unsigned int i = VariadicIndex; i < CE->getNumArgs(); ++i) {
+    for (unsigned i = VariadicIndex; i < CE->getNumArgs(); ++i) {
       if ((IsTainted = isTaintedOrPointsToTainted(CE->getArg(i), State, C)))
         break;
     }
   }
 
   if (PropagationFunc)
     IsTainted = PropagationFunc(IsTainted, CE, C);
 
GenericTaintChecker::TaintPropagationRule::process(const CallExpr *CE,
   }
 
   // Mark all variadic arguments tainted if present.
   if (VariadicType::Dst == VarType) {
     // For all pointer and references that were passed in:
     //   If they are not pointing to const data, mark data as tainted.
     //   TODO: So far we are just going one level down; ideally we'd need to
     //         recurse here.
-    for (unsigned int i = VariadicIndex; i < CE->getNumArgs(); ++i) {
+    for (unsigned i = VariadicIndex; i < CE->getNumArgs(); ++i) {
       const Expr *Arg = CE->getArg(i);
       // Process pointer argument.
       const Type *ArgTy = Arg->getType().getTypePtr();
       QualType PType = ArgTy->getPointeeType();
       if ((!PType.isNull() && !PType.isConstQualified()) ||
           (ArgTy->isReferenceType() && !Arg->getType().isConstQualified()))
         State = State->add<TaintArgsOnPostVisit>(i);
     }
if ((D->getName().find("stdin") != StringRef::npos) && D->isExternC()) {
         return true;
     }
   }
   return false;
 }
 
 static bool getPrintfFormatArgumentNum(const CallExpr *CE,
                                        const CheckerContext &C,
-                                       unsigned int &ArgNum) {
+                                       unsigned &ArgNum) {
   // Find if the function contains a format string argument.
   // Handles: fprintf, printf, sprintf, snprintf, vfprintf, vprintf, vsprintf,
   // vsnprintf, syslog, custom annotated functions.
   const FunctionDecl *FDecl = C.getCalleeDecl(CE);
   if (!FDecl)
     return false;
   for (const auto *Format : FDecl->specific_attrs<FormatAttr>()) {
     ArgNum = Format->getFormatIdx() - 1;
if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
     return true;
   }
   return false;
 }
 
 bool GenericTaintChecker::checkUncontrolledFormatString(
     const CallExpr *CE, CheckerContext &C) const {
   // Check if the function contains a format string argument.
-  unsigned int ArgNum = 0;
+  unsigned ArgNum = 0;
   if (!getPrintfFormatArgumentNum(CE, C, ArgNum))
     return false;
 
   // If either the format string content or the pointer itself are tainted,
   // warn.
   return generateReportIfTainted(CE->getArg(ArgNum),
                                  MsgUncontrolledFormatString, C);
 }
if (ArgNum == InvalidArgIndex) {
     else if (C.isCLibraryFunction(FDecl, "bcopy"))
       ArgNum = 2;
   }
 
   return ArgNum != InvalidArgIndex && CE->getNumArgs() > ArgNum &&
          generateReportIfTainted(CE->getArg(ArgNum), MsgTaintedBufferSize, C);
 }
 
-void ento::registerGenericTaintChecker(CheckerManager &mgr) {
-  mgr.registerChecker<GenericTaintChecker>();
+void ento::registerGenericTaintChecker(CheckerManager &Mgr) {
+  auto *Checker = Mgr.registerChecker<GenericTaintChecker>();
+  std::string Option{"Config"};
+  StringRef ConfigFile =
+      Mgr.getAnalyzerOptions().getCheckerStringOption(Checker, Option);
+  llvm::Optional<TaintConfig> Config =
+      getConfiguration<TaintConfig>(Mgr, Checker, Option, ConfigFile);
+  if (Config)
+    Checker->parseConfiguration(Mgr, Option, std::move(Config).getValue());
 }
 
 bool ento::shouldRegisterGenericTaintChecker(const LangOptions &LO) {
   return true;
 }

cfe/trunk/lib/StaticAnalyzer/Checkers/Yaml.h

+//== Yaml.h ---------------------------------------------------- -*- C++ -*--=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines convenience functions for handling YAML configuration files
+// for checkers/packages.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKER_YAML_H
+#define LLVM_CLANG_LIB_STATICANALYZER_CHECKER_YAML_H
+
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "llvm/Support/YAMLTraits.h"
+
+namespace clang {
+namespace ento {
+
+/// Read the given file from the filesystem and parse it as a yaml file. The
+/// template parameter must have a yaml MappingTraits.
+/// Emit diagnostic error in case of any failure.
+template <class T, class Checker>
+llvm::Optional<T> getConfiguration(CheckerManager &Mgr, Checker *Chk,
+                                   StringRef Option, StringRef ConfigFile) {
+  if (ConfigFile.trim().empty())
+    return None;
+
+  llvm::vfs::FileSystem *FS = llvm::vfs::getRealFileSystem().get();
+  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
+      FS->getBufferForFile(ConfigFile.str());
+
+  if (std::error_code ec = Buffer.getError()) {
+    Mgr.reportInvalidCheckerOptionValue(Chk, Option,
+                                        "a valid filename instead of '" +
+                                            std::string(ConfigFile) + "'");
+    return None;
+  }
+
+  llvm::yaml::Input Input(Buffer.get()->getBuffer());
+  T Config;
+  Input >> Config;
+
+  if (std::error_code ec = Input.error()) {
+    Mgr.reportInvalidCheckerOptionValue(Chk, Option,
+                                        "a valid yaml file: " + ec.message());
+    return None;
+  }
+
+  return Config;
+}
+
+} // namespace ento
+} // namespace clang
+
+#endif // LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MOVE_H

cfe/trunk/test/Analysis/Inputs/taint-generic-config-ill-formed.yaml

+Propagations:
+  - Name:     mySource1
+    DstArgs:  [-1]
+    NotExist: 1

cfe/trunk/test/Analysis/Inputs/taint-generic-config-invalid-arg.yaml

+Propagations:
+  - Name:     mySource1
+    DstArgs:  [-2]

cfe/trunk/test/Analysis/Inputs/taint-generic-config.yaml

+# A list of source/propagation function
+Propagations:
+  # int x = mySource1(); // x is tainted
+  - Name:     mySource1
+    DstArgs:  [-1] # Index for return value
+
+  # int x;
+  # mySource2(&x); // x is tainted
+  - Name:     mySource2
+    DstArgs:  [0]
+
+  # int x, y;
+  # myScanf("%d %d", &x, &y); // x and y are tainted
+  - Name:          myScanf
+    VariadicType:  Dst
+    VariadicIndex: 1
+
+  # int x; // x is tainted
+  # int y;
+  # myPropagator(x, &y); // y is tainted
+  - Name:     myPropagator
+    SrcArgs:  [0]
+    DstArgs:  [1]
+
+  # constexpr unsigned size = 100;
+  # char buf[size];
+  # int x, y;
+  # int n = mySprintf(buf, size, "%d %d", x, y); // If size, x or y is tainted
+  # // the return value and the buf will be tainted
+  - Name:          mySnprintf
+    SrcArgs:       [1]
+    DstArgs:       [0, -1]
+    VariadicType:  Src
+    VariadicIndex: 3
+
+# A list of filter functions
+Filters:
+  # int x; // x is tainted
+  # myFilter(&x); // x is not tainted anymore
+  - Name: myFilter
+    Args: [0]
+
+# A list of sink functions
+Sinks:
+  # int x, y; // x and y are tainted
+  # mySink(x, 0, 1); // It will warn
+  # mySink(0, 1, y); // It will warn
+  # mySink(0, x, 1); // It won't warn
+  - Name: mySink
+    Args: [0, 2]

cfe/trunk/test/Analysis/analyzer-config.c

 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.ConfigDumper > %t 2>&1
 // RUN: FileCheck --input-file=%t %s --match-full-lines
 
 // CHECK: [config]
 // CHECK-NEXT: add-pop-up-notes = true
 // CHECK-NEXT: aggressive-binary-operation-simplification = false
 // CHECK-NEXT: alpha.clone.CloneChecker:IgnoredFilesPattern = ""
 // CHECK-NEXT: alpha.clone.CloneChecker:MinimumCloneComplexity = 50
 // CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
+// CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""
 // CHECK-NEXT: avoid-suppressing-null-argument-paths = false
 // CHECK-NEXT: c++-allocator-inlining = true
 // CHECK-NEXT: c++-container-inlining = false
 // CHECK-NEXT: c++-inlining = destructors
 // CHECK-NEXT: c++-shared_ptr-inlining = false
 // CHECK-NEXT: c++-stdlib-inlining = true
 // CHECK-NEXT: c++-temp-dtor-inlining = true
 // CHECK-NEXT: c++-template-inlining = true

 // CHECK-NEXT: suppress-inlined-defensive-checks = true
 // CHECK-NEXT: suppress-null-return-paths = true
 // CHECK-NEXT: track-conditions = false
 // CHECK-NEXT: track-conditions-debug = false
 // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 88
+// CHECK-NEXT: num-entries = 89

cfe/trunk/test/Analysis/taint-generic.c

-// RUN: %clang_analyze_cc1  -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -Wno-format-security -verify %s
-// RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -Wno-format-security -verify %s
+// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
+// RUN:   -analyzer-checker=alpha.security.taint \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.security.ArrayBoundV2 \
+// RUN:   -analyzer-config \
+// RUN:     alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
+
+// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
+// RUN:   -DFILE_IS_STRUCT \
+// RUN:   -analyzer-checker=alpha.security.taint \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.security.ArrayBoundV2 \
+// RUN:   -analyzer-config \
+// RUN:     alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=alpha.security.taint \
+// RUN:   -analyzer-config \
+// RUN:     alpha.security.taint.TaintPropagation:Config=justguessit \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-FILE
+
+// CHECK-INVALID-FILE: (frontend): invalid input for checker option
+// CHECK-INVALID-FILE-SAME:        'alpha.security.taint.TaintPropagation:Config',
+// CHECK-INVALID-FILE-SAME:        that expects a valid filename instead of
+// CHECK-INVALID-FILE-SAME:        'justguessit'
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=alpha.security.taint \
+// RUN:   -analyzer-config \
+// RUN:     alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config-ill-formed.yaml \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-ILL-FORMED
+
+// CHECK-ILL-FORMED: (frontend): invalid input for checker option
+// CHECK-ILL-FORMED-SAME:        'alpha.security.taint.TaintPropagation:Config',
+// CHECK-ILL-FORMED-SAME:        that expects a valid yaml file: Invalid argument
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=alpha.security.taint \
+// RUN:   -analyzer-config \
+// RUN:     alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config-invalid-arg.yaml \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-INVALID-ARG
+
+// CHECK-INVALID-ARG: (frontend): invalid input for checker option
+// CHECK-INVALID-ARG-SAME:        'alpha.security.taint.TaintPropagation:Config',
+// CHECK-INVALID-ARG-SAME:        that expects an argument number for propagation
+// CHECK-INVALID-ARG-SAME:        rules greater or equal to -1
 
 int scanf(const char *restrict format, ...);
 char *gets(char *str);
 int getchar(void);
 
 typedef struct _FILE FILE;
 #ifdef FILE_IS_STRUCT
 extern struct _FILE *stdin;