Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -41,19 +41,56 @@ public: void checkPostCall(const CallEvent &Call, CheckerContext &C) const; void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const; + + class Visitor : public BugReporterVisitor { + public: + void Profile(llvm::FoldingSetNodeID &ID) const { + static int X = 0; + ID.AddPointer(&X); + } + + std::shared_ptr VisitNode(const ExplodedNode *N, + BugReporterContext &BRC, BugReport &R); + }; }; } // end anonymous namespace -REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, bool) +// FIXME: It's a 'const ParmVarDecl *' but there's no ready-made GDM traits +// specialization for this sort of types. +REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, const void *) + +std::shared_ptr +MIGChecker::Visitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, + BugReport &R) { + const auto *NewPVD = static_cast( + N->getState()->get()); + const auto *OldPVD = static_cast( + N->getFirstPred()->getState()->get()); + if (OldPVD == NewPVD) + return nullptr; + + assert(NewPVD && "What is deallocated cannot be un-deallocated!"); + SmallString<64> Str; + llvm::raw_svector_ostream OS(Str); + OS << "Value passed through parameter '" << NewPVD->getName() + << "' is deallocated"; + + PathDiagnosticLocation Loc = + PathDiagnosticLocation::create(N->getLocation(), BRC.getSourceManager()); + return std::make_shared(Loc, OS.str()); +} -static bool isCurrentArgSVal(SVal V, CheckerContext &C) { +static const ParmVarDecl *getOriginParam(SVal V, CheckerContext &C) { SymbolRef Sym = V.getAsSymbol(); if (!Sym) - return false; + return nullptr; const auto *VR = dyn_cast_or_null(Sym->getOriginRegion()); - return VR && VR->hasStackParametersStorage() && - VR->getStackFrame()->inTopFrame(); + if (VR && VR->hasStackParametersStorage() && + VR->getStackFrame()->inTopFrame()) + return cast(VR->getDecl()); + + return nullptr; } static bool isInMIGCall(CheckerContext &C) { @@ -96,8 +133,11 @@ // TODO: Unhardcode "1". SVal Arg = Call.getArgSVal(1); - if (isCurrentArgSVal(Arg, C)) - C.addTransition(C.getState()->set(true)); + const ParmVarDecl *PVD = getOriginParam(Arg, C); + if (!PVD) + return; + + C.addTransition(C.getState()->set(PVD)); } void MIGChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const { @@ -139,6 +179,9 @@ "deallocate it again", N); + R->addRange(RS->getSourceRange()); + bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, false); + R->addVisitor(llvm::make_unique()); C.emitReport(std::move(R)); } Index: clang/test/Analysis/mig.mm =================================================================== --- clang/test/Analysis/mig.mm +++ clang/test/Analysis/mig.mm @@ -1,5 +1,5 @@ // RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.osx.MIG\ -// RUN: -fblocks -verify %s +// RUN: -analyzer-output=text -fblocks -verify %s // XNU APIs. @@ -20,9 +20,11 @@ MIG_SERVER_ROUTINE kern_return_t basic_test(mach_port_name_t port, vm_address_t address, vm_size_t size) { - vm_deallocate(port, address, size); - if (size > 10) { + vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}} + if (size > 10) { // expected-note{{Assuming 'size' is > 10}} + // expected-note@-1{{Taking true branch}} return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}} + // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}} } return KERN_SUCCESS; } @@ -42,6 +44,18 @@ vm_deallocate(port, address, size); } +// When releasing two parameters, add a note for both of them. +// Also when returning a variable, explain why do we think that it contains +// a non-success code. +MIG_SERVER_ROUTINE +kern_return_t release_twice(mach_port_name_t port, vm_address_t addr1, vm_address_t addr2, vm_size_t size) { + kern_return_t ret = KERN_ERROR; // expected-note{{'ret' initialized to 1}} + vm_deallocate(port, addr1, size); // expected-note{{Value passed through parameter 'addr1' is deallocated}} + vm_deallocate(port, addr2, size); // expected-note{{Value passed through parameter 'addr2' is deallocated}} + return ret; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}} + // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}} +} + // Check that we work on Objective-C messages and blocks. @interface I - (kern_return_t)fooAtPort:(mach_port_name_t)port withAddress:(vm_address_t)address ofSize:(vm_size_t)size; @@ -51,8 +65,9 @@ - (kern_return_t)fooAtPort:(mach_port_name_t)port withAddress:(vm_address_t)address ofSize:(vm_size_t)size MIG_SERVER_ROUTINE { - vm_deallocate(port, address, size); + vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}} return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}} + // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}} } @end @@ -60,8 +75,9 @@ kern_return_t (^block)(mach_port_name_t, vm_address_t, vm_size_t) = ^MIG_SERVER_ROUTINE (mach_port_name_t port, vm_address_t address, vm_size_t size) { - vm_deallocate(port, address, size); + vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}}   return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}} + // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}} }; }