Skip to content

Static analyzer cherrypicks 4 #418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions clang/lib/Analysis/BodyFarm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -741,13 +741,17 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
// First, find the backing ivar.
const ObjCIvarDecl *IVar = nullptr;

// Property accessor stubs sometimes do not correspond to any property.
// Property accessor stubs sometimes do not correspond to any property decl
// in the current interface (but in a superclass). They still have a
// corresponding property impl decl in this case.
if (MD->isSynthesizedAccessorStub()) {
const ObjCInterfaceDecl *IntD = MD->getClassInterface();
const ObjCImplementationDecl *ImpD = IntD->getImplementation();
for (const auto *V: ImpD->ivars()) {
if (V->getName() == MD->getSelector().getNameForSlot(0))
IVar = V;
for (const auto *PI: ImpD->property_impls()) {
if (const ObjCPropertyDecl *P = PI->getPropertyDecl()) {
if (P->getGetterName() == MD->getSelector())
IVar = P->getPropertyIvarDecl();
}
}
}

Expand Down
93 changes: 60 additions & 33 deletions clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,20 @@ class GenericTaintChecker
}

/// Catch taint related bugs. Check if tainted data is passed to a
/// system call etc.
bool checkPre(const CallExpr *CE, CheckerContext &C) const;
/// system call etc. Returns true on matching.
bool checkPre(const CallExpr *CE, const FunctionDecl *FDecl, StringRef Name,
CheckerContext &C) const;

/// Add taint sources on a pre-visit.
void addSourcesPre(const CallExpr *CE, CheckerContext &C) const;
/// Add taint sources on a pre-visit. Returns true on matching.
bool addSourcesPre(const CallExpr *CE, const FunctionDecl *FDecl,
StringRef Name, CheckerContext &C) const;

/// Propagate taint generated at pre-visit.
/// Mark filter's arguments not tainted on a pre-visit. Returns true on
/// matching.
bool addFiltersPre(const CallExpr *CE, StringRef Name,
CheckerContext &C) const;

/// Propagate taint generated at pre-visit. Returns true on matching.
bool propagateFromPre(const CallExpr *CE, CheckerContext &C) const;

/// Check if the region the expression evaluates to is the standard input,
Expand Down Expand Up @@ -442,14 +449,26 @@ GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(

void GenericTaintChecker::checkPreStmt(const CallExpr *CE,
CheckerContext &C) const {
const FunctionDecl *FDecl = C.getCalleeDecl(CE);
// Check for non-global functions.
if (!FDecl || FDecl->getKind() != Decl::Function)
return;

StringRef Name = C.getCalleeName(FDecl);
if (Name.empty())
return;

// Check for taintedness related errors first: system call, uncontrolled
// format string, tainted buffer size.
if (checkPre(CE, C))
if (checkPre(CE, FDecl, Name, C))
return;

// Marks the function's arguments and/or return value tainted if it present in
// the list.
addSourcesPre(CE, C);
if (addSourcesPre(CE, FDecl, Name, C))
return;

addFiltersPre(CE, Name, C);
}

void GenericTaintChecker::checkPostStmt(const CallExpr *CE,
Expand All @@ -464,31 +483,46 @@ void GenericTaintChecker::printState(raw_ostream &Out, ProgramStateRef State,
printTaint(State, Out, NL, Sep);
}

void GenericTaintChecker::addSourcesPre(const CallExpr *CE,
bool GenericTaintChecker::addSourcesPre(const CallExpr *CE,
const FunctionDecl *FDecl,
StringRef Name,
CheckerContext &C) const {
ProgramStateRef State = nullptr;
const FunctionDecl *FDecl = C.getCalleeDecl(CE);
if (!FDecl || FDecl->getKind() != Decl::Function)
return;

StringRef Name = C.getCalleeName(FDecl);
if (Name.empty())
return;

// First, try generating a propagation rule for this function.
TaintPropagationRule Rule = TaintPropagationRule::getTaintPropagationRule(
this->CustomPropagations, FDecl, Name, C);
if (!Rule.isNull()) {
State = Rule.process(CE, C);
if (!State)
return;
C.addTransition(State);
return;
ProgramStateRef State = Rule.process(CE, C);
if (State) {
C.addTransition(State);
return true;
}
}
return false;
}

if (!State)
return;
C.addTransition(State);
bool GenericTaintChecker::addFiltersPre(const CallExpr *CE, StringRef Name,
CheckerContext &C) const {
auto It = CustomFilters.find(Name);
if (It == CustomFilters.end())
return false;

ProgramStateRef State = C.getState();
const ArgVector &Args = It->getValue();
for (unsigned ArgNum : Args) {
if (ArgNum >= CE->getNumArgs())
continue;

const Expr *Arg = CE->getArg(ArgNum);
Optional<SVal> V = getPointedToSVal(C, Arg);
if (V)
State = removeTaint(State, *V);
}

if (State != C.getState()) {
C.addTransition(State);
return true;
}
return false;
}

bool GenericTaintChecker::propagateFromPre(const CallExpr *CE,
Expand Down Expand Up @@ -530,19 +564,12 @@ bool GenericTaintChecker::propagateFromPre(const CallExpr *CE,
}

bool GenericTaintChecker::checkPre(const CallExpr *CE,
const FunctionDecl *FDecl, StringRef Name,
CheckerContext &C) const {

if (checkUncontrolledFormatString(CE, C))
return true;

const FunctionDecl *FDecl = C.getCalleeDecl(CE);
if (!FDecl || FDecl->getKind() != Decl::Function)
return false;

StringRef Name = C.getCalleeName(FDecl);
if (Name.empty())
return false;

if (checkSystemCall(CE, Name, C))
return true;

Expand Down
37 changes: 31 additions & 6 deletions clang/lib/StaticAnalyzer/Checkers/Taint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ void taint::printTaint(ProgramStateRef State, raw_ostream &Out, const char *NL,
Out << I.first << " : " << I.second << NL;
}

void dumpTaint(ProgramStateRef State) {
printTaint(State, llvm::errs());
}
void dumpTaint(ProgramStateRef State) { printTaint(State, llvm::errs()); }

ProgramStateRef taint::addTaint(ProgramStateRef State, const Stmt *S,
const LocationContext *LCtx,
Expand All @@ -64,8 +62,8 @@ ProgramStateRef taint::addTaint(ProgramStateRef State, SVal V,
// region of the parent region.
if (auto LCV = V.getAs<nonloc::LazyCompoundVal>()) {
if (Optional<SVal> binding =
State->getStateManager().getStoreManager()
.getDefaultBinding(*LCV)) {
State->getStateManager().getStoreManager().getDefaultBinding(
*LCV)) {
if (SymbolRef Sym = binding->getAsSymbol())
return addPartialTaint(State, Sym, LCV->getRegion(), Kind);
}
Expand Down Expand Up @@ -94,6 +92,32 @@ ProgramStateRef taint::addTaint(ProgramStateRef State, SymbolRef Sym,
return NewState;
}

ProgramStateRef taint::removeTaint(ProgramStateRef State, SVal V) {
SymbolRef Sym = V.getAsSymbol();
if (Sym)
return removeTaint(State, Sym);

const MemRegion *R = V.getAsRegion();
return removeTaint(State, R);
}

ProgramStateRef taint::removeTaint(ProgramStateRef State, const MemRegion *R) {
if (const SymbolicRegion *SR = dyn_cast_or_null<SymbolicRegion>(R))
return removeTaint(State, SR->getSymbol());
return State;
}

ProgramStateRef taint::removeTaint(ProgramStateRef State, SymbolRef Sym) {
// If this is a symbol cast, remove the cast before adding the taint. Taint
// is cast agnostic.
while (const SymbolCast *SC = dyn_cast<SymbolCast>(Sym))
Sym = SC->getOperand();

ProgramStateRef NewState = State->remove<TaintMap>(Sym);
assert(NewState);
return NewState;
}

ProgramStateRef taint::addPartialTaint(ProgramStateRef State,
SymbolRef ParentSym,
const SubRegion *SubRegion,
Expand Down Expand Up @@ -157,7 +181,8 @@ bool taint::isTainted(ProgramStateRef State, SymbolRef Sym, TaintTagType Kind) {

// Traverse all the symbols this symbol depends on to see if any are tainted.
for (SymExpr::symbol_iterator SI = Sym->symbol_begin(),
SE = Sym->symbol_end(); SI != SE; ++SI) {
SE = Sym->symbol_end();
SI != SE; ++SI) {
if (!isa<SymbolData>(*SI))
continue;

Expand Down
38 changes: 21 additions & 17 deletions clang/lib/StaticAnalyzer/Checkers/Taint.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,34 +27,39 @@ using TaintTagType = unsigned;
static constexpr TaintTagType TaintTagGeneric = 0;

/// Create a new state in which the value of the statement is marked as tainted.
LLVM_NODISCARD ProgramStateRef
addTaint(ProgramStateRef State, const Stmt *S, const LocationContext *LCtx,
TaintTagType Kind = TaintTagGeneric);
LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State, const Stmt *S,
const LocationContext *LCtx,
TaintTagType Kind = TaintTagGeneric);

/// Create a new state in which the value is marked as tainted.
LLVM_NODISCARD ProgramStateRef
addTaint(ProgramStateRef State, SVal V,
TaintTagType Kind = TaintTagGeneric);
LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State, SVal V,
TaintTagType Kind = TaintTagGeneric);

/// Create a new state in which the symbol is marked as tainted.
LLVM_NODISCARD ProgramStateRef
addTaint(ProgramStateRef State, SymbolRef Sym,
TaintTagType Kind = TaintTagGeneric);
LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State, SymbolRef Sym,
TaintTagType Kind = TaintTagGeneric);

/// Create a new state in which the pointer represented by the region
/// is marked as tainted.
LLVM_NODISCARD ProgramStateRef
addTaint(ProgramStateRef State, const MemRegion *R,
TaintTagType Kind = TaintTagGeneric);
LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State,
const MemRegion *R,
TaintTagType Kind = TaintTagGeneric);

LLVM_NODISCARD ProgramStateRef removeTaint(ProgramStateRef State, SVal V);

LLVM_NODISCARD ProgramStateRef removeTaint(ProgramStateRef State,
const MemRegion *R);

LLVM_NODISCARD ProgramStateRef removeTaint(ProgramStateRef State,
SymbolRef Sym);

/// Create a new state in a which a sub-region of a given symbol is tainted.
/// This might be necessary when referring to regions that can not have an
/// individual symbol, e.g. if they are represented by the default binding of
/// a LazyCompoundVal.
LLVM_NODISCARD ProgramStateRef
addPartialTaint(ProgramStateRef State,
SymbolRef ParentSym, const SubRegion *SubRegion,
TaintTagType Kind = TaintTagGeneric);
LLVM_NODISCARD ProgramStateRef addPartialTaint(
ProgramStateRef State, SymbolRef ParentSym, const SubRegion *SubRegion,
TaintTagType Kind = TaintTagGeneric);

/// Check if the statement has a tainted value in the given state.
bool isTainted(ProgramStateRef State, const Stmt *S,
Expand Down Expand Up @@ -99,4 +104,3 @@ class TaintBugVisitor final : public BugReporterVisitor {
} // namespace clang

#endif

4 changes: 2 additions & 2 deletions clang/test/Analysis/Inputs/taint-generic-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ Propagations:
# A list of filter functions
Filters:
# int x; // x is tainted
# myFilter(&x); // x is not tainted anymore
- Name: myFilter
# isOutOfRange(&x); // x is not tainted anymore
- Name: isOutOfRange
Args: [0]

# A list of sink functions
Expand Down
10 changes: 9 additions & 1 deletion clang/test/Analysis/properties.m
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,8 @@ - (NSObject *)getShadowedIvar;
- (void)clearShadowedIvar;
- (NSObject *)getShadowedProp;
- (void)clearShadowedProp;

@property (assign) NSObject *o2;
@end

@implementation Shadowed
Expand Down Expand Up @@ -1078,12 +1080,18 @@ @implementation Shadowing
@synthesize o;

-(void)testPropertyShadowing {
NSObject *oo = self.o;
NSObject *oo = self.o; // no-crash
clang_analyzer_eval(self.o == oo); // expected-warning{{TRUE}}
clang_analyzer_eval([self getShadowedIvar] == oo); // expected-warning{{UNKNOWN}}
[self clearShadowedIvar];
clang_analyzer_eval(self.o == oo); // expected-warning{{TRUE}}
clang_analyzer_eval([self getShadowedIvar] == oo); // expected-warning{{UNKNOWN}}
clang_analyzer_eval([self getShadowedIvar] == nil); // expected-warning{{TRUE}}
}

@synthesize o2 = ooo2;

-(void)testPropertyShadowingWithExplicitIvar {
NSObject *oo2 = self.o2; // no-crash
}
@end
10 changes: 10 additions & 0 deletions clang/test/Analysis/taint-generic.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ extern struct _FILE *stdin;
extern FILE *stdin;
#endif

#define bool _Bool

int fscanf(FILE *restrict stream, const char *restrict format, ...);
int sprintf(char *str, const char *format, ...);
void setproctitle(const char *fmt, ...);
Expand Down Expand Up @@ -346,6 +348,7 @@ void mySource2(int*);
void myScanf(const char*, ...);
int myPropagator(int, int*);
int mySnprintf(char*, size_t, const char*, ...);
bool isOutOfRange(const int*);
void mySink(int, int, int);

void testConfigurationSources1() {
Expand All @@ -372,6 +375,13 @@ void testConfigurationPropagation() {
Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
}

void testConfigurationFilter() {
int x = mySource1();
if (isOutOfRange(&x)) // the filter function
return;
Buffer[x] = 1; // no-warning
}

void testConfigurationSinks() {
int x = mySource1();
mySink(x, 1, 2);
Expand Down