[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

So, as far as I understand, your thinking here is that `CXXOperatorCallExpr`s (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?



================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:289-293
+      {"c_str", {"std::__cxx11::basic_string", {{0}, {ReturnValueIndex}}}},
+      {"data", {"std::__cxx11::basic_string", {{0}, {ReturnValueIndex}}}},
+      {"size", {"std::__cxx11::basic_string", {{0}, {ReturnValueIndex}}}},
+      {"length", {"std::__cxx11::basic_string", {{0}, {ReturnValueIndex}}}},
+      {"getline", {"std::", {{0}, {1, ReturnValueIndex}}}}};
----------------
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.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:579
+                                             CheckerContext &C) {
+  const auto *OCE = dyn_cast<CXXOperatorCallExpr>(Call.getOriginExpr());
+  if (OCE) {
----------------
As far as I know, `Call.getOriginExpr()` may be null, we should probably use `dyn_cast_or_null`.




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71524/new/

https://reviews.llvm.org/D71524