[analyzer] Add custom filter functions for GenericTaintChecker

[boga95]

Summary

One can pass a configuration file to the checker with the following argument: -analyzer-config alpha.security.taint.TaintPropagation:Config=/path/to/the/file/taint-generic-config.yaml. The config file can contain:

• Propagations: One can define functions which propagate or create the taintedness. It has five fields:
  • Name: The name of the function. Mandatory field.
  • SrcArgs: A list of arguments. If any of them tainted, the destination arguments will be marked tainted. It's not defined, the destination arguments always will be marked as tainted.
  • DstArgs: A list of arguments. Set the tainted flag for the arguments, if they are marked. The return value's index is 4294967294(it is temporary).
  • VarType: It's an enum with three possible values: None, Src, Dst. The default value is None and do nothing.
  • VarIndex: It's the first variadic argument for the function. If VarType == Src and any of them is tainted, the destination arguments will be marked ad tainted. If VarType == Dst and they are marked, all argument from the VarIndex will be marked as tainted.
• Filters: One can define function remove the tainted flag if it is passed to the proper argument.
  • Name: The name of the function. Mandatory field.
  • Args: A list of arguments. If a tainted value is passed to it, the tainted flag will be removed. Mandatory field.
• Sinks: A list of function which will give a warning if it gets a tainted value.
  • Name: The name of the function. Mandatory field.
  • Args: A list of arguments. If any of those arguments get a tainted value, it will give a warning. Mandatory field.

For the propagations, it uses the config to deduce the TaintPropagationRules from the function's name.
The filter functions are understandable as functions which mark their arguments not tainted. I improved the information flow from pre-visit to post-visit, therefore, the TaintTagType could be passed to the setTaint function. Currently, it only works if the argument is a pointer.

[Szelethus]

I'm very much guilty of doing functional and refactoring changes within the same patch, but I think working on GenericTaintChecker AND in the same patch doing (seemingly unrelated) function name changes in ProgramState might be overkill -- Could you please divide this patch into smaller parts please?

[Szelethus]

Same thing.

[boga95]

I add a new taint type, which represents a lack of taintedness. That's why I changed the name of addTaint() to setTaint(). Of course, it's not an important change, I can move it to another patch.

[NoQ]

Hi, i wanted to squeeze in D59861 somewhere in the middle of your work, would you mind?
I'll definitely have a look at your patches soon :)

[boga95]

Rebase after https://reviews.llvm.org/D59861.
Fix custom filter test case: functions without definition always remove taintedness.

[Szelethus]

In general, the patch is looking alright, I'll take a second look later on. Don't mind my inlines too much, they are more directed towards the original code then your changes.

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

53–56

Interesting, isn't this predefined for std::pair?

474–476

When do these happen? For implicit functions in C?

478–480

And this? Lambdas maybe?

560

Please add the full type name, else it isn't obvious why we don't take it as a const reference.

561–562

We don't use std::pair, so we have descriptive field names, do we need these?

lib/StaticAnalyzer/Checkers/Taint.h

25–28

Is there a very good reason behind us not using an enum instead?

[NoQ]

I'd like the test cases to actually demonstrate the correct use of the filters and the correct behavior of the Analyzer when the filters are annotated correctly, but it looks to me that they either demonstrate behavior when the annotation is not used correctly, or we disagree about how the taint should work in the first place. Testing the behavior when the annotation is misplaced is fine (with enough comments and probably FIXMEs where applicable), but "positive" tests are more valuable because they are the actual common cases (hopefully).

clang/test/Analysis/taint-generic.c

381–385 ↗

(On Diff #219981)

myFilter1 will create a new conjured symbol within x when evaluated conservatively. In this case there clearly shouldn't be a warning on Buffer[x] = 1, because nobody marked the new symbol as tainted to begin with. Therefore i think that this test doesn't really test the new functionality.

387–391 ↗

(On Diff #219981)

Assuming myFilter2 is inlined, we have two execution paths here. On one of them x is reset to a concrete 0. Because concrete values cannot carry taint to begin with, this execution path doesn't test the new functionality. On the other path x indeed has the old tainted value but it's now constrained to [0, INT_MAX]. Because it's still not constrained to be less than BUFSIZE, i'd say this looks more like a false negative and the new functionality makes the analysis worse. So this test does test the new functionality, but it kinda makes the new functionality look bad rather than good.

393–397 ↗

(On Diff #219981)

In this example myFilter3 promises not to alter the value of x due to const-qualifier on the pointee type of the parameter. Additionally, the function has no chance of preventing the program from reaching the buffer overflow line, other than crashing the program (given that it's C and there are no exceptions). Therefore i'd say this is also a false negative.

A better motivational example that doesn't immediately look like a false negative may look like this:

void testConfigurationFilter3() {
  int x = mySource1();
  if (isOutOfRange(x)) // the filter function
    return;
  Buffer[x] = 1; // no-warning
}

In this case the function looks at the value of x (there's really no need to pass it by pointer) and notifies the caller through its return value that it is unsafe to access the buffer with such index. This is probably the only situation where a "filter" annotation is actually worth it.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

517 ↗

(On Diff #219981)

Why not simply remove taint?

[boga95]

Ping

clang/test/Analysis/taint-generic.c

393–397 ↗

(On Diff #219981)

It could work with C++ despite it hasn't supported any specific feature (yet).

I pass it by const int* because it currently doesn't support the value semantics :(. It has been already in my TODO list.

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

53–56

Unfortunately, it's not.

474–476

For example, a lambda doesn't have an FunctionDecl.

478–480

I haven't found any example of this. It's just a copy-paste refactoring so I don't know the intentions behind that.

I think the checker should work perfectly without it.

lib/StaticAnalyzer/Checkers/Taint.h

25–28

Unfortunately, enums cannot put into FoldingSet. It has to be a primitive type or a struct with the necessary functions. I can use enums and cast them to unsigned and back, but it isn't convenient.

[boga95]

Ping

[steakhal]

I think this patch is ok.

Although there are remarks:

• I think the current implementation of the taint filtering functions does not follow the expected semantics. Now the modelling would remove taint before calling the function (pre statement). One might expect that the modelling would remove taint only on function return (post statement). This way we would not catch this:

int myFilter(int* p) { // this function is stated as a filter function in the config file
  int lookupTable[32] = {};
  int value = lookupTable[*p]; // using a tainted value for indexing
  escape(p);
  return value;
}

• I agree with @NoQ about the testConfigurationFilter, which now does not test the implementation but the behavior of the static analyzer if it conservatively invalidates in case of an unknown function call.
• I also think that we should have testcases for testing the filtering functionality of the config file. Eg. using the myFilter1 could do the job here in the tests.

All in all, I still say that it seems to be a good patch.

clang/test/Analysis/taint-generic.c

59 ↗

(On Diff #220385)

Have you considered using _Bool instead?

[NoQ]

@NoQ: "Why not simply remove taint?"
@boga95: *removes taint*
@NoQ: "Hmm, now that i think about it, adding a 'no taint' marker might actually work correctly more often."

Like, if you have taint on $x and try to remove taint from derived<$x, x.a>, your current implementation will do nothing. But the approach with adding a 'no taint' marker will actually add a new marker and make subsequent lookups to derived<$x, x.a> will return the newly added marker, which is the correct behavior; additionally, derived<$x, x.b> would remain tainted (where b != a), which is also the correct behavior. It would have still failed when you describe the sub-region slightly differently, but that'd be a fairly minor glitch.

The right way to proceed further with the removeTaint() approach on SymbolDerived is to introduce removePartialTaint() that would complement addPartialTaint(). But that will require changing the data structure in the program state from a simple set of tainted sub-regions to a sophisticated tree of sub-regions that are marked up as tainted or not tainted. Which might have as well been a marker.

I still tend to believe that removeTaint() is the right approach, but it's a bit harder to get right and a bit worse if not enough effort is invested into it.

There definitely is, however, a good use for the removeTaint() function in both approaches: namely, our taint checker still lacks checkDeadSymbols :D

clang/lib/StaticAnalyzer/Checkers/Taint.cpp

111–112 ↗

(On Diff #220385)

That's not entirely true. Like, if you check (char)x, you still have 24 bytes of x controlled by the attacker. But that's a good false-positive-proof approach.

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

474–476

Lambda most certainly has a FunctionDecl, which is the declaration of its operator()(), and that's exactly what's going to be in FDecl if a lambda is invoked. However, the getKind() of this FunctionDecl will not be Decl::Function but Decl::CXXMethod, like of any other member function.

So this second check is checking that the function is a simple global function.

I recommend replacing with !isa<CXXMethodDecl>(FDecl), purely for readability, or at least adding a comment.

[Szelethus]

LGTM, provided that the inlines are addressed! Thanks!

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

102–103 ↗

(On Diff #220385)

I recall that the current thinking is preferring CallEvent, though leave this as-is for now, because @steakhal might already have it locally.

105 ↗

(On Diff #220385)

We might as well explain what the return value here means (and for the other functions too).

[boga95]

I did the required changes and tried to commit it, but I couldn't. I heard the codebase was migrated to GitHub. Maybe it affected my commit access.

[NoQ]

You're now supposed to push directly to github.
You might also have missed http://lists.llvm.org/pipermail/cfe-dev/2019-August/063219.html.

[NoQ]

See also https://llvm.org/docs/DeveloperPolicy.html#current-contributors-transfering-from-svn.

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

 #include <initializer_list>
 #include <utility>
 
 using namespace clang;
 using namespace ento;
 using namespace taint;
 
 namespace {
+/// A struct to store tainted argument and taint type as a pair in the program
+/// state.
+struct TaintArgTypePair {
+  unsigned Arg;
+  TaintTagType TagType;
+
+  bool operator==(const TaintArgTypePair &X) const {
+    return Arg == X.Arg && TagType == X.TagType;
+  }
+
+  bool operator<(const TaintArgTypePair &X) const {
+    if (Arg != X.Arg)
+      return Arg < X.Arg;
+    return TagType < X.TagType;
+  }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const {
+    ID.AddInteger(Arg);
+    ID.AddInteger(TagType);
+  }
+};
+
 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;
 
   using ArgVector = SmallVector<unsigned, 2>;
 
   enum class VariadicType { None, Src, Dst };
 
   /// The ``TaintConfiguration`` is used to parse configuration file.
   struct TaintConfiguration {
     using NameArgsPair = std::pair<std::string, ArgVector>;
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.
-  bool checkPre(const CallExpr *CE, CheckerContext &C) const;
+  bool checkPre(const CallExpr *CE, const FunctionDecl *FDecl, StringRef Name,
+                CheckerContext &C) const;
 
   /// Add taint sources on a pre-visit.
-  void addSourcesPre(const CallExpr *CE, CheckerContext &C) const;
+  bool addSourcesPre(const CallExpr *CE, const FunctionDecl *FDecl,
+                     StringRef Name, CheckerContext &C) const;
+
+  /// Mark filter's arguments not tainted on a pre-visit.
+  bool addFiltersPre(const CallExpr *CE, StringRef Name,
+                     CheckerContext &C) const;
 
   /// Propagate taint generated at pre-visit.
   bool propagateFromPre(const CallExpr *CE, CheckerContext &C) const;
 
   /// Check if the region the expression evaluates to is the standard input,
   /// and thus, is tainted.
   static bool isStdin(const Expr *E, CheckerContext &C);
 

 };
 } // 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)
+REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, TaintArgTypePair)
 
 void GenericTaintChecker::getConfiguration(StringRef ConfigFile) {
   if (ConfigFile.trim().empty())
     return;
 
   llvm::vfs::FileSystem *FS = llvm::vfs::getRealFileSystem().get();
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
       FS->getBufferForFile(ConfigFile.str());
GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
   if (It != CustomPropagations.end())
     return It->getValue();
 
   return TaintPropagationRule();
 }
 
 void GenericTaintChecker::checkPreStmt(const CallExpr *CE,
                                        CheckerContext &C) const {
+  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+  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, C))
+  if (checkPre(CE, FDecl, Name, C))
     return;
 
   // Marks the function's arguments and/or return value tainted if it present in
   // the list.
-  addSourcesPre(CE, C);
+  if (addSourcesPre(CE, FDecl, Name, C))
+    return;
+
+  addFiltersPre(CE, Name, C);
 }
 
 void GenericTaintChecker::checkPostStmt(const CallExpr *CE,
                                         CheckerContext &C) const {
   // Set the marked values as tainted. The return value only accessible from
   // checkPostStmt.
   propagateFromPre(CE, C);
 }
 
 void GenericTaintChecker::printState(raw_ostream &Out, ProgramStateRef State,
                                      const char *NL, const char *Sep) const {
   printTaint(State, Out, NL, Sep);
 }
 
-void GenericTaintChecker::addSourcesPre(const CallExpr *CE,
+bool GenericTaintChecker::addSourcesPre(const CallExpr *CE,
+                                        const FunctionDecl *FDecl,
+                                        StringRef Name,
                                         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(FDecl, Name, C);
   if (!Rule.isNull()) {
-    State = Rule.process(CE, C);
-    if (!State)
-      return;
+    ProgramStateRef State = Rule.process(CE, C);
+    if (State) {
       C.addTransition(State);
-    return;
-  }
+      return true;
+    }
+  }
+  return false;
 
-  if (!State)
-    return;
+bool GenericTaintChecker::addFiltersPre(const CallExpr *CE, StringRef Name,
+                                        CheckerContext &C) const {
+  auto It = CustomFilters.find(Name);
+  if (It == CustomFilters.end())
+    return false;
+
+  ProgramStateRef State = C.getState();
+  const GenericTaintChecker::ArgVector &Args = It->getValue();
+  for (unsigned ArgNum : Args) {
+    if (ArgNum >= CE->getNumArgs()) {
+      llvm::errs() << "Skip out of bound filter Arg: " << Name << ":" << ArgNum
+                   << '\n';
+      continue;
+    }
+
+    State = State->add<TaintArgsOnPostVisit>({ArgNum, TaintTagNotTainted});
+  }
+
+  if (State != C.getState()) {
     C.addTransition(State);
+    return true;
+  }
+  return false;
 }
 
 bool GenericTaintChecker::propagateFromPre(const CallExpr *CE,
                                            CheckerContext &C) const {
   ProgramStateRef State = C.getState();
 
   // Depending on what was tainted at pre-visit, we determined a set of
   // arguments which should be tainted after the function returns. These are
   // stored in the state as TaintArgsOnPostVisit set.
   TaintArgsOnPostVisitTy TaintArgs = State->get<TaintArgsOnPostVisit>();
   if (TaintArgs.isEmpty())
     return false;
 
-  for (unsigned ArgNum : TaintArgs) {
+  for (auto ArgTypePair : TaintArgs) {
+    unsigned ArgNum = ArgTypePair.Arg;
+    TaintTagType TagType = ArgTypePair.TagType;
     // Special handling for the tainted return value.
     if (ArgNum == ReturnValueIndex) {
-      State = addTaint(State, CE, C.getLocationContext());
+      State = addTaint(State, CE, C.getLocationContext(), TagType);
       continue;
     }
 
     // The arguments are pointer arguments. The data they are pointing at is
     // tainted after the call.
     if (CE->getNumArgs() < (ArgNum + 1))
       return false;
     const Expr *Arg = CE->getArg(ArgNum);
     Optional<SVal> V = getPointedToSVal(C, Arg);
     if (V)
-      State = addTaint(State, *V);
+      State = addTaint(State, *V, TagType);
   }
 
   // Clear up the taint info from the state.
   State = State->remove<TaintArgsOnPostVisit>();
 
   if (State != C.getState()) {
     C.addTransition(State);
     return true;
   }
   return false;
 }
 
 bool GenericTaintChecker::checkPre(const CallExpr *CE,
+                                   const FunctionDecl *FDecl, StringRef Name,
                                    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))
     return true;
 
   if (checkTaintedBufferSize(CE, FDecl, C))
     return true;
 
   if (checkCustomSinks(CE, Name, C))
     return true;
GenericTaintChecker::TaintPropagationRule::process(const CallExpr *CE,
 
   if (!IsTainted)
     return State;
 
   // Mark the arguments which should be tainted after the function returns.
   for (unsigned ArgNum : DstArgs) {
     // Should mark the return value?
     if (ArgNum == ReturnValueIndex) {
-      State = State->add<TaintArgsOnPostVisit>(ReturnValueIndex);
+      State =
+          State->add<TaintArgsOnPostVisit>({ReturnValueIndex, TaintTagGeneric});
       continue;
     }
 
     if (ArgNum >= CE->getNumArgs()) {
       StringRef Name = C.getCalleeName(C.getCalleeDecl(CE));
       llvm::errs() << "Skip out of bound DstArg: " << Name << ":" << ArgNum
                    << '\n';
       continue;
     }
 
     // Mark the given argument.
-    State = State->add<TaintArgsOnPostVisit>(ArgNum);
+    State = State->add<TaintArgsOnPostVisit>({ArgNum, TaintTagGeneric});
   }
 
   // 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 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);
+        State = State->add<TaintArgsOnPostVisit>({i, TaintTagGeneric});
     }
   }
 
   return State;
 }
 
 // If argument 0(protocol domain) is network, the return value should get taint.
 bool GenericTaintChecker::TaintPropagationRule::postSocket(bool /*IsTainted*/,

 }
 
 bool GenericTaintChecker::checkCustomSinks(const CallExpr *CE, StringRef Name,
                                            CheckerContext &C) const {
   auto It = CustomSinks.find(Name);
   if (It == CustomSinks.end())
     return false;
 
-  const GenericTaintChecker::ArgVector& Args = It->getValue();
+  const GenericTaintChecker::ArgVector &Args = It->getValue();
   for (unsigned ArgNum : Args) {
     if (ArgNum >= CE->getNumArgs()) {
       llvm::errs() << "Skip out of bound sink Arg: " << Name << ":" << ArgNum
                    << '\n';
       continue;
     }
 
     if (generateReportIfTainted(CE->getArg(ArgNum), MsgCustomSink, C))

lib/StaticAnalyzer/Checkers/Taint.h

 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 
 namespace clang {
 namespace ento {
 namespace taint {
 
 /// The type of taint, which helps to differentiate between different types of
 /// taint.
 using TaintTagType = unsigned;
 
-static constexpr TaintTagType TaintTagGeneric = 0;
+static constexpr TaintTagType TaintTagNotTainted = 0;
+static constexpr TaintTagType TaintTagGeneric = 1;
 
 /// Create a new state in which the value of the statement is marked as tainted.
-LLVM_NODISCARD ProgramStateRef
-addTaint(ProgramStateRef State, const Stmt *S, const LocationContext *LCtx,
+LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State, const Stmt *S,
+                                        const LocationContext *LCtx,
                                         TaintTagType Kind = TaintTagGeneric);
 
 /// Create a new state in which the value is marked as tainted.
-LLVM_NODISCARD ProgramStateRef
-addTaint(ProgramStateRef State, SVal V,
+LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State, SVal V,
                                         TaintTagType Kind = TaintTagGeneric);
 
 /// Create a new state in which the symbol is marked as tainted.
-LLVM_NODISCARD ProgramStateRef
-addTaint(ProgramStateRef State, SymbolRef Sym,
+LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State, SymbolRef Sym,
                                         TaintTagType Kind = TaintTagGeneric);
 
 /// Create a new state in which the pointer represented by the region
 /// is marked as tainted.
-LLVM_NODISCARD ProgramStateRef
-addTaint(ProgramStateRef State, const MemRegion *R,
+LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State,
+                                        const MemRegion *R,
                                         TaintTagType Kind = TaintTagGeneric);
 
 /// Create a new state in a which a sub-region of a given symbol is tainted.
 /// This might be necessary when referring to regions that can not have an
 /// individual symbol, e.g. if they are represented by the default binding of
 /// a LazyCompoundVal.
-LLVM_NODISCARD ProgramStateRef
-addPartialTaint(ProgramStateRef State,
-                SymbolRef ParentSym, const SubRegion *SubRegion,
-                TaintTagType Kind = TaintTagGeneric);
+LLVM_NODISCARD ProgramStateRef addPartialTaint(
+    ProgramStateRef State, SymbolRef ParentSym, const SubRegion *SubRegion,
 
 /// Check if the statement has a tainted value in the given state.
 bool isTainted(ProgramStateRef State, const Stmt *S,
                const LocationContext *LCtx,
                TaintTagType Kind = TaintTagGeneric);
 
 /// Check if the value is tainted in the given state.
 bool isTainted(ProgramStateRef State, SVal V,
std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
                                                  BugReport &BR) override;
 };
 
 } // namespace taint
 } // namespace ento
 } // namespace clang
 
 #endif
-

lib/StaticAnalyzer/Checkers/Taint.cpp

void taint::printTaint(ProgramStateRef State, raw_ostream &Out, const char *NL,
 
   if (!TM.isEmpty())
     Out << "Tainted symbols:" << NL;
 
   for (const auto &I : TM)
     Out << I.first << " : " << I.second << NL;
 }
 
-void dumpTaint(ProgramStateRef State) {
-  printTaint(State, llvm::errs());
-}
+void dumpTaint(ProgramStateRef State) { printTaint(State, llvm::errs()); }
 
 ProgramStateRef taint::addTaint(ProgramStateRef State, const Stmt *S,
                                 const LocationContext *LCtx,
                                 TaintTagType Kind) {
   return addTaint(State, State->getSVal(S, LCtx), Kind);
 }
 
 ProgramStateRef taint::addTaint(ProgramStateRef State, SVal V,
                                 TaintTagType Kind) {
   SymbolRef Sym = V.getAsSymbol();
   if (Sym)
     return addTaint(State, Sym, Kind);
 
   // If the SVal represents a structure, try to mass-taint all values within the
   // structure. For now it only works efficiently on lazy compound values that
   // were conjured during a conservative evaluation of a function - either as
   // return values of functions that return structures or arrays by value, or as
   // values of structures or arrays passed into the function by reference,
   // directly or through pointer aliasing. Such lazy compound values are
   // characterized by having exactly one binding in their captured store within
   // their parent region, which is a conjured symbol default-bound to the base
   // region of the parent region.
   if (auto LCV = V.getAs<nonloc::LazyCompoundVal>()) {
     if (Optional<SVal> binding =
-            State->getStateManager().getStoreManager()
-                                    .getDefaultBinding(*LCV)) {
+            State->getStateManager().getStoreManager().getDefaultBinding(
+                *LCV)) {
       if (SymbolRef Sym = binding->getAsSymbol())
         return addPartialTaint(State, Sym, LCV->getRegion(), Kind);
     }
   }
 
   const MemRegion *R = V.getAsRegion();
   return addTaint(State, R, Kind);
 }

 }
 
 bool taint::isTainted(ProgramStateRef State, SymbolRef Sym, TaintTagType Kind) {
   if (!Sym)
     return false;
 
   // Traverse all the symbols this symbol depends on to see if any are tainted.
   for (SymExpr::symbol_iterator SI = Sym->symbol_begin(),
-                                SE = Sym->symbol_end(); SI != SE; ++SI) {
+                                SE = Sym->symbol_end();
+       SI != SE; ++SI) {
     if (!isa<SymbolData>(*SI))
       continue;
 
     if (const TaintTagType *Tag = State->get<TaintMap>(*SI)) {
-      if (*Tag == Kind)
+      if (*Tag != TaintTagNotTainted && *Tag == Kind)
         return true;
     }
 
     if (const auto *SD = dyn_cast<SymbolDerived>(*SI)) {
       // If this is a SymbolDerived with a tainted parent, it's also tainted.
       if (isTainted(State, SD->getParentSymbol(), Kind))
         return true;
 

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

- Name:     mySnprintf
     SrcArgs:  [1]
     DstArgs:  [0, 4294967294]
     VarType:  Src
     VarIndex: 3
 
 # A list of filter functions
 Filters:
   # int x; // x is tainted
-  # myFilter(&x); // x is not tainted anymore
-  - Name: myFilter
+  # myFilter1(&x); // x is not tainted anymore
+  - Name: myFilter1
     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]
 

test/Analysis/taint-generic.c

void constraintManagerShouldTreatAsOpaque(int rhs) {
   // This comparison used to hit an assertion in the constraint manager,
   // which didn't handle NonLoc sym-sym comparisons.
   if (i < rhs)
     return;
   if (i < rhs)
     *(volatile int *) 0; // no-warning
 }
 
-
 // Test configuration
 int mySource1();
 void mySource2(int*);
 void myScanf(const char*, ...);
 int myPropagator(int, int*);
 int mySnprintf(char*, size_t, const char*, ...);
+void myFilter1(int* x) {}
+void myFilter2(const int*);
+void myFilter3(int);
 void mySink(int, int, int);
 
 void testConfigurationSources1() {
   int x = mySource1();
   Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
 }
 
 void testConfigurationSources2() {

 
 void testConfigurationPropagation() {
   int x = mySource1();
   int y;
   myPropagator(x, &y);
   Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
 }
 
+void testConfigurationFilter1() {
+  int x = mySource1();
+  myFilter1(&x);
+  Buffer[x] = 1; // no-warning
+}
+
+void testConfigurationFilter2() {
+  int x = mySource1();
+  myFilter2(&x); // Pass by const int*
+  Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationFilter3() {
+  int x = mySource1();
+  myFilter3(x); // Pass by value
+  Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
+}
+
 void testConfigurationSinks() {
   int x = mySource1();
   mySink(x, 1, 2); // expected-warning {{Untrusted data is passed to a user defined sink}}
   mySink(1, x, 2); // no-warning
   mySink(1, 2, x); // expected-warning {{Untrusted data is passed to a user defined sink}}
 }