Skip to content

Commit 956a787

Browse files
authored
Merge pull request #418 from haoNoQ/static-analyzer-cherrypicks-4
Static analyzer cherrypicks 4
2 parents 0005b3f + 47dd569 commit 956a787

File tree

7 files changed

+141
-63
lines changed

7 files changed

+141
-63
lines changed

clang/lib/Analysis/BodyFarm.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -741,13 +741,17 @@ static Stmt *createObjCPropertyGetter(ASTContext &Ctx,
741741
// First, find the backing ivar.
742742
const ObjCIvarDecl *IVar = nullptr;
743743

744-
// Property accessor stubs sometimes do not correspond to any property.
744+
// Property accessor stubs sometimes do not correspond to any property decl
745+
// in the current interface (but in a superclass). They still have a
746+
// corresponding property impl decl in this case.
745747
if (MD->isSynthesizedAccessorStub()) {
746748
const ObjCInterfaceDecl *IntD = MD->getClassInterface();
747749
const ObjCImplementationDecl *ImpD = IntD->getImplementation();
748-
for (const auto *V: ImpD->ivars()) {
749-
if (V->getName() == MD->getSelector().getNameForSlot(0))
750-
IVar = V;
750+
for (const auto *PI: ImpD->property_impls()) {
751+
if (const ObjCPropertyDecl *P = PI->getPropertyDecl()) {
752+
if (P->getGetterName() == MD->getSelector())
753+
IVar = P->getPropertyIvarDecl();
754+
}
751755
}
752756
}
753757

clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

Lines changed: 60 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,20 @@ class GenericTaintChecker
9898
}
9999

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

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

107-
/// Propagate taint generated at pre-visit.
109+
/// Mark filter's arguments not tainted on a pre-visit. Returns true on
110+
/// matching.
111+
bool addFiltersPre(const CallExpr *CE, StringRef Name,
112+
CheckerContext &C) const;
113+
114+
/// Propagate taint generated at pre-visit. Returns true on matching.
108115
bool propagateFromPre(const CallExpr *CE, CheckerContext &C) const;
109116

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

443450
void GenericTaintChecker::checkPreStmt(const CallExpr *CE,
444451
CheckerContext &C) const {
452+
const FunctionDecl *FDecl = C.getCalleeDecl(CE);
453+
// Check for non-global functions.
454+
if (!FDecl || FDecl->getKind() != Decl::Function)
455+
return;
456+
457+
StringRef Name = C.getCalleeName(FDecl);
458+
if (Name.empty())
459+
return;
460+
445461
// Check for taintedness related errors first: system call, uncontrolled
446462
// format string, tainted buffer size.
447-
if (checkPre(CE, C))
463+
if (checkPre(CE, FDecl, Name, C))
448464
return;
449465

450466
// Marks the function's arguments and/or return value tainted if it present in
451467
// the list.
452-
addSourcesPre(CE, C);
468+
if (addSourcesPre(CE, FDecl, Name, C))
469+
return;
470+
471+
addFiltersPre(CE, Name, C);
453472
}
454473

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

467-
void GenericTaintChecker::addSourcesPre(const CallExpr *CE,
486+
bool GenericTaintChecker::addSourcesPre(const CallExpr *CE,
487+
const FunctionDecl *FDecl,
488+
StringRef Name,
468489
CheckerContext &C) const {
469-
ProgramStateRef State = nullptr;
470-
const FunctionDecl *FDecl = C.getCalleeDecl(CE);
471-
if (!FDecl || FDecl->getKind() != Decl::Function)
472-
return;
473-
474-
StringRef Name = C.getCalleeName(FDecl);
475-
if (Name.empty())
476-
return;
477-
478490
// First, try generating a propagation rule for this function.
479491
TaintPropagationRule Rule = TaintPropagationRule::getTaintPropagationRule(
480492
this->CustomPropagations, FDecl, Name, C);
481493
if (!Rule.isNull()) {
482-
State = Rule.process(CE, C);
483-
if (!State)
484-
return;
485-
C.addTransition(State);
486-
return;
494+
ProgramStateRef State = Rule.process(CE, C);
495+
if (State) {
496+
C.addTransition(State);
497+
return true;
498+
}
487499
}
500+
return false;
501+
}
488502

489-
if (!State)
490-
return;
491-
C.addTransition(State);
503+
bool GenericTaintChecker::addFiltersPre(const CallExpr *CE, StringRef Name,
504+
CheckerContext &C) const {
505+
auto It = CustomFilters.find(Name);
506+
if (It == CustomFilters.end())
507+
return false;
508+
509+
ProgramStateRef State = C.getState();
510+
const ArgVector &Args = It->getValue();
511+
for (unsigned ArgNum : Args) {
512+
if (ArgNum >= CE->getNumArgs())
513+
continue;
514+
515+
const Expr *Arg = CE->getArg(ArgNum);
516+
Optional<SVal> V = getPointedToSVal(C, Arg);
517+
if (V)
518+
State = removeTaint(State, *V);
519+
}
520+
521+
if (State != C.getState()) {
522+
C.addTransition(State);
523+
return true;
524+
}
525+
return false;
492526
}
493527

494528
bool GenericTaintChecker::propagateFromPre(const CallExpr *CE,
@@ -530,19 +564,12 @@ bool GenericTaintChecker::propagateFromPre(const CallExpr *CE,
530564
}
531565

532566
bool GenericTaintChecker::checkPre(const CallExpr *CE,
567+
const FunctionDecl *FDecl, StringRef Name,
533568
CheckerContext &C) const {
534569

535570
if (checkUncontrolledFormatString(CE, C))
536571
return true;
537572

538-
const FunctionDecl *FDecl = C.getCalleeDecl(CE);
539-
if (!FDecl || FDecl->getKind() != Decl::Function)
540-
return false;
541-
542-
StringRef Name = C.getCalleeName(FDecl);
543-
if (Name.empty())
544-
return false;
545-
546573
if (checkSystemCall(CE, Name, C))
547574
return true;
548575

clang/lib/StaticAnalyzer/Checkers/Taint.cpp

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,7 @@ void taint::printTaint(ProgramStateRef State, raw_ostream &Out, const char *NL,
3737
Out << I.first << " : " << I.second << NL;
3838
}
3939

40-
void dumpTaint(ProgramStateRef State) {
41-
printTaint(State, llvm::errs());
42-
}
40+
void dumpTaint(ProgramStateRef State) { printTaint(State, llvm::errs()); }
4341

4442
ProgramStateRef taint::addTaint(ProgramStateRef State, const Stmt *S,
4543
const LocationContext *LCtx,
@@ -64,8 +62,8 @@ ProgramStateRef taint::addTaint(ProgramStateRef State, SVal V,
6462
// region of the parent region.
6563
if (auto LCV = V.getAs<nonloc::LazyCompoundVal>()) {
6664
if (Optional<SVal> binding =
67-
State->getStateManager().getStoreManager()
68-
.getDefaultBinding(*LCV)) {
65+
State->getStateManager().getStoreManager().getDefaultBinding(
66+
*LCV)) {
6967
if (SymbolRef Sym = binding->getAsSymbol())
7068
return addPartialTaint(State, Sym, LCV->getRegion(), Kind);
7169
}
@@ -94,6 +92,32 @@ ProgramStateRef taint::addTaint(ProgramStateRef State, SymbolRef Sym,
9492
return NewState;
9593
}
9694

95+
ProgramStateRef taint::removeTaint(ProgramStateRef State, SVal V) {
96+
SymbolRef Sym = V.getAsSymbol();
97+
if (Sym)
98+
return removeTaint(State, Sym);
99+
100+
const MemRegion *R = V.getAsRegion();
101+
return removeTaint(State, R);
102+
}
103+
104+
ProgramStateRef taint::removeTaint(ProgramStateRef State, const MemRegion *R) {
105+
if (const SymbolicRegion *SR = dyn_cast_or_null<SymbolicRegion>(R))
106+
return removeTaint(State, SR->getSymbol());
107+
return State;
108+
}
109+
110+
ProgramStateRef taint::removeTaint(ProgramStateRef State, SymbolRef Sym) {
111+
// If this is a symbol cast, remove the cast before adding the taint. Taint
112+
// is cast agnostic.
113+
while (const SymbolCast *SC = dyn_cast<SymbolCast>(Sym))
114+
Sym = SC->getOperand();
115+
116+
ProgramStateRef NewState = State->remove<TaintMap>(Sym);
117+
assert(NewState);
118+
return NewState;
119+
}
120+
97121
ProgramStateRef taint::addPartialTaint(ProgramStateRef State,
98122
SymbolRef ParentSym,
99123
const SubRegion *SubRegion,
@@ -157,7 +181,8 @@ bool taint::isTainted(ProgramStateRef State, SymbolRef Sym, TaintTagType Kind) {
157181

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

clang/lib/StaticAnalyzer/Checkers/Taint.h

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,34 +27,39 @@ using TaintTagType = unsigned;
2727
static constexpr TaintTagType TaintTagGeneric = 0;
2828

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

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

3938
/// Create a new state in which the symbol is marked as tainted.
40-
LLVM_NODISCARD ProgramStateRef
41-
addTaint(ProgramStateRef State, SymbolRef Sym,
42-
TaintTagType Kind = TaintTagGeneric);
39+
LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State, SymbolRef Sym,
40+
TaintTagType Kind = TaintTagGeneric);
4341

4442
/// Create a new state in which the pointer represented by the region
4543
/// is marked as tainted.
46-
LLVM_NODISCARD ProgramStateRef
47-
addTaint(ProgramStateRef State, const MemRegion *R,
48-
TaintTagType Kind = TaintTagGeneric);
44+
LLVM_NODISCARD ProgramStateRef addTaint(ProgramStateRef State,
45+
const MemRegion *R,
46+
TaintTagType Kind = TaintTagGeneric);
47+
48+
LLVM_NODISCARD ProgramStateRef removeTaint(ProgramStateRef State, SVal V);
49+
50+
LLVM_NODISCARD ProgramStateRef removeTaint(ProgramStateRef State,
51+
const MemRegion *R);
52+
53+
LLVM_NODISCARD ProgramStateRef removeTaint(ProgramStateRef State,
54+
SymbolRef Sym);
4955

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

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

101106
#endif
102-

clang/test/Analysis/Inputs/taint-generic-config.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ Propagations:
3636
# A list of filter functions
3737
Filters:
3838
# int x; // x is tainted
39-
# myFilter(&x); // x is not tainted anymore
40-
- Name: myFilter
39+
# isOutOfRange(&x); // x is not tainted anymore
40+
- Name: isOutOfRange
4141
Args: [0]
4242

4343
# A list of sink functions

clang/test/Analysis/properties.m

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1049,6 +1049,8 @@ - (NSObject *)getShadowedIvar;
10491049
- (void)clearShadowedIvar;
10501050
- (NSObject *)getShadowedProp;
10511051
- (void)clearShadowedProp;
1052+
1053+
@property (assign) NSObject *o2;
10521054
@end
10531055

10541056
@implementation Shadowed
@@ -1078,12 +1080,18 @@ @implementation Shadowing
10781080
@synthesize o;
10791081

10801082
-(void)testPropertyShadowing {
1081-
NSObject *oo = self.o;
1083+
NSObject *oo = self.o; // no-crash
10821084
clang_analyzer_eval(self.o == oo); // expected-warning{{TRUE}}
10831085
clang_analyzer_eval([self getShadowedIvar] == oo); // expected-warning{{UNKNOWN}}
10841086
[self clearShadowedIvar];
10851087
clang_analyzer_eval(self.o == oo); // expected-warning{{TRUE}}
10861088
clang_analyzer_eval([self getShadowedIvar] == oo); // expected-warning{{UNKNOWN}}
10871089
clang_analyzer_eval([self getShadowedIvar] == nil); // expected-warning{{TRUE}}
10881090
}
1091+
1092+
@synthesize o2 = ooo2;
1093+
1094+
-(void)testPropertyShadowingWithExplicitIvar {
1095+
NSObject *oo2 = self.o2; // no-crash
1096+
}
10891097
@end

clang/test/Analysis/taint-generic.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ extern struct _FILE *stdin;
5656
extern FILE *stdin;
5757
#endif
5858

59+
#define bool _Bool
60+
5961
int fscanf(FILE *restrict stream, const char *restrict format, ...);
6062
int sprintf(char *str, const char *format, ...);
6163
void setproctitle(const char *fmt, ...);
@@ -346,6 +348,7 @@ void mySource2(int*);
346348
void myScanf(const char*, ...);
347349
int myPropagator(int, int*);
348350
int mySnprintf(char*, size_t, const char*, ...);
351+
bool isOutOfRange(const int*);
349352
void mySink(int, int, int);
350353

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

378+
void testConfigurationFilter() {
379+
int x = mySource1();
380+
if (isOutOfRange(&x)) // the filter function
381+
return;
382+
Buffer[x] = 1; // no-warning
383+
}
384+
375385
void testConfigurationSinks() {
376386
int x = mySource1();
377387
mySink(x, 1, 2);

0 commit comments

Comments
 (0)