Author: Artem Dergachev Date: 2020-12-10T19:46:33-08:00 New Revision: 8c5ca7c6e62c203b9642dca2a1d9118c36777ad5
URL: https://github.com/llvm/llvm-project/commit/8c5ca7c6e62c203b9642dca2a1d9118c36777ad5 DIFF: https://github.com/llvm/llvm-project/commit/8c5ca7c6e62c203b9642dca2a1d9118c36777ad5.diff LOG: [analyzer] OSObjectCStyleCast: Improve warning message. Suggest OSRequiredCast as a closer alternative to C-style cast. Explain how to decide. Added: Modified: clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp clang/test/Analysis/osobjectcstylecastchecker_test.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp b/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp index 53ed0e187a4c..270b66dab020 100644 --- a/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/OSObjectCStyleCast.cpp @@ -24,32 +24,36 @@ using namespace ento; using namespace ast_matchers; namespace { - -const char *WarnAtNode = "OSObjCast"; +static constexpr const char *const WarnAtNode = "WarnAtNode"; +static constexpr const char *const WarnRecordDecl = "WarnRecordDecl"; class OSObjectCStyleCastChecker : public Checker<check::ASTCodeBody> { public: - void checkASTCodeBody(const Decl *D, - AnalysisManager &AM, + void checkASTCodeBody(const Decl *D, AnalysisManager &AM, BugReporter &BR) const; }; +} static void emitDiagnostics(const BoundNodes &Nodes, BugReporter &BR, AnalysisDeclContext *ADC, const OSObjectCStyleCastChecker *Checker) { const auto *CE = Nodes.getNodeAs<CastExpr>(WarnAtNode); - assert(CE); + const CXXRecordDecl *RD = Nodes.getNodeAs<CXXRecordDecl>(WarnRecordDecl); + assert(CE && RD); std::string Diagnostics; llvm::raw_string_ostream OS(Diagnostics); - OS << "C-style cast of OSObject. Use OSDynamicCast instead."; + OS << "C-style cast of an OSObject is prone to type confusion attacks; " + << "use 'OSRequiredCast' if the object is definitely of type '" + << RD->getNameAsString() << "', or 'OSDynamicCast' followed by " + << "a null check if unsure", BR.EmitBasicReport( ADC->getDecl(), Checker, /*Name=*/"OSObject C-Style Cast", - /*BugCategory=*/"Security", + categories::SecurityError, OS.str(), PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), ADC), CE->getSourceRange()); @@ -68,7 +72,7 @@ void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D, AnalysisManager auto OSObjTypeM = hasTypePointingTo(cxxRecordDecl(isDerivedFrom("OSMetaClassBase"))); auto OSObjSubclassM = hasTypePointingTo( - cxxRecordDecl(isDerivedFrom("OSObject"))); + cxxRecordDecl(isDerivedFrom("OSObject")).bind(WarnRecordDecl)); auto CastM = cStyleCastExpr( allOf(hasSourceExpression(allOf(OSObjTypeM, unless(DynamicCastM))), @@ -78,7 +82,6 @@ void OSObjectCStyleCastChecker::checkASTCodeBody(const Decl *D, AnalysisManager for (BoundNodes Match : Matches) emitDiagnostics(Match, BR, ADC, this); } -} void ento::registerOSObjectCStyleCast(CheckerManager &Mgr) { Mgr.registerChecker<OSObjectCStyleCastChecker>(); diff --git a/clang/test/Analysis/osobjectcstylecastchecker_test.cpp b/clang/test/Analysis/osobjectcstylecastchecker_test.cpp index fabed7ee34b1..a5ebb2823a92 100644 --- a/clang/test/Analysis/osobjectcstylecastchecker_test.cpp +++ b/clang/test/Analysis/osobjectcstylecastchecker_test.cpp @@ -13,7 +13,7 @@ struct B : public A { }; unsigned warn_on_explicit_downcast(OSObject * obj) { - OSArray *a = (OSArray *) obj; // expected-warning{{C-style cast of OSObject. Use OSDynamicCast instead}} + OSArray *a = (OSArray *) obj; // expected-warning{{C-style cast of an OSObject is prone to type confusion attacks; use 'OSRequiredCast' if the object is definitely of type 'OSArray', or 'OSDynamicCast' followed by a null check if unsure}} return a->getCount(); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
