r282411 - [analyzer] Improve CastToStruct checker so it can also detect widening casts of struct data
Daniel Marjamaki via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 26 08:17:19 PDT 2016
Author: danielmarjamaki
Date: Mon Sep 26 10:17:18 2016
New Revision: 282411
URL: http://llvm.org/viewvc/llvm-project?rev=282411&view=rev
Log:
[analyzer] Improve CastToStruct checker so it can also detect widening casts of struct data
Example:
struct AB {
int A;
int B;
};
struct ABC {
int A;
int B;
int C;
};
void f() {
struct AB Data;
struct ABC *P = (struct ABC *)&Data;
}
Differential Revision: https://reviews.llvm.org/D23508
Added:
cfe/trunk/test/Analysis/cast-to-struct.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
cfe/trunk/test/Analysis/casts.c
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp?rev=282411&r1=282410&r2=282411&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp Mon Sep 26 10:17:18 2016
@@ -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 clang;
using namespace ento;
namespace {
-class CastToStructChecker : public Checker< check::PreStmt<CastExpr> > {
- mutable std::unique_ptr<BuiltinBug> BT;
+class CastToStructVisitor : public RecursiveASTVisitor<CastToStructVisitor> {
+ 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 @@ void CastToStructChecker::checkPreStmt(c
const PointerType *ToPTy = dyn_cast<PointerType>(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<BugReport>(*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<UnaryOperator>(E);
+ if (!U || U->getOpcode() != UO_AddrOf)
+ return true;
+
+ // Don't warn for references
+ const ValueDecl *VD = nullptr;
+ if (const auto *SE = dyn_cast<DeclRefExpr>(U->getSubExpr()))
+ VD = dyn_cast<ValueDecl>(SE->getDecl());
+ else if (const auto *SE = dyn_cast<MemberExpr>(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<check::ASTCodeBody> {
+public:
+ void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
+ BugReporter &BR) const {
+ CastToStructVisitor Visitor(BR, this, Mgr.getAnalysisDeclContext(D));
+ Visitor.TraverseDecl(const_cast<Decl *>(D));
+ }
+};
+} // end anonymous namespace
+
void ento::registerCastToStructChecker(CheckerManager &mgr) {
mgr.registerChecker<CastToStructChecker>();
}
Added: cfe/trunk/test/Analysis/cast-to-struct.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cast-to-struct.cpp?rev=282411&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/cast-to-struct.cpp (added)
+++ cfe/trunk/test/Analysis/cast-to-struct.cpp Mon Sep 26 10:17:18 2016
@@ -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<Derived *>(&B1);
+ D2 = static_cast<Derived *>(&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<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 = static_cast<Derived *>(&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<Derived *>(B3);
+ D2 = static_cast<Derived *>(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;
+}
Modified: cfe/trunk/test/Analysis/casts.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/casts.c?rev=282411&r1=282410&r2=282411&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/casts.c (original)
+++ cfe/trunk/test/Analysis/casts.c Mon Sep 26 10:17:18 2016
@@ -18,7 +18,7 @@ void getsockname();
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
More information about the cfe-commits
mailing list