vlad.tsyrklevich updated this revision to Diff 88493.
vlad.tsyrklevich added a comment.

Remove an unnecessary call to `getBaseRegion()`, remove the logic to create a 
new SymbolDerived as it's currently unused, and add a comment to reflect that 
`getLCVSymbol()` is called in a PostStmt and a default binding should always be 
present.


https://reviews.llvm.org/D28445

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/taint-generic.c

Index: test/Analysis/taint-generic.c
===================================================================
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -169,6 +169,43 @@
   sock = socket(AF_LOCAL, SOCK_STREAM, 0);
   read(sock, buffer, 100);
   execl(buffer, "filename", 0); // no-warning
+
+  sock = socket(AF_INET, SOCK_STREAM, 0);
+  // References to both buffer and &buffer as an argument should taint the argument
+  read(sock, &buffer, 100);
+  execl(buffer, "filename", 0); // expected-warning {{Untrusted data is passed to a system call}}
+}
+
+void testStruct() {
+  struct {
+    char buf[16];
+    int length;
+  } tainted;
+
+  char buffer[16];
+  int sock;
+
+  sock = socket(AF_INET, SOCK_STREAM, 0);
+  read(sock, &tainted, sizeof(tainted));
+  __builtin_memcpy(buffer, tainted.buf, tainted.length); // expected-warning {{Untrusted data is used to specify the buffer size}}
+}
+
+void testStructArray() {
+  struct {
+    char buf[16];
+    struct {
+      int length;
+    } st[1];
+  } tainted;
+
+  char buffer[16];
+  int sock;
+
+  sock = socket(AF_INET, SOCK_STREAM, 0);
+  read(sock, &tainted.buf[0], sizeof(tainted.buf));
+  read(sock, &tainted.st[0], sizeof(tainted.st));
+  // FIXME: tainted.st[0].length should be marked tainted
+  __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // no-warning
 }
 
 int testDivByZero() {
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -494,6 +494,17 @@
     return getBinding(getRegionBindings(S), L, T);
   }
 
+  SVal getDefaultBinding(Store S, nonloc::LazyCompoundVal L) override {
+    if (!L.getRegion())
+      return UnknownVal();
+
+    RegionBindingsRef B = getRegionBindings(S);
+    if (Optional<SVal> V = B.getDefaultBinding(L.getRegion()))
+      return *V;
+
+    return UnknownVal();
+  }
+
   SVal getBinding(RegionBindingsConstRef B, Loc L, QualType T = QualType());
 
   SVal getBindingForElement(RegionBindingsConstRef B, const ElementRegion *R);
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -65,6 +65,10 @@
   /// and thus, is tainted.
   static bool isStdin(const Expr *E, CheckerContext &C);
 
+  /// \brief Get the symbol for the region underlying a LazyCompoundVal.
+  static SymbolRef getLCVSymbol(CheckerContext &C,
+                                nonloc::LazyCompoundVal &LCV);
+
   /// \brief Given a pointer argument, get the symbol of the value it contains
   /// (points to).
   static SymbolRef getPointedToSymbol(CheckerContext &C, const Expr *Arg);
@@ -423,6 +427,25 @@
   return false;
 }
 
+SymbolRef GenericTaintChecker::getLCVSymbol(CheckerContext &C,
+                                            nonloc::LazyCompoundVal &LCV) {
+  StoreManager &StoreMgr = C.getStoreManager();
+
+  // getLCVSymbol() is reached in a PostStmt so we can always expect a default
+  // binding to exist if one is present.
+  SymbolRef Sym = StoreMgr.getDefaultBinding(LCV.getStore(), LCV).getAsSymbol();
+  if (!Sym)
+    return nullptr;
+
+  // If the LCV covers an entire base region return the default conjured symbol.
+  if (LCV.getRegion() == LCV.getRegion()->getBaseRegion())
+    return Sym;
+
+  // Otherwise, return a nullptr as there's not yet a functional way to taint
+  // sub-regions of LCVs.
+  return nullptr;
+}
+
 SymbolRef GenericTaintChecker::getPointedToSymbol(CheckerContext &C,
                                                   const Expr* Arg) {
   ProgramStateRef State = C.getState();
@@ -438,6 +461,10 @@
     dyn_cast<PointerType>(Arg->getType().getCanonicalType().getTypePtr());
   SVal Val = State->getSVal(*AddrLoc,
                             ArgTy ? ArgTy->getPointeeType(): QualType());
+
+  if (auto LCV = Val.getAs<nonloc::LazyCompoundVal>())
+    return getLCVSymbol(C, *LCV);
+
   return Val.getAsSymbol();
 }
 
Index: include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -59,6 +59,12 @@
   /// \return The value bound to the location \c loc.
   virtual SVal getBinding(Store store, Loc loc, QualType T = QualType()) = 0;
 
+  /// Return the default value bound to a LazyCompoundVal in a given state.
+  /// \param[in] store The analysis state.
+  /// \param[in] lcv The lazy compound value.
+  /// \return The default value bound to the LazyCompoundVal \c lcv.
+  virtual SVal getDefaultBinding(Store store, nonloc::LazyCompoundVal lcv) = 0;
+
   /// Return a state with the specified value bound to the given location.
   /// \param[in] store The analysis state.
   /// \param[in] loc The symbolic memory location.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to