vlad.tsyrklevich updated this revision to Diff 91615.
vlad.tsyrklevich added a comment.
Fix a stray assert(), correctly this time..
https://reviews.llvm.org/D30909
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
lib/StaticAnalyzer/Core/ProgramState.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
@@ -204,8 +204,10 @@
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
+ __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // expected-warning {{Untrusted data is used to specify the buffer size}}
+
+ read(sock, &tainted.st, sizeof(tainted.st));
+ __builtin_memcpy(buffer, tainted.buf, tainted.st[0].length); // expected-warning {{Untrusted data is used to specify the buffer size}}
}
int testDivByZero() {
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -496,7 +496,10 @@
Optional<SVal> getDefaultBinding(Store S, const MemRegion *R) override {
RegionBindingsRef B = getRegionBindings(S);
- return B.getDefaultBinding(R);
+ // Default bindings are always applied over a base region so look up the
+ // base region's default binding, otherwise the lookup will fail when R
+ // is at an offset from R->getBaseRegion().
+ return B.getDefaultBinding(R->getBaseRegion());
}
SVal getBinding(RegionBindingsConstRef B, Loc L, QualType T = QualType());
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -671,6 +671,17 @@
ProgramStateRef NewState = set<TaintMap>(Sym, Kind);
assert(NewState);
+
+ if (const SymbolDerived *SD = dyn_cast<SymbolDerived>(Sym)) {
+ TaintedSymRegionsRef SymRegions(0, TSRFactory.getTreeFactory());
+ if (contains<DerivedSymTaint>(SD->getParentSymbol()))
+ SymRegions = *get<DerivedSymTaint>(SD->getParentSymbol());
+
+ SymRegions = SymRegions.add(SD->getRegion());
+ NewState = NewState->set<DerivedSymTaint>(SD->getParentSymbol(), SymRegions);
+ assert(NewState);
+ }
+
return NewState;
}
@@ -723,15 +734,31 @@
const TaintTagType *Tag = get<TaintMap>(*SI);
Tainted = (Tag && *Tag == Kind);
- // If this is a SymbolDerived with a tainted parent, it's also tainted.
- if (const SymbolDerived *SD = dyn_cast<SymbolDerived>(*SI))
+ if (const SymbolDerived *SD = dyn_cast<SymbolDerived>(*SI)) {
+ // If this is a SymbolDerived with a tainted parent, it's also tainted.
Tainted = Tainted || isTainted(SD->getParentSymbol(), Kind);
+ // If this is a SymbolDerived with the same parent symbol as another
+ // tainted SymbolDerived and a region that's a sub-region of that tainted
+ // symbol, it's also tainted.
+ if (contains<DerivedSymTaint>(SD->getParentSymbol())) {
+ const TypedValueRegion *R = SD->getRegion();
+ const TaintedSymRegionsRef *SymRegions =
+ get<DerivedSymTaint>(SD->getParentSymbol());
+ for (TaintedSymRegionsRef::iterator I = SymRegions->begin(),
+ E = SymRegions->end();
+ I != E; ++I) {
+ if (R == *I || R->isSubRegionOf(*I))
+ return true;
+ }
+ }
+ }
+
// If memory region is tainted, data is also tainted.
if (const SymbolRegionValue *SRV = dyn_cast<SymbolRegionValue>(*SI))
Tainted = Tainted || isTainted(SRV->getRegion(), Kind);
- // If If this is a SymbolCast from a tainted value, it's also tainted.
+ // If this is a SymbolCast from a tainted value, it's also tainted.
if (const SymbolCast *SC = dyn_cast<SymbolCast>(*SI))
Tainted = Tainted || isTainted(SC->getOperand(), Kind);
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -72,8 +72,6 @@
/// covers the entire region, e.g. we avoid false positives by not returning
/// a default bindingc for an entire struct if the symbol for only a single
/// field or element within it is requested.
- // TODO: Return an appropriate symbol for sub-fields/elements of an LCV so
- // that they are also appropriately tainted.
static SymbolRef getLCVSymbol(CheckerContext &C,
nonloc::LazyCompoundVal &LCV);
@@ -479,19 +477,21 @@
// getLCVSymbol() is reached in a PostStmt so we can always expect a default
// binding to exist if one is present.
- if (Optional<SVal> binding = StoreMgr.getDefaultBinding(LCV)) {
- SymbolRef Sym = binding->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;
- }
+ Optional<SVal> binding = StoreMgr.getDefaultBinding(LCV);
+ if (!binding)
+ return nullptr;
- // Otherwise, return a nullptr as there's not yet a functional way to taint
- // sub-regions of LCVs.
- return nullptr;
+ SymbolRef Sym = binding->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,
Index: include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
@@ -35,6 +35,16 @@
static void *GDMIndex() { static int index = 0; return &index; }
};
+/// The GDM component mapping derived symbols' parent symbols to their
+/// underlying regions. This is used to efficiently check whether a symbol is
+/// tainted when it represents a sub-region of a tainted symbol.
+struct DerivedSymTaint {};
+typedef llvm::ImmutableMap<SymbolRef, TaintedSymRegionsRef> DerivedSymTaintImpl;
+template<> struct ProgramStateTrait<DerivedSymTaint>
+ : public ProgramStatePartialTrait<DerivedSymTaintImpl> {
+ static void *GDMIndex() { static int index = 0; return &index; }
+};
+
class TaintManager {
TaintManager() {}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -43,6 +43,8 @@
ProgramStateManager &, SubEngine *);
typedef std::unique_ptr<StoreManager>(*StoreManagerCreator)(
ProgramStateManager &);
+typedef llvm::ImmutableSet<const TypedValueRegion*> TaintedSymRegions;
+typedef llvm::ImmutableSetRef<const TypedValueRegion*> TaintedSymRegionsRef;
//===----------------------------------------------------------------------===//
// ProgramStateTrait - Traits used by the Generic Data Map of a ProgramState.
@@ -87,6 +89,7 @@
Store store; // Maps a location to its current value.
GenericDataMap GDM; // Custom data stored by a client of this class.
unsigned refCount;
+ TaintedSymRegions::Factory TSRFactory;
/// makeWithStore - Return a ProgramState with the same values as the current
/// state with the exception of using the specified Store.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
