[analyzer] Support tainted objects in GenericTaintChecker

[boga95]

Summary

I extended the supported C++ features:

• The this pointer can be tainted (0. argument)
• All std::basic_istream objects are tainted unconditionally (std::cin, std::ifstream, etc.)
• std::getline and some member function of std::string propagates taint
• Extraction operator and copy assignment propagate taint

[boga95]

Ping

[Szelethus]

I strongly agree with this comment: D70878#1780513, maybe the placement of functions like getArg and getNumArgs would be most appropriate in CallDescription. How about we try to cut down on duplicating functionalities?

I realize that many, if not all of my comments should've been placed at D70878, I was unfortunately not available then, but I think it would be a lot better if we settled on this before making the eventual changes too hard to switch. Checkers that were written with CallDescriptionMap (D70818, D63915) or have recently been converted to it (D67706, D62557, D68165) are a joy to look at, and reduce the overall complexity of the codebase greatly. Are there strong arguments against it?

The overall direction is great, as demonstrated by the test files. Its very exciting to see taintness analysis improving this much lately!

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

105–135

I know this isn't really relevant, but isn't this redundant with CallDescription?

142

Extraction operator? Is that a thing?

204

http://llvm.org/docs/ProgrammersManual.html#other-map-like-container-options

We never use hash_set and unordered_set because they are generally very expensive (each insertion requires a malloc) and very non-portable.

369

I would be shocked if this is the first time I've come across very similar function -- in any case, could you rename it to something like getArgIgnoringImplicitThis?

809

In this case, maybe llvm::Regex is overkill? [[https://llvm.org/doxygen/classllvm_1_1StringRef.html#a82369bea2700347f68e1f43e30d2d47b | llvm::StringRef::find()]] may be sufficient.

[xazax.hun]

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

270–271

If we consider Stdin and Stdstream to be tainted does it make sense to fold them into isTainted so we never miss checking for them?

[Szelethus]

A portion of my concerns are answered by this patch: D72035.

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

105–135

Ah, okay, so CallDescription doesn't really have the FunctionDecl, but this still feels like a duplication of functionalities.

[Szelethus]

This patch is really cool, but I still feel anxious a bit about duplicating so much functionality, especially since we're working very hard to make CallEvent a widespread thing. @steakhal's patch D72035 already made some great progress in this direction, and should land before this one. @NoQ, do you agree that we should maybe stop for a bit before making the code a bit more consistent with the rest of the checkers?

I added @steakhal as reviewer if you don't mind.

[boga95]

@steakhal's revision is on the top of this. Changing the order will only cause unnecessary work on both sides.

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

105–135

We have already thought about that and it is on our TODO list.

142

I can call it operator>> if you think that is better.

204

I didn't find any multimap among the alternatives. I think the performance won't be an issue here, because the elements are inserted at the beginning of the analysis if there is any. Otherwise, we are planning to replace it with CallDescriptionMap.

270–271

Then we have to pass the CheckerContext to the isTainted functions. I think this function wraps it well.

[Szelethus]

In D71524#1889566, @boga95 wrote:

@steakhal's revision is on the top of this. Changing the order will only cause unnecessary work on both sides.

He recently rebased on top of master. I'm no fan of creating unnecessary work for either of you, and it might just be the case that my worry is greater then necessary. But for now, landing more patches with this infrastructure and having to painstakingly redo them all sounds like more work.

It'd be best to wait for @NoQ's feedback, after all, he did approve earlier patches. And again, sorry for not being available earlier.

[balazske]

clang/test/Analysis/taint-generic.cpp

191

These std declarations are at a better place in system-header-simulator-cxx.h or a similar file.

[steakhal]

In D71524#1889566, @boga95 wrote:

@steakhal's revision is on the top of this. Changing the order will only cause unnecessary work on both sides.

I would happily rebase this patch if you want.

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

142

I think extraction operator is the right term for this.
It is used in the standard: http://eel.is/c++draft/input.streams#istream.extractors

clang/test/Analysis/taint-generic.cpp

191

In the current form, it seems to be a bit verbose.
Why don't we create a minimal std::string which does not inherit from anything and implements the features and behavior only what is necessary.

After minimizing this class there would be no benefit moving to the system-header-simulator-cxx.h header.

[steakhal]

In D71524#1902654, @steakhal wrote:

In D71524#1889566, @boga95 wrote:

@steakhal's revision is on the top of this. Changing the order will only cause unnecessary work on both sides.

I would happily rebase this patch if you want.

Here is the rebased version of your patch.
https://github.com/steakhal/llvm-project/compare/use-callevent...steakhal:boga-rebased-tainted-objects-D71524

Minor modifications applied:

• clang-formatted every modified lines
• simplified the getArgWithImplicitThis function body
• simplified the getNumArgsWithImplicitThis function body
• omitting the spelling of the TaintPropagationRule type name where obvious

[boga95]

Rebase to master.

[Szelethus]

So, as far as I understand, your thinking here is that CXXOperatorCallExprs (which are global, non-member operators) when they are >>, they will propagate taintedness from the 0th argument to the 1st and the return value, correct? So, if I do this:

struct Panda {};

// Turn integers into pandas!
bool operator>>(int i, Panda p) {
  return printf("Panda integer party!");
}

// Turn characters into pandas!
bool operator>>(char i, Panda p) {
  return printf("Panda character party!");
}

int main() {
  Panda p1, p2;
  bool success1 = getTaintedInt() >> p1;
  bool success2 = getTaintedChar() >> p2;
  // success1, success2, p1, p2 are all tainted
}

Are we sure this is what we want? If this is a heuristic, we should document it well, and even then I'm not sure whether we want it. I'm also pretty sure this would make the eventual change to CallDescriptionMap more difficult, because the way taintedness is propagated around std::basic_istream not really the property of the global >> operator and what its parameters are, but rather the property of std::basic_istream<CharT,Traits>::operator>>, right? What do you think?

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

290–294

Hmm, is this the appropriate place to put these? It seems like this job is handled in getTaintPropagationRule. I thought CustomPropagations are reserved for the config file.

582

As far as I know, Call.getOriginExpr() may be null, we should probably use dyn_cast_or_null.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

290

So 0 stands for this? Can we have a named constant please? ^.^

clang/lib/StaticAnalyzer/Checkers/Taint.cpp

155–162

Aha, ok, so this is your plan: only ever deal with fully conjured objects.

I suspect that it'll fail when the function that produces the object is fully inlined. It's probably unlikely to happen with things like std::cin but it's pretty likely to happen with custom taint sources defined by the user. Once you want to fix this, you'll probably have to iterate through all direct bindings in the structure and mark them tainted as well.

clang/test/Analysis/taint-generic.cpp

131

Let's add a header simulator for this if we didn't already.

[Szelethus]

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

142

Well, I consider myself proven wrong :)

[steakhal]

In D71524#1917251, @Szelethus wrote:

Are we sure this is what we want? If this is a heuristic, we should document it well, and even then I'm not sure whether we want it. I'm also pretty sure this would make the eventual change to CallDescriptionMap more difficult, because the way taintedness is propagated around std::basic_istream not really the property of the global >> operator and what its parameters are, but rather the property of std::basic_istream<CharT,Traits>::operator>>, right? What do you think?

I think CallDescription can only identify objects/functions which has IdefntifyerInfo in them. AFAIK operators don't have such. Though somehow AST matchers of Clang Tidy were triggered with this: functionDecl(hasName("operator>>"))
I'm afraid it needs to be a different patch to replace with CallDescriptionMap, even though I agree with you.

[Szelethus]

In D71524#1924508, @steakhal wrote:

I think CallDescription can only identify objects/functions which has IdefntifyerInfo in them. AFAIK operators don't have such. Though somehow AST matchers of Clang Tidy were triggered with this: functionDecl(hasName("operator>>"))

Yup, we could make improvements on CallDescription as well :)

I'm afraid it needs to be a different patch to replace with CallDescriptionMap, even though I agree with you.

I don't mean to deploy the map in this patch, but if this lands as-is, the proposed logic will have to be revisited again.

[boga95]

Ping

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

290–294

We are planning to move all of the propagation rules into a configuration file.

[Szelethus]

I figured you're still working on this, sorry! I'd really like to chat about my earlier comment D71524#1917251, as it kind of challenges the high level idea.

[steakhal]

In D71524#2284386, @Szelethus wrote:

I figured you're still working on this, sorry! I'd really like to chat about my earlier comment D71524#1917251, as it kind of challenges the high level idea.

What about marking the std::cin object itself as tainted and any object created by ifstream::ifstream(const char*) or similar functions.
Then propagate taint via the extraction operator (operator>>) only if the stream was tainted.
This way we could reduce the false-positives of this crude heuristic. What do you think?

[boga95]

In D71524#2291925, @steakhal wrote:

In D71524#2284386, @Szelethus wrote:

I figured you're still working on this, sorry! I'd really like to chat about my earlier comment D71524#1917251, as it kind of challenges the high level idea.

What about marking the std::cin object itself as tainted and any object created by ifstream::ifstream(const char*) or similar functions.
Then propagate taint via the extraction operator (operator>>) only if the stream was tainted.
This way we could reduce the false-positives of this crude heuristic. What do you think?

As far as I remember I tried to make std::cin tainted, but it was complicated. I run the checker against many projects and there wasn't any false positive related to this heuristic.
We can restrict the operator>> to std::basic_stream and cover only the standard library. I think most of the programmers will use this in a conventional way, therefore it should work for their implementation too.

[steakhal]

In D71524#2304742, @boga95 wrote:

As far as I remember I tried to make std::cin tainted, but it was complicated.

What sort of issues did you find by implementing that approach? Could you elaborate?

I run the checker against many projects and there wasn't any false positive related to this heuristic.

It seems a bit hand-wavy to me.

We can restrict the operator>> to std::basic_stream and cover only the standard library.

If we choose this approach, then we have to do something about this, yes. This could be a reasonable choice.

We are arguing about whether the operator>> is a propagation or a source function.
IMO this operator should propagate taint, from the stream to an object.

I've opened the cppreference to find an example where your logic would introduce serious false positives.
I've spent like 2 minutes to come up with this, there might be many more:

std::istringstream is("hello, world");
char arr[10];
is >> std::setw(6) >> arr;
std::cout << "Input from \"" << is.str() << "\" with setw(6) gave \""
          << arr << "\"\n";

godbolt

You would deliberately mark arr as a tainted.
Fortunately/unfortunately it's quite hard to remove taint - so any unnecessarily tainted value is a serious problem, thus requires careful design decisions.

This example might seem to be a made-up one, however, we should consider developers who want to extract data from streams - which aren't tainted.
Marking the stream itself tainted, and propagating taint over the extraction operator would not have such false positives.

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

public:
   /// Convert SignedArgVector to ArgVector.
   ArgVector convertToArgVector(CheckerManager &Mgr, const std::string &Option,
                                const SignedArgVector &Args);
 
   /// Parse the config.
   void parseConfiguration(CheckerManager &Mgr, const std::string &Option,
                           TaintConfiguration &&Config);
 
+  static const unsigned ImplicitArgIndex{0};
   static const unsigned InvalidArgIndex{std::numeric_limits<unsigned>::max()};
   /// Denotes the return vale.
   static const unsigned ReturnValueIndex{std::numeric_limits<unsigned>::max() -
                                          1};
 
 private:
   mutable std::unique_ptr<BugType> BT;
   void initBugType() const {
     if (!BT)
       BT = std::make_unique<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 CallEvent &Call,
                                          const CheckerContext &C) {
       assert(Call.getDecl());
       const FunctionDecl *FDecl = Call.getDecl()->getAsFunction();
       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 CallEvent &Call, const FunctionData &FData,
                 CheckerContext &C) const;
 
+  /// Add taint sources for extraction operator on pre-visit.
+  static bool addOverloadedOpPre(const CallEvent &Call, CheckerContext &C);
+
   /// Add taint sources on a pre-visit. Returns true on matching.
   bool addSourcesPre(const CallEvent &Call, const FunctionData &FData,
                      CheckerContext &C) const;
 
   /// Mark filter's arguments not tainted on a pre-visit. Returns true on
   /// matching.
   bool addFiltersPre(const CallEvent &Call, const FunctionData &FData,
                      CheckerContext &C) const;
 
   /// Propagate taint generated at pre-visit. Returns true on matching.
   static bool propagateFromPre(const CallEvent &Call, CheckerContext &C);
 
   /// Check if the region the expression evaluates to is the standard input,
   /// and thus, is tainted.
   static bool isStdin(const Expr *E, CheckerContext &C);
 
+  /// Check if the type of the variable is std::basic_istream
+  static bool isStdstream(const Expr *E, CheckerContext &C);
+
   /// Given a pointer argument, return the value it points to.
   static Optional<SVal> getPointeeOf(CheckerContext &C, const Expr *Arg);
 
   /// Check for CWE-134: Uncontrolled Format String.
   static constexpr llvm::StringLiteral MsgUncontrolledFormatString =
       "Untrusted data is used as a format string "
       "(CWE-134: Uncontrolled Format String)";
   bool checkUncontrolledFormatString(const CallEvent &Call,
private:
 
   /// Generate a report if the expression is tainted or points to tainted data.
   bool generateReportIfTainted(const Expr *E, StringRef Msg,
                                CheckerContext &C) const;
 
   struct TaintPropagationRule;
   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);
bool isNull() const {
       return SrcArgs.empty() && DstArgs.empty() &&
              VariadicType::None == VarType;
     }
 
     bool isDestinationArgument(unsigned ArgNum) const {
       return (llvm::find(DstArgs, ArgNum) != DstArgs.end());
     }
 
-    static bool isTaintedOrPointsToTainted(const Expr *E,
-                                           const ProgramStateRef &State,
+    static bool isTaintedOrPointsToTainted(const Expr *E, ProgramStateRef State,
                                            CheckerContext &C) {
-      if (isTainted(State, E, C.getLocationContext()) || isStdin(E, C))
+      if (isTainted(State, E, C.getLocationContext()) || isStdin(E, C) ||
+          isStdstream(E, C))
         return true;
 
-      if (!E->getType().getTypePtr()->isPointerType())
-        return false;
-
       Optional<SVal> V = getPointeeOf(C, E);
       return (V && isTainted(State, *V));
     }
 
     /// Pre-process a function which propagates taint according to the
     /// taint rule.
     ProgramStateRef process(const CallEvent &Call, CheckerContext &C) const;
 
     // Functions for custom taintedness propagation.
     static bool postSocket(bool IsTainted, const CallEvent &Call,
                            CheckerContext &C);
   };
 
   /// Defines a map between the propagation function's name, scope
   /// and TaintPropagationRule.
-  NameRuleMap CustomPropagations;
+  NameRuleMap CustomPropagations{
+      {"c_str",
+       {"std::basic_string", {{ImplicitArgIndex}, {ReturnValueIndex}}}},
+      {"data", {"std::basic_string", {{ImplicitArgIndex}, {ReturnValueIndex}}}},
+      {"size", {"std::basic_string", {{ImplicitArgIndex}, {ReturnValueIndex}}}},
+      {"length",
+       {"std::basic_string", {{ImplicitArgIndex}, {ReturnValueIndex}}}},
+      {"getline", {"std::", {{0}, {1, ReturnValueIndex}}}}};
 
   /// Defines a map between the filter function's name, scope and filtering
   /// args.
   NameArgMap CustomFilters;
 
   /// Defines a map between the sink function's name, scope and sinking args.
   NameArgMap CustomSinks;
 };

 } // 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)
 
+/// Treat implicit this parameter as the 0. argument.
+const Expr *getArgWithImplicitThis(const CallEvent &Call, unsigned ArgNum) {
+  if (const auto *InstanceCall = dyn_cast<CXXInstanceCall>(&Call))
+    return ArgNum == 0 ? InstanceCall->getCXXThisExpr()
+                       : InstanceCall->getArgExpr(ArgNum - 1);
+  return Call.getArgExpr(ArgNum);
+}
+
+/// Class member functions has an implicit this parameter.
+unsigned getNumArgsWithImplicitThis(const CallEvent &Call) {
+  return Call.getNumArgs() + static_cast<unsigned>(isa<CXXInstanceCall>(Call));
+}
+
 GenericTaintChecker::ArgVector
 GenericTaintChecker::convertToArgVector(CheckerManager &Mgr,
                                         const std::string &Option,
                                         const SignedArgVector &Args) {
   ArgVector Result;
   for (int Arg : Args) {
     if (Arg == -1)
       Result.push_back(ReturnValueIndex);
GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
   auto It = findFunctionInConfig(CustomPropagations, FData);
   if (It != CustomPropagations.end())
     return It->second.second;
   return {};
 }
 
 void GenericTaintChecker::checkPreCall(const CallEvent &Call,
                                        CheckerContext &C) const {
+  if (addOverloadedOpPre(Call, C))
+    return;
+
   Optional<FunctionData> FData = FunctionData::create(Call, C);
   if (!FData)
     return;
 
   // Check for taintedness related errors first: system call, uncontrolled
   // format string, tainted buffer size.
   if (checkPre(Call, *FData, C))
     return;
void GenericTaintChecker::checkPostCall(const CallEvent &Call,
   propagateFromPre(Call, C);
 }
 
 void GenericTaintChecker::printState(raw_ostream &Out, ProgramStateRef State,
                                      const char *NL, const char *Sep) const {
   printTaint(State, Out, NL, Sep);
 }
 
+bool GenericTaintChecker::addOverloadedOpPre(const CallEvent &Call,
+                                             CheckerContext &C) {
+  if (const auto *OCE =
+          dyn_cast_or_null<CXXOperatorCallExpr>(Call.getOriginExpr())) {
+    TaintPropagationRule Rule;
+    switch (OCE->getOperator()) {
+    case OO_GreaterGreater:
+      Rule = {{ImplicitArgIndex}, {1, ReturnValueIndex}};
+      break;
+    case OO_Equal:
+      Rule = {{1}, {ImplicitArgIndex}};
+      break;
+    default:
+      return false;
+    }
+
+    ProgramStateRef State = Rule.process(Call, C);
+    if (State) {
+      C.addTransition(State);
+      return true;
+    }
+  }
+  return false;
+}
+
 bool GenericTaintChecker::addSourcesPre(const CallEvent &Call,
                                         const FunctionData &FData,
                                         CheckerContext &C) const {
   // First, try generating a propagation rule for this function.
   TaintPropagationRule Rule = TaintPropagationRule::getTaintPropagationRule(
       this->CustomPropagations, FData, C);
   if (!Rule.isNull()) {
     ProgramStateRef State = Rule.process(Call, C);
bool GenericTaintChecker::addFiltersPre(const CallEvent &Call,
   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 >= Call.getNumArgs())
+    if (ArgNum >= getNumArgsWithImplicitThis(Call))
       continue;
 
-    const Expr *Arg = Call.getArgExpr(ArgNum);
+    const Expr *Arg = getArgWithImplicitThis(Call, ArgNum);
     Optional<SVal> V = getPointeeOf(C, Arg);
     if (V)
       State = removeTaint(State, *V);
   }
 
   if (State != C.getState()) {
     C.addTransition(State);
     return true;
for (unsigned ArgNum : TaintArgs) {
     // Special handling for the tainted return value.
     if (ArgNum == ReturnValueIndex) {
       State = addTaint(State, Call.getReturnValue());
       continue;
     }
 
     // The arguments are pointer arguments. The data they are pointing at is
     // tainted after the call.
-    if (Call.getNumArgs() < (ArgNum + 1))
+    if (getNumArgsWithImplicitThis(Call) < (ArgNum + 1))
       return false;
-    const Expr *Arg = Call.getArgExpr(ArgNum);
+    const Expr *Arg = getArgWithImplicitThis(Call, ArgNum);
     Optional<SVal> V = getPointeeOf(C, Arg);
     if (V)
       State = addTaint(State, *V);
   }
 
   // Clear up the taint info from the state.
   State = State->remove<TaintArgsOnPostVisit>();
 

 ProgramStateRef
 GenericTaintChecker::TaintPropagationRule::process(const CallEvent &Call,
                                                    CheckerContext &C) const {
   ProgramStateRef State = C.getState();
 
   // Check for taint in arguments.
   bool IsTainted = true;
   for (unsigned ArgNum : SrcArgs) {
-    if (ArgNum >= Call.getNumArgs())
+    if (ArgNum >= getNumArgsWithImplicitThis(Call))
       continue;
 
-    if ((IsTainted =
-             isTaintedOrPointsToTainted(Call.getArgExpr(ArgNum), State, C)))
+    if ((IsTainted = isTaintedOrPointsToTainted(
+             getArgWithImplicitThis(Call, 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 i = VariadicIndex; i < Call.getNumArgs(); ++i) {
-      if ((IsTainted =
-               isTaintedOrPointsToTainted(Call.getArgExpr(i), State, C)))
+    for (unsigned i = VariadicIndex; i < getNumArgsWithImplicitThis(Call);
+         ++i) {
+      if ((IsTainted = isTaintedOrPointsToTainted(
+               getArgWithImplicitThis(Call, i), State, C)))
         break;
     }
   }
 
   if (PropagationFunc)
     IsTainted = PropagationFunc(IsTainted, Call, C);
 
   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);
       continue;
     }
 
-    if (ArgNum >= Call.getNumArgs())
+    if (ArgNum >= getNumArgsWithImplicitThis(Call))
       continue;
 
     // Mark the given argument.
     State = State->add<TaintArgsOnPostVisit>(ArgNum);
   }
 
   // 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 < Call.getNumArgs(); ++i) {
-      const Expr *Arg = Call.getArgExpr(i);
+    for (unsigned i = VariadicIndex; i < getNumArgsWithImplicitThis(Call);
+         ++i) {
+      const Expr *Arg = getArgWithImplicitThis(Call, 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);
       }
     }
   }
 
   return State;
 }
 
 // If argument 0(protocol domain) is network, the return value should get taint.
 bool GenericTaintChecker::TaintPropagationRule::postSocket(
     bool /*IsTainted*/, const CallEvent &Call, CheckerContext &C) {
-  SourceLocation DomLoc = Call.getArgExpr(0)->getExprLoc();
+  SourceLocation DomLoc = getArgWithImplicitThis(Call, 0)->getExprLoc();
   StringRef DomName = C.getMacroNameOrSpelling(DomLoc);
   // White list the internal communication protocols.
   if (DomName.equals("AF_SYSTEM") || DomName.equals("AF_LOCAL") ||
       DomName.equals("AF_UNIX") || DomName.equals("AF_RESERVED_36"))
     return false;
   return true;
 }
 
+bool GenericTaintChecker::isStdstream(const Expr *E, CheckerContext &C) {
+  const std::string CT = E->getType().getCanonicalType().getAsString();
+  return StringRef(CT).find("std::basic_istream") != StringRef::npos;
+}
+
 bool GenericTaintChecker::isStdin(const Expr *E, CheckerContext &C) {
   ProgramStateRef State = C.getState();
   SVal Val = C.getSVal(E);
 
   // stdin is a pointer, so it would be a region.
   const MemRegion *MemReg = Val.getAsRegion();
 
   // The region should be symbolic, we do not know it's value.
static bool getPrintfFormatArgumentNum(const CallEvent &Call,
   // Handles: fprintf, printf, sprintf, snprintf, vfprintf, vprintf, vsprintf,
   // vsnprintf, syslog, custom annotated functions.
   const FunctionDecl *FDecl = Call.getDecl()->getAsFunction();
   if (!FDecl)
     return false;
   for (const auto *Format : FDecl->specific_attrs<FormatAttr>()) {
     ArgNum = Format->getFormatIdx() - 1;
     if ((Format->getType()->getName() == "printf") &&
-        Call.getNumArgs() > ArgNum)
+        getNumArgsWithImplicitThis(Call) > ArgNum)
       return true;
   }
 
   // Or if a function is named setproctitle (this is a heuristic).
   if (C.getCalleeName(FDecl).find("setproctitle") != StringRef::npos) {
     ArgNum = 0;
     return true;
   }
bool GenericTaintChecker::checkUncontrolledFormatString(
     const CallEvent &Call, CheckerContext &C) const {
   // Check if the function contains a format string argument.
   unsigned ArgNum = 0;
   if (!getPrintfFormatArgumentNum(Call, C, ArgNum))
     return false;
 
   // If either the format string content or the pointer itself are tainted,
   // warn.
-  return generateReportIfTainted(Call.getArgExpr(ArgNum),
+  return generateReportIfTainted(getArgWithImplicitThis(Call, ArgNum),
                                  MsgUncontrolledFormatString, C);
 }
 
 bool GenericTaintChecker::checkSystemCall(const CallEvent &Call, StringRef Name,
                                           CheckerContext &C) const {
   // TODO: It might make sense to run this check on demand. In some cases,
   // we should check if the environment has been cleansed here. We also might
   // need to know if the user was reset before these calls(seteuid).
   unsigned ArgNum = llvm::StringSwitch<unsigned>(Name)
                         .Case("system", 0)
                         .Case("popen", 0)
                         .Case("execl", 0)
                         .Case("execle", 0)
                         .Case("execlp", 0)
                         .Case("execv", 0)
                         .Case("execvp", 0)
                         .Case("execvP", 0)
                         .Case("execve", 0)
                         .Case("dlopen", 0)
                         .Default(InvalidArgIndex);
 
-  if (ArgNum == InvalidArgIndex || Call.getNumArgs() < (ArgNum + 1))
+  if (ArgNum == InvalidArgIndex ||
+      getNumArgsWithImplicitThis(Call) < (ArgNum + 1))
     return false;
 
-  return generateReportIfTainted(Call.getArgExpr(ArgNum), MsgSanitizeSystemArgs,
-                                 C);
+  return generateReportIfTainted(getArgWithImplicitThis(Call, ArgNum),
+                                 MsgSanitizeSystemArgs, C);
 }
 
 // TODO: Should this check be a part of the CString checker?
 // If yes, should taint be a global setting?
 bool GenericTaintChecker::checkTaintedBufferSize(const CallEvent &Call,
                                                  CheckerContext &C) const {
   const auto *FDecl = Call.getDecl()->getAsFunction();
   // If the function has a buffer size argument, set ArgNum.
if (ArgNum == InvalidArgIndex) {
     else if (CCtx::isCLibraryFunction(FDecl, "memccpy"))
       ArgNum = 3;
     else if (CCtx::isCLibraryFunction(FDecl, "realloc"))
       ArgNum = 1;
     else if (CCtx::isCLibraryFunction(FDecl, "bcopy"))
       ArgNum = 2;
   }
 
-  return ArgNum != InvalidArgIndex && Call.getNumArgs() > ArgNum &&
-         generateReportIfTainted(Call.getArgExpr(ArgNum), MsgTaintedBufferSize,
-                                 C);
+  return ArgNum != InvalidArgIndex &&
+         getNumArgsWithImplicitThis(Call) > ArgNum &&
+         generateReportIfTainted(getArgWithImplicitThis(Call, ArgNum),
+                                 MsgTaintedBufferSize, C);
 }
 
 bool GenericTaintChecker::checkCustomSinks(const CallEvent &Call,
                                            const FunctionData &FData,
                                            CheckerContext &C) const {
   auto It = findFunctionInConfig(CustomSinks, FData);
   if (It == CustomSinks.end())
     return false;
 
   const auto &Value = It->second;
   const GenericTaintChecker::ArgVector &Args = Value.second;
   for (unsigned ArgNum : Args) {
-    if (ArgNum >= Call.getNumArgs())
+    if (ArgNum >= getNumArgsWithImplicitThis(Call))
       continue;
 
-    if (generateReportIfTainted(Call.getArgExpr(ArgNum), MsgCustomSink, C))
+    if (generateReportIfTainted(getArgWithImplicitThis(Call, ArgNum),
+                                MsgCustomSink, C))
       return true;
   }
 
   return false;
 }
 
 void ento::registerGenericTaintChecker(CheckerManager &Mgr) {
   auto *Checker = Mgr.registerChecker<GenericTaintChecker>();

clang/lib/StaticAnalyzer/Checkers/Taint.cpp

bool taint::isTainted(ProgramStateRef State, const Stmt *S,
   return isTainted(State, val, Kind);
 }
 
 bool taint::isTainted(ProgramStateRef State, SVal V, TaintTagType Kind) {
   if (const SymExpr *Sym = V.getAsSymExpr())
     return isTainted(State, Sym, Kind);
   if (const MemRegion *Reg = V.getAsRegion())
     return isTainted(State, Reg, Kind);
+  if (auto LCV = V.getAs<nonloc::LazyCompoundVal>()) {
+    if (Optional<SVal> binding =
+            State->getStateManager().getStoreManager().getDefaultBinding(
+                *LCV)) {
+      if (SymbolRef Sym = binding->getAsSymbol())
+        return isTainted(State, Sym, Kind);
+    }
+  }
   return false;
 }
 
 bool taint::isTainted(ProgramStateRef State, const MemRegion *Reg,
                       TaintTagType K) {
   if (!Reg)
     return false;
 

clang/test/Analysis/Inputs/system-header-simulator-cxx.h

public:
     basic_string &insert(size_type index, size_type count, CharT ch);
     basic_string &replace(size_type pos, size_type count, const basic_string &str);
     void pop_back();
     void push_back(CharT ch);
     void reserve(size_type new_cap);
     void resize(size_type count);
     void shrink_to_fit();
     void swap(basic_string &other);
+  private:
+    unsigned size;
+    char *ptr;
   };
 
   typedef basic_string<char> string;
   typedef basic_string<wchar_t> wstring;
 #if __cplusplus >= 201103L
   typedef basic_string<char16_t> u16string;
   typedef basic_string<char32_t> u32string;
 #endif
 
+  template <class CharT>
+  class char_traits {};
+
+  class ios_base {};
+
+  template <class CharT, class Traits = char_traits<CharT> >
+  class basic_ios : public ios_base {};
+
+  template <class CharT, class Traits = char_traits<CharT> >
+  class basic_istream : virtual public basic_ios<CharT, Traits> {};
+  template <class CharT, class Traits = char_traits<CharT> >
+  class basic_ostream : virtual public basic_ios<CharT, Traits> {};
+
+  using istream = basic_istream<char>;
+  using ostream = basic_ostream<char>;
+  using wistream = basic_istream<wchar_t>;
+
+  istream &operator>>(istream &is, int &val);
+  wistream &operator>>(wistream &is, int &val);
+
+  extern istream cin;
+  extern istream wcin;
+
+  template <class CharT, class Traits = char_traits<CharT> >
+  class basic_fstream : public basic_istream<CharT, Traits>, public basic_ostream<CharT, Traits> {
+  public:
+    basic_fstream(const char *) {}
+  };
+  template <class CharT, class Traits = char_traits<CharT> >
+  class basic_ifstream : public basic_istream<CharT, Traits> {
+  public:
+    basic_ifstream(const char *) {}
+  };
+
+  using ifstream = basic_ifstream<char>;
+  using fstream = basic_ifstream<char>;
+
+  istream &operator>>(istream &is, string &val);
+  istream &getline(istream &is, string &str);
+
   class exception {
   public:
     exception() throw();
     virtual ~exception() throw();
     virtual const char *what() const throw() {
       return 0;
     }
   };

clang/test/Analysis/diagnostics/explicit-suppression.cpp

class C {
   // The virtual function is to make C not trivially copy assignable so that we call the
   // variant of std::copy() that does not defer to memmove().
   virtual int f();
 };
 
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:699 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:742 {{Called C++ object pointer is null}}
 #endif
 }

clang/test/Analysis/taint-generic.cpp

 // 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
 
+#include "Inputs/system-header-simulator-cxx.h"
+
 #define BUFSIZE 10
 int Buffer[BUFSIZE];
 
 int scanf(const char*, ...);
 int mySource1();
 int mySource3();
 
 bool isOutOfRange2(const 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 }}
 }
+
+// Test I/O
+void testCin() {
+  int x, y;
+  std::cin >> x >> y;
+  Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testWcin() {
+  int x, y;
+  std::wcin >> x >> y;
+  Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void mySink(const std::string &, int, const char *);
+
+void testFstream() {
+  std::string str;
+  std::ifstream file("example.txt");
+  file >> str;
+  mySink(str, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testIfstream() {
+  std::string str;
+  std::fstream file("example.txt");
+  file >> str;
+  mySink(str, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testStdstring() {
+  std::string str1;
+  std::cin >> str1;
+
+  std::string &str2 = str1;
+  mySink(str2, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+
+  const std::string &str3 = str1;
+  mySink(str3, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testTaintedThis() {
+  std::string str;
+  mySink(std::string(), 0, str.c_str()); // no-warning
+
+  std::cin >> str;
+  mySink(std::string(), 0, str.c_str()); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testOverloadedAssignmentOp() {
+  std::string str1, str2;
+  std::cin >> str1;
+  str2 = str1;
+  mySink(str2, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testGetline() {
+  std::string str;
+  std::getline(std::cin, str);
+  mySink(str, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}