diff options
-rw-r--r-- | include/clang/Analysis/PathSensitive/CheckerVisitor.def | 1 | ||||
-rw-r--r-- | include/clang/Analysis/PathSensitive/CheckerVisitor.h | 9 | ||||
-rw-r--r-- | lib/Analysis/CastToStructChecker.cpp | 78 | ||||
-rw-r--r-- | lib/Analysis/GRExprEngine.cpp | 15 | ||||
-rw-r--r-- | lib/Analysis/GRExprEngineInternalChecks.cpp | 2 | ||||
-rw-r--r-- | lib/Analysis/GRExprEngineInternalChecks.h | 1 | ||||
-rw-r--r-- | test/Analysis/array-struct.c | 2 | ||||
-rw-r--r-- | test/Analysis/misc-ps-region-store.m | 2 |
8 files changed, 102 insertions, 8 deletions
diff --git a/include/clang/Analysis/PathSensitive/CheckerVisitor.def b/include/clang/Analysis/PathSensitive/CheckerVisitor.def index d445f3afe8..e533d9e1e5 100644 --- a/include/clang/Analysis/PathSensitive/CheckerVisitor.def +++ b/include/clang/Analysis/PathSensitive/CheckerVisitor.def @@ -13,6 +13,7 @@ PREVISIT(BinaryOperator) PREVISIT(CallExpr) +PREVISIT(CastExpr) PREVISIT(DeclStmt) PREVISIT(ObjCMessageExpr) PREVISIT(ReturnStmt) diff --git a/include/clang/Analysis/PathSensitive/CheckerVisitor.h b/include/clang/Analysis/PathSensitive/CheckerVisitor.h index e74f49c9a7..f2e8f29aa6 100644 --- a/include/clang/Analysis/PathSensitive/CheckerVisitor.h +++ b/include/clang/Analysis/PathSensitive/CheckerVisitor.h @@ -36,10 +36,19 @@ public: default: assert(false && "Unsupport statement."); return; + + case Stmt::ImplicitCastExprClass: + case Stmt::ExplicitCastExprClass: + case Stmt::CStyleCastExprClass: + static_cast<ImplClass*>(this)->PreVisitCastExpr(C, + static_cast<const CastExpr*>(S)); + break; + case Stmt::CompoundAssignOperatorClass: static_cast<ImplClass*>(this)->PreVisitBinaryOperator(C, static_cast<const BinaryOperator*>(S)); break; + #define PREVISIT(NAME) \ case Stmt::NAME ## Class:\ static_cast<ImplClass*>(this)->PreVisit ## NAME(C,static_cast<const NAME*>(S));\ diff --git a/lib/Analysis/CastToStructChecker.cpp b/lib/Analysis/CastToStructChecker.cpp new file mode 100644 index 0000000000..5d0110e4eb --- /dev/null +++ b/lib/Analysis/CastToStructChecker.cpp @@ -0,0 +1,78 @@ +//=== CastToStructChecker.cpp - Fixed address usage checker ----*- C++ -*--===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This files defines CastToStructChecker, a builtin checker that checks for +// assignment of a fixed address to a pointer. +// This check corresponds to CWE-588. +// +//===----------------------------------------------------------------------===// + +#include "clang/Analysis/PathSensitive/CheckerVisitor.h" +#include "GRExprEngineInternalChecks.h" + +using namespace clang; + +namespace { +class VISIBILITY_HIDDEN CastToStructChecker + : public CheckerVisitor<CastToStructChecker> { + BuiltinBug *BT; +public: + CastToStructChecker() : BT(0) {} + static void *getTag(); + void PreVisitCastExpr(CheckerContext &C, const CastExpr *B); +}; +} + +void *CastToStructChecker::getTag() { + static int x; + return &x; +} + +void CastToStructChecker::PreVisitCastExpr(CheckerContext &C, + const CastExpr *CE) { + const Expr *E = CE->getSubExpr(); + ASTContext &Ctx = C.getASTContext(); + QualType OrigTy = Ctx.getCanonicalType(E->getType()); + QualType ToTy = Ctx.getCanonicalType(CE->getType()); + + PointerType *OrigPTy = dyn_cast<PointerType>(OrigTy.getTypePtr()); + PointerType *ToPTy = dyn_cast<PointerType>(ToTy.getTypePtr()); + + if (!ToPTy || !OrigPTy) + return; + + QualType OrigPointeeTy = OrigPTy->getPointeeType(); + QualType ToPointeeTy = ToPTy->getPointeeType(); + + if (!ToPointeeTy->isRecordType()) + return; + + // We allow cast from void*. + if (OrigPointeeTy->isVoidType()) + return; + + // Now the cast-to-type is struct pointer, the original type is not void*. + if (!OrigPointeeTy->isRecordType()) { + if (ExplodedNode *N = C.GenerateNode(CE)) { + if (!BT) + BT = new BuiltinBug("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."); + RangedBugReport *R = new RangedBugReport(*BT,BT->getDescription().c_str(), + N); + R->addRange(CE->getSourceRange()); + C.EmitReport(R); + } + } +} + +void clang::RegisterCastToStructChecker(GRExprEngine &Eng) { + Eng.registerCheck(new CastToStructChecker()); +} diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp index eaa4e27f39..41c5da9ee1 100644 --- a/lib/Analysis/GRExprEngine.cpp +++ b/lib/Analysis/GRExprEngine.cpp @@ -2035,7 +2035,8 @@ void GRExprEngine::VisitObjCMessageExprDispatchHelper(ObjCMessageExpr* ME, // Transfer functions: Miscellaneous statements. //===----------------------------------------------------------------------===// -void GRExprEngine::VisitCast(Expr* CastE, Expr* Ex, ExplodedNode* Pred, ExplodedNodeSet& Dst){ +void GRExprEngine::VisitCast(Expr* CastE, Expr* Ex, ExplodedNode* Pred, + ExplodedNodeSet& Dst){ ExplodedNodeSet S1; QualType T = CastE->getType(); QualType ExTy = Ex->getType(); @@ -2048,16 +2049,18 @@ void GRExprEngine::VisitCast(Expr* CastE, Expr* Ex, ExplodedNode* Pred, Exploded else Visit(Ex, Pred, S1); + ExplodedNodeSet S2; + CheckerVisit(CastE, S2, S1, true); + // Check for casting to "void". if (T->isVoidType()) { - for (ExplodedNodeSet::iterator I1 = S1.begin(), E1 = S1.end(); I1 != E1; ++I1) - Dst.Add(*I1); - + for (ExplodedNodeSet::iterator I = S2.begin(), E = S2.end(); I != E; ++I) + Dst.Add(*I); return; } - for (ExplodedNodeSet::iterator I1 = S1.begin(), E1 = S1.end(); I1 != E1; ++I1) { - ExplodedNode* N = *I1; + for (ExplodedNodeSet::iterator I = S2.begin(), E = S2.end(); I != E; ++I) { + ExplodedNode* N = *I; const GRState* state = GetState(N); SVal V = state->getSVal(Ex); const SValuator::CastResult &Res = SVator.EvalCast(V, state, T, ExTy); diff --git a/lib/Analysis/GRExprEngineInternalChecks.cpp b/lib/Analysis/GRExprEngineInternalChecks.cpp index 37e7bfdf61..41bb65558c 100644 --- a/lib/Analysis/GRExprEngineInternalChecks.cpp +++ b/lib/Analysis/GRExprEngineInternalChecks.cpp @@ -416,4 +416,6 @@ void GRExprEngine::RegisterInternalChecks() { RegisterFixedAddressChecker(*this); // Note that this must be registered after ReturnStackAddressChecker. RegisterReturnPointerRangeChecker(*this); + + RegisterCastToStructChecker(*this); } diff --git a/lib/Analysis/GRExprEngineInternalChecks.h b/lib/Analysis/GRExprEngineInternalChecks.h index 90a2826787..4f8baadb78 100644 --- a/lib/Analysis/GRExprEngineInternalChecks.h +++ b/lib/Analysis/GRExprEngineInternalChecks.h @@ -26,5 +26,6 @@ void RegisterReturnUndefChecker(GRExprEngine &Eng); void RegisterVLASizeChecker(GRExprEngine &Eng); void RegisterPointerSubChecker(GRExprEngine &Eng); void RegisterFixedAddressChecker(GRExprEngine &Eng); +void RegisterCastToStructChecker(GRExprEngine &Eng); } // end clang namespace #endif diff --git a/test/Analysis/array-struct.c b/test/Analysis/array-struct.c index 18a6ed5074..1ab67874c6 100644 --- a/test/Analysis/array-struct.c +++ b/test/Analysis/array-struct.c @@ -153,7 +153,7 @@ struct s3 p[1]; // an ElementRegion of type 'char'. Then load a nonloc::SymbolVal from it and // assigns to 'a'. void f16(struct s3 *p) { - struct s3 a = *((struct s3*) ((char*) &p[0])); + struct s3 a = *((struct s3*) ((char*) &p[0])); // expected-warning{{Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption.}} } void inv(struct s1 *); diff --git a/test/Analysis/misc-ps-region-store.m b/test/Analysis/misc-ps-region-store.m index edea24224b..2c5180791e 100644 --- a/test/Analysis/misc-ps-region-store.m +++ b/test/Analysis/misc-ps-region-store.m @@ -286,7 +286,7 @@ struct WrappedStruct { unsigned z; }; int test_handle_array_wrapper() { struct ArrayWrapper x; test_handle_array_wrapper(&x); - struct WrappedStruct *p = (struct WrappedStruct*) x.y; + struct WrappedStruct *p = (struct WrappedStruct*) x.y; // expected-warning{{Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption.}} return p->z; // no-warning } |