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

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 15 21:42:44 PDT 2020


NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:289
+  NameRuleMap CustomPropagations{
+      {"c_str", {"std::__cxx11::basic_string", {{0}, {ReturnValueIndex}}}},
+      {"data", {"std::__cxx11::basic_string", {{0}, {ReturnValueIndex}}}},
----------------
Szelethus wrote:
> 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.
So `0` stands for `this`? Can we have a named constant please? ^.^


================
Comment at: clang/lib/StaticAnalyzer/Checkers/Taint.cpp:155-162
+  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);
+    }
----------------
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.


================
Comment at: clang/test/Analysis/taint-generic.cpp:129
+// Test I/O
+namespace std {
+template <class CharT>
----------------
Let's add a header simulator for this if we didn't already.


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

https://reviews.llvm.org/D71524





More information about the cfe-commits mailing list