Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp @@ -1,4 +1,4 @@ -//=== CastToStructChecker.cpp - Fixed address usage checker ----*- C++ -*--===// +//=== CastToStructChecker.cpp ----------------------------------*- C++ -*--===// // // The LLVM Compiler Infrastructure // @@ -8,12 +8,13 @@ //===----------------------------------------------------------------------===// // // This files defines CastToStructChecker, a builtin checker that checks for -// cast from non-struct pointer to struct pointer. +// cast from non-struct pointer to struct pointer and widening struct data cast. // This check corresponds to CWE-588. // //===----------------------------------------------------------------------===// #include "ClangSACheckers.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/CheckerManager.h" @@ -23,18 +24,22 @@ using namespace ento; namespace { -class CastToStructChecker : public Checker< check::PreStmt > { - mutable std::unique_ptr BT; +class CastToStructVisitor : public RecursiveASTVisitor { + BugReporter &BR; + const CheckerBase *Checker; + AnalysisDeclContext *AC; public: - void checkPreStmt(const CastExpr *CE, CheckerContext &C) const; + explicit CastToStructVisitor(BugReporter &B, const CheckerBase *Checker, + AnalysisDeclContext *A) + : BR(B), Checker(Checker), AC(A) {} + bool VisitCastExpr(const CastExpr *CE); }; } -void CastToStructChecker::checkPreStmt(const CastExpr *CE, - CheckerContext &C) const { +bool CastToStructVisitor::VisitCastExpr(const CastExpr *CE) { const Expr *E = CE->getSubExpr(); - ASTContext &Ctx = C.getASTContext(); + ASTContext &Ctx = AC->getASTContext(); QualType OrigTy = Ctx.getCanonicalType(E->getType()); QualType ToTy = Ctx.getCanonicalType(CE->getType()); @@ -42,34 +47,72 @@ const PointerType *ToPTy = dyn_cast(ToTy.getTypePtr()); if (!ToPTy || !OrigPTy) - return; + return true; QualType OrigPointeeTy = OrigPTy->getPointeeType(); QualType ToPointeeTy = ToPTy->getPointeeType(); if (!ToPointeeTy->isStructureOrClassType()) - return; + return true; // We allow cast from void*. if (OrigPointeeTy->isVoidType()) - return; + return true; // Now the cast-to-type is struct pointer, the original type is not void*. if (!OrigPointeeTy->isRecordType()) { - if (ExplodedNode *N = C.generateNonFatalErrorNode()) { - if (!BT) - BT.reset( - new BuiltinBug(this, "Cast from non-struct type to struct type", - "Casting a non-structure type to a structure type " - "and accessing a field can lead to memory access " - "errors or data corruption.")); - auto R = llvm::make_unique(*BT, BT->getDescription(), N); - R->addRange(CE->getSourceRange()); - C.emitReport(std::move(R)); - } + SourceRange Sr[1] = {CE->getSourceRange()}; + PathDiagnosticLocation Loc(CE, BR.getSourceManager(), AC); + BR.EmitBasicReport( + AC->getDecl(), Checker, "Cast from non-struct type to struct type", + categories::LogicError, "Casting a non-structure type to a structure " + "type and accessing a field can lead to memory " + "access errors or data corruption.", + Loc, Sr); + } else { + // Don't warn when size of data is unknown. + const auto *U = dyn_cast(E); + if (!U || U->getOpcode() != UO_AddrOf) + return true; + + // Don't warn for references + const ValueDecl *VD = nullptr; + if (const auto *SE = dyn_cast(U->getSubExpr())) + VD = dyn_cast(SE->getDecl()); + else if (const auto *SE = dyn_cast(U->getSubExpr())) + VD = SE->getMemberDecl(); + if (!VD || VD->getType()->isReferenceType()) + return true; + + // Warn when there is widening cast. + unsigned ToWidth = Ctx.getTypeInfo(ToPointeeTy).Width; + unsigned OrigWidth = Ctx.getTypeInfo(OrigPointeeTy).Width; + if (ToWidth <= OrigWidth) + return true; + + PathDiagnosticLocation Loc(CE, BR.getSourceManager(), AC); + BR.EmitBasicReport(AC->getDecl(), Checker, "Widening cast to struct type", + categories::LogicError, + "Casting data to a larger structure type and accessing " + "a field can lead to memory access errors or data " + "corruption.", + Loc, CE->getSourceRange()); } + + return true; } +namespace { +class CastToStructChecker : public Checker { +public: + void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr, + BugReporter &BR) const { + CastToStructVisitor Visitor(BR, this, Mgr.getAnalysisDeclContext(D)); + Visitor.TraverseDecl(const_cast(D)); + } +}; +} // end anonymous namespace + void ento::registerCastToStructChecker(CheckerManager &mgr) { mgr.registerChecker(); } Index: cfe/trunk/test/Analysis/cast-to-struct.cpp =================================================================== --- cfe/trunk/test/Analysis/cast-to-struct.cpp +++ cfe/trunk/test/Analysis/cast-to-struct.cpp @@ -0,0 +1,67 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.core.CastToStruct,core -verify %s + +struct AB { + int A; + int B; +}; + +struct ABC { + int A; + int B; + int C; +}; + +struct Base { + Base() : A(0), B(0) {} + virtual ~Base() {} + + int A; + int B; +}; + +struct Derived : public Base { + Derived() : Base(), C(0) {} + int C; +}; + +void structToStruct(struct AB *P) { + struct AB Ab; + struct ABC *Abc; + Abc = (struct ABC *)&Ab; // expected-warning {{Casting data to a larger structure type and accessing a field can lead to memory access errors or data corruption}} + Abc = (struct ABC *)P; // No warning; It is not known what data P points at. + Abc = (struct ABC *)&*P; + + // Don't warn when the cast is not widening. + P = (struct AB *)&Ab; // struct AB * => struct AB * + struct ABC Abc2; + P = (struct AB *)&Abc2; // struct ABC * => struct AB * + + // True negatives when casting from Base to Derived. + Derived D1, *D2; + Base &B1 = D1; + D2 = (Derived *)&B1; + D2 = dynamic_cast(&B1); + D2 = static_cast(&B1); + + // True positives when casting from Base to Derived. + Base B2; + D2 = (Derived *)&B2;// expected-warning {{Casting data to a larger structure type and accessing a field can lead to memory access errors or data corruption}} + D2 = dynamic_cast(&B2);// expected-warning {{Casting data to a larger structure type and accessing a field can lead to memory access errors or data corruption}} + D2 = static_cast(&B2);// expected-warning {{Casting data to a larger structure type and accessing a field can lead to memory access errors or data corruption}} + + // False negatives, cast from Base to Derived. With path sensitive analysis + // these false negatives could be fixed. + Base *B3 = &B2; + D2 = (Derived *)B3; + D2 = dynamic_cast(B3); + D2 = static_cast(B3); +} + +void intToStruct(int *P) { + struct ABC *Abc; + Abc = (struct ABC *)P; // expected-warning {{Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption}} + + // Cast from void *. + void *VP = P; + Abc = (struct ABC *)VP; +} Index: cfe/trunk/test/Analysis/casts.c =================================================================== --- cfe/trunk/test/Analysis/casts.c +++ cfe/trunk/test/Analysis/casts.c @@ -18,7 +18,7 @@ void f(int sock) { struct sockaddr_storage storage; - struct sockaddr* sockaddr = (struct sockaddr*)&storage; + struct sockaddr* sockaddr = (struct sockaddr*)&storage; // expected-warning{{Casting data to a larger structure type and accessing a field can lead to memory access errors or data corruption}} socklen_t addrlen = sizeof(storage); getsockname(sock, sockaddr, &addrlen); switch (sockaddr->sa_family) { // no-warning