[PATCH] D28445: [Analyzer] Extend taint propagation and checking
Vlad Tsyrklevich via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 7 00:53:47 PST 2017
vlad.tsyrklevich created this revision.
vlad.tsyrklevich added reviewers: zaks.anna, cfe-commits.
While extending the GenericTaintChecker for my purposes I'm hitting a case where taint is not propagated where I believe it should be. Currently, the following example will propagate taint correctly:
char buf[16];
taint_source(buf);
taint_sink(buf);
However, the following fails:
char buf[16];
taint_source(&buf);
taint_sink(buf);
In the first example, buf has it's symbol correctly extracted (via `GenericTaintChecker::getPointedToSymbol()`) as a `SymbolDerived{conj_$N{char}, element{buf,0 S64b,char}}`, it's marked as tainted and then the taint check correctly finds it using `ProgramState::isTainted()`.
In the second example, the SVal obtained in `GenericTaintChecker::getPointedToSymbol()` is a LazyCompoundVal so `SVal::getAsSymbol()` returns a NULL SymbolRef, meaning the symbol is not tainted.
This change extends GenericTaintChecker to obtain a SymbolRegionValue from the LCV MemRegion in `getPointedToSymbol()`, and then extends `ProgramState::isTainted()` to correctly match a `SymbolRegionValue{buf}` against a `SymbolDerived{conj_$N{char}, element{buf,0 S64b,char}}`.
I'm not familiar enough with analyzer internals to be confident in whether this is the right approach to fix this bug, so feedback is welcome.
https://reviews.llvm.org/D28445
Files:
lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
lib/StaticAnalyzer/Core/ProgramState.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,11 @@
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}}
}
int testDivByZero() {
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -690,6 +690,15 @@
if (!Reg)
return false;
+ if (const TypedValueRegion *TVR = dyn_cast<TypedValueRegion>(Reg)) {
+ SymbolRef Sym = getSymbolManager().getRegionValueSymbol(TVR);
+
+ // We don't call isTainted(Sym, K) here because it could lead to infinite recursion.
+ const TaintTagType *Tag = get<TaintMap>(Sym);
+ if (Tag && *Tag == K)
+ return true;
+ }
+
// Element region (array element) is tainted if either the base or the offset
// are tainted.
if (const ElementRegion *ER = dyn_cast<ElementRegion>(Reg))
@@ -720,7 +729,8 @@
// If this is a SymbolDerived with a tainted parent, it's also tainted.
if (const SymbolDerived *SD = dyn_cast<SymbolDerived>(*SI))
- Tainted = Tainted || isTainted(SD->getParentSymbol(), Kind);
+ Tainted = Tainted || isTainted(SD->getParentSymbol(), Kind)
+ || isTainted(SD->getOriginRegion(), Kind);
// If memory region is tainted, data is also tainted.
if (const SymbolRegionValue *SRV = dyn_cast<SymbolRegionValue>(*SI))
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -438,6 +438,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 C.getSymbolManager().getRegionValueSymbol(LCV->getRegion());
+
return Val.getAsSymbol();
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D28445.83521.patch
Type: text/x-patch
Size: 2478 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170107/20143dae/attachment.bin>
More information about the cfe-commits
mailing list