vlad.tsyrklevich updated this revision to Diff 87237.
vlad.tsyrklevich marked 4 inline comments as done.
vlad.tsyrklevich added a comment.
As Artem mentioned, I reworked `getLCVSymbol()` to get the default bindings
directly from the StoreManager which required adding a new method. I also
reverted the tainting logic to add taint more conservatively. I have another
change that will fix the new FIXME test that is added, but it might involved
some refactoring and discussion so I've left it out to make this change as
simple as possible.
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,18 @@
return getBinding(getRegionBindings(S), L, T);
}
+ SVal getDefaultBinding(Store S, nonloc::LazyCompoundVal L) override {
+ if (!L.getRegion())
+ return UnknownVal();
+
+ RegionBindingsRef B = getRegionBindings(S);
+ const MemRegion *MR = L.getRegion()->getBaseRegion();
+ if (Optional<SVal> V = B.getDefaultBinding(MR))
+ 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,22 @@
return false;
}
+SymbolRef GenericTaintChecker::getLCVSymbol(CheckerContext &C,
+ nonloc::LazyCompoundVal &LCV) {
+ StoreManager &StoreMgr = C.getStoreManager();
+ 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 derived symbol indicating only a sub-region is tainted
+ SymbolManager &SM = C.getSymbolManager();
+ return SM.getDerivedSymbol(Sym, LCV.getRegion());
+}
+
SymbolRef GenericTaintChecker::getPointedToSymbol(CheckerContext &C,
const Expr* Arg) {
ProgramStateRef State = C.getState();
@@ -438,6 +458,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
