Implement bad cast checks using control flow integrity information.

This scheme checks that pointer and lvalue casts are made to an object of
the correct dynamic type; that is, the dynamic type of the object must be
a derived class of the pointee type of the cast. The checks are currently
only introduced where the class being casted to is a polymorphic class.

Differential Revision: http://reviews.llvm.org/D8312

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@232241 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/docs/ControlFlowIntegrity.rst b/docs/ControlFlowIntegrity.rst
index a4c60b3d98a74daac8bc5ce4b78af4ec64fdd9a8..51c99177674e7c59c9f39ef58ec55f8d2d445831 100644 (file)
--- a/docs/ControlFlowIntegrity.rst
+++ b/docs/ControlFlowIntegrity.rst
@@ -58,6 +58,60 @@ virtual-call-heavy SPEC 2006 xalancbmk.
 Note that this scheme has not yet been optimized for binary size; an increase
 of up to 15% has been observed for Chromium.
 
+Bad Cast Checking
+-----------------
+
+This scheme checks that pointer casts are made to an object of the correct
+dynamic type; that is, the dynamic type of the object must be a derived class
+of the pointee type of the cast. The checks are currently only introduced
+where the class being casted to is a polymorphic class.
+
+Bad casts are not in themselves control flow integrity violations, but they
+can also create security vulnerabilities, and the implementation uses many
+of the same mechanisms.
+
+There are two types of bad cast that may be forbidden: bad casts
+from a base class to a derived class (which can be checked with
+``-fsanitize=cfi-derived-cast``), and bad casts from a pointer of
+type ``void*`` or another unrelated type (which can be checked with
+``-fsanitize=cfi-unrelated-cast``).
+
+The difference between these two types of casts is that the first is defined
+by the C++ standard to produce an undefined value, while the second is not
+in itself undefined behavior (it is well defined to cast the pointer back
+to its original type).
+
+If a program as a matter of policy forbids the second type of cast, that
+restriction can normally be enforced. However it may in some cases be necessary
+for a function to perform a forbidden cast to conform with an external API
+(e.g. the ``allocate`` member function of a standard library allocator). Such
+functions may be blacklisted using a :doc:`SanitizerSpecialCaseList`.
+
+For this scheme to work, all translation units containing the definition
+of a virtual member function (whether inline or not) must be compiled with
+``-fsanitize=cfi-derived-cast`` or ``-fsanitize=cfi-unrelated-cast`` enabled
+and be statically linked into the program. Classes in the C++ standard library
+(under namespace ``std``) are exempted from checking, and therefore programs
+may be linked against a pre-built standard library, but this may change in
+the future.
+
+.. _cfi-strictness:
+
+Strictness
+~~~~~~~~~~
+
+If a class has a single non-virtual base and does not introduce or override
+virtual member functions or fields other than an implicitly defined virtual
+destructor, it will have the same layout and virtual function semantics as
+its base. By default, casts to such classes are checked as if they were made
+to the least derived such class.
+
+Casting an instance of a base class to such a derived class is technically
+undefined behavior, but it is a relatively common hack for introducing
+member functions on class instances with specific properties that works under
+most compilers and should not have security implications, so we allow it by
+default. It can be disabled with ``-fsanitize=cfi-cast-strict``.
+
 Design
 ------
 

diff --git a/docs/UsersManual.rst b/docs/UsersManual.rst
index c02495b8d3d2127d6b4b3f12d3f4e85fff670035..bde3c4a08e92137748a33b8e02f59dc47c9caff8 100644 (file)
--- a/docs/UsersManual.rst
+++ b/docs/UsersManual.rst
@@ -968,6 +968,12 @@ are listed below.
       ``true`` nor ``false``.
    -  ``-fsanitize=bounds``: Out of bounds array indexing, in cases
       where the array bound can be statically determined.
+   -  ``-fsanitize=cfi-cast-strict``: Enables :ref:`strict cast checks
+      <cfi-strictness>`.
+   -  ``-fsanitize=cfi-derived-cast``: Base-to-derived cast to the wrong
+      dynamic type. Implies ``-flto``.
+   -  ``-fsanitize=cfi-unrelated-cast``: Cast from ``void*`` or another
+      unrelated type to the wrong dynamic type. Implies ``-flto``.
    -  ``-fsanitize=cfi-vptr``: Use of an object whose vptr is of the
       wrong dynamic type. Implies ``-flto``.
    -  ``-fsanitize=enum``: Load of a value of an enumerated type which

diff --git a/include/clang/Basic/Sanitizers.def b/include/clang/Basic/Sanitizers.def
index cded80a9cf0a1539830bb13b4de0b23e8682df97..fa58a34a034bfcbdc3ca186d10ce415d80a9a7dc 100644 (file)
--- a/include/clang/Basic/Sanitizers.def
+++ b/include/clang/Basic/Sanitizers.def
@@ -79,8 +79,11 @@ SANITIZER("unsigned-integer-overflow", UnsignedIntegerOverflow)
 SANITIZER("dataflow", DataFlow)
 
 // Control Flow Integrity
+SANITIZER("cfi-cast-strict", CFICastStrict)
+SANITIZER("cfi-derived-cast", CFIDerivedCast)
+SANITIZER("cfi-unrelated-cast", CFIUnrelatedCast)
 SANITIZER("cfi-vptr", CFIVptr)
-SANITIZER_GROUP("cfi", CFI, CFIVptr)
+SANITIZER_GROUP("cfi", CFI, CFIDerivedCast | CFIUnrelatedCast | CFIVptr)
 
 // -fsanitize=undefined-trap includes sanitizers from -fsanitize=undefined
 // that can be used without runtime support, generally by providing extra

diff --git a/lib/CodeGen/CGClass.cpp b/lib/CodeGen/CGClass.cpp
index 5649708d780b0c60a363483cff6178fa0ee12151..84d6437abbecacc2b9ed6523d4e0afdb1bbb7562 100644 (file)
--- a/lib/CodeGen/CGClass.cpp
+++ b/lib/CodeGen/CGClass.cpp
@@ -2093,7 +2093,96 @@ void CodeGenFunction::EmitVTablePtrCheckForCall(const CXXMethodDecl *MD,
   if (!SanOpts.has(SanitizerKind::CFIVptr))
     return;
 
-  const CXXRecordDecl *RD = MD->getParent();
+  EmitVTablePtrCheck(MD->getParent(), VTable);
+}
+
+// If a class has a single non-virtual base and does not introduce or override
+// virtual member functions or fields, it will have the same layout as its base.
+// This function returns the least derived such class.
+//
+// Casting an instance of a base class to such a derived class is technically
+// undefined behavior, but it is a relatively common hack for introducing member
+// functions on class instances with specific properties (e.g. llvm::Operator)
+// that works under most compilers and should not have security implications, so
+// we allow it by default. It can be disabled with -fsanitize=cfi-cast-strict.
+static const CXXRecordDecl *
+LeastDerivedClassWithSameLayout(const CXXRecordDecl *RD) {
+  if (!RD->field_empty())
+    return RD;
+
+  if (RD->getNumVBases() != 0)
+    return RD;
+
+  if (RD->getNumBases() != 1)
+    return RD;
+
+  for (const CXXMethodDecl *MD : RD->methods()) {
+    if (MD->isVirtual()) {
+      // Virtual member functions are only ok if they are implicit destructors
+      // because the implicit destructor will have the same semantics as the
+      // base class's destructor if no fields are added.
+      if (isa<CXXDestructorDecl>(MD) && MD->isImplicit())
+        continue;
+      return RD;
+    }
+  }
+
+  return LeastDerivedClassWithSameLayout(
+      RD->bases_begin()->getType()->getAsCXXRecordDecl());
+}
+
+void CodeGenFunction::EmitVTablePtrCheckForCast(QualType T,
+                                                llvm::Value *Derived,
+                                                bool MayBeNull) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  auto *ClassTy = T->getAs<RecordType>();
+  if (!ClassTy)
+    return;
+
+  const CXXRecordDecl *ClassDecl = cast<CXXRecordDecl>(ClassTy->getDecl());
+
+  if (!ClassDecl->isCompleteDefinition() || !ClassDecl->isDynamicClass())
+    return;
+
+  SmallString<64> MangledName;
+  llvm::raw_svector_ostream Out(MangledName);
+  CGM.getCXXABI().getMangleContext().mangleCXXRTTI(T.getUnqualifiedType(),
+                                                   Out);
+
+  // Blacklist based on the mangled type.
+  if (CGM.getContext().getSanitizerBlacklist().isBlacklistedType(Out.str()))
+    return;
+
+  if (!SanOpts.has(SanitizerKind::CFICastStrict))
+    ClassDecl = LeastDerivedClassWithSameLayout(ClassDecl);
+
+  llvm::BasicBlock *ContBlock = 0;
+
+  if (MayBeNull) {
+    llvm::Value *DerivedNotNull =
+        Builder.CreateIsNotNull(Derived, "cast.nonnull");
+
+    llvm::BasicBlock *CheckBlock = createBasicBlock("cast.check");
+    ContBlock = createBasicBlock("cast.cont");
+
+    Builder.CreateCondBr(DerivedNotNull, CheckBlock, ContBlock);
+
+    EmitBlock(CheckBlock);
+  }
+
+  llvm::Value *VTable = GetVTablePtr(Derived, Int8PtrTy);
+  EmitVTablePtrCheck(ClassDecl, VTable);
+
+  if (MayBeNull) {
+    Builder.CreateBr(ContBlock);
+    EmitBlock(ContBlock);
+  }
+}
+
+void CodeGenFunction::EmitVTablePtrCheck(const CXXRecordDecl *RD,
+                                         llvm::Value *VTable) {
   // FIXME: Add blacklisting scheme.
   if (RD->isInStdNamespace())
     return;

diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index 4a382007813d2868f7fdb5cd0b62321e67f21343..35275e58e93f0fb046289bb7c77e98a7c53c422d 100644 (file)
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -3031,6 +3031,9 @@ LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) {
       EmitTypeCheck(TCK_DowncastReference, E->getExprLoc(),
                     Derived, E->getType());
 
+    if (SanOpts.has(SanitizerKind::CFIDerivedCast))
+      EmitVTablePtrCheckForCast(E->getType(), Derived, /*MayBeNull=*/false);
+
     return MakeAddrLValue(Derived, E->getType());
   }
   case CK_LValueBitCast: {
@@ -3040,6 +3043,10 @@ LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) {
     LValue LV = EmitLValue(E->getSubExpr());
     llvm::Value *V = Builder.CreateBitCast(LV.getAddress(),
                                            ConvertType(CE->getTypeAsWritten()));
+
+    if (SanOpts.has(SanitizerKind::CFIUnrelatedCast))
+      EmitVTablePtrCheckForCast(E->getType(), V, /*MayBeNull=*/false);
+
     return MakeAddrLValue(V, E->getType());
   }
   case CK_ObjCObjectLValueCast: {

diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp
index d9694120358761936ddb54e775769c69c4c3a0ab..e5d612ccc35a49af869890388f5de6ef0e2381f5 100644 (file)
--- a/lib/CodeGen/CGExprScalar.cpp
+++ b/lib/CodeGen/CGExprScalar.cpp
@@ -1355,6 +1355,13 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
       llvm_unreachable("wrong cast for pointers in different address spaces"
                        "(must be an address space cast)!");
     }
+
+    if (CGF.SanOpts.has(SanitizerKind::CFIUnrelatedCast)) {
+      if (auto PT = DestTy->getAs<PointerType>())
+        CGF.EmitVTablePtrCheckForCast(PT->getPointeeType(), Src,
+                                      /*MayBeNull=*/true);
+    }
+
     return Builder.CreateBitCast(Src, DstTy);
   }
   case CK_AddressSpaceConversion: {
@@ -1384,6 +1391,10 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
       CGF.EmitTypeCheck(CodeGenFunction::TCK_DowncastPointer, CE->getExprLoc(),
                         Derived, DestTy->getPointeeType());
 
+    if (CGF.SanOpts.has(SanitizerKind::CFIDerivedCast))
+      CGF.EmitVTablePtrCheckForCast(DestTy->getPointeeType(), Derived,
+                                    /*MayBeNull=*/true);
+
     return Derived;
   }
   case CK_UncheckedDerivedToBase:

diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index 1795ba3488c17e1331507848c1a46ba655ad73dc..474668abf89e2c5c7fc4850f75645ef94af026bc 100644 (file)
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -1288,10 +1288,20 @@ public:
   /// to by This.
   llvm::Value *GetVTablePtr(llvm::Value *This, llvm::Type *Ty);
 
+  /// \brief Derived is the presumed address of an object of type T after a
+  /// cast. If T is a polymorphic class type, emit a check that the virtual
+  /// table for Derived belongs to a class derived from T.
+  void EmitVTablePtrCheckForCast(QualType T, llvm::Value *Derived,
+                                 bool MayBeNull);
+
   /// EmitVTablePtrCheckForCall - Virtual method MD is being called via VTable.
   /// If vptr CFI is enabled, emit a check that VTable is valid.
   void EmitVTablePtrCheckForCall(const CXXMethodDecl *MD, llvm::Value *VTable);
 
+  /// EmitVTablePtrCheck - Emit a check that VTable is a valid virtual table for
+  /// RD using llvm.bitset.test.
+  void EmitVTablePtrCheck(const CXXRecordDecl *RD, llvm::Value *VTable);
+
   /// CanDevirtualizeMemberFunctionCalls - Checks whether virtual calls on given
   /// expr can be devirtualized.
   bool CanDevirtualizeMemberFunctionCall(const Expr *Base,

diff --git a/lib/Driver/SanitizerArgs.cpp b/lib/Driver/SanitizerArgs.cpp
index 557b62f931ce1f4be52adf8aa2df096d8997f440..8c526ce34158d187b940e2ba711bf7a96619b61e 100644 (file)
--- a/lib/Driver/SanitizerArgs.cpp
+++ b/lib/Driver/SanitizerArgs.cpp
@@ -48,7 +48,7 @@ enum SanitizeKind : uint64_t {
   RecoverableByDefault = Undefined | Integer,
   Unrecoverable = Address | Unreachable | Return,
   LegacyFsanitizeRecoverMask = Undefined | Integer,
-  NeedsLTO = CFIVptr,
+  NeedsLTO = CFIDerivedCast | CFIUnrelatedCast | CFIVptr,
 };
 }
 
@@ -150,7 +150,7 @@ bool SanitizerArgs::needsUnwindTables() const {
 }
 
 bool SanitizerArgs::needsLTO() const {
-  return hasOneOf(Sanitizers, CFIVptr);
+  return hasOneOf(Sanitizers, NeedsLTO);
 }
 
 void SanitizerArgs::clear() {

diff --git a/test/CodeGenCXX/cfi-cast.cpp b/test/CodeGenCXX/cfi-cast.cpp
new file mode 100644 (file)
index 0000000..c671bad
--- /dev/null
+++ b/test/CodeGenCXX/cfi-cast.cpp
@@ -0,0 +1,109 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-derived-cast -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-DCAST %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-unrelated-cast -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-UCAST %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-unrelated-cast,cfi-cast-strict -emit-llvm -o - %s | FileCheck -check-prefix=CHECK-UCAST-STRICT %s
+
+// In this test the main thing we are searching for is something like
+// 'metadata !"1B"' where "1B" is the mangled name of the class we are
+// casting to (or maybe its base class in non-strict mode).
+
+struct A {
+  virtual void f();
+};
+
+struct B : A {
+  virtual void f();
+};
+
+struct C : A {};
+
+// CHECK-DCAST-LABEL: define void @_Z3abpP1A
+void abp(A *a) {
+  // CHECK-DCAST: [[P:%[^ ]*]] = call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"1B")
+  // CHECK-DCAST-NEXT: br i1 [[P]], label %[[CONTBB:[^ ]*]], label %[[TRAPBB:[^ ]*]]
+
+  // CHECK-DCAST: [[TRAPBB]]
+  // CHECK-DCAST-NEXT: call void @llvm.trap()
+  // CHECK-DCAST-NEXT: unreachable
+
+  // CHECK-DCAST: [[CONTBB]]
+  // CHECK-DCAST: ret
+  static_cast<B*>(a);
+}
+
+// CHECK-DCAST-LABEL: define void @_Z3abrR1A
+void abr(A &a) {
+  // CHECK-DCAST: [[P:%[^ ]*]] = call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"1B")
+  // CHECK-DCAST-NEXT: br i1 [[P]], label %[[CONTBB:[^ ]*]], label %[[TRAPBB:[^ ]*]]
+
+  // CHECK-DCAST: [[TRAPBB]]
+  // CHECK-DCAST-NEXT: call void @llvm.trap()
+  // CHECK-DCAST-NEXT: unreachable
+
+  // CHECK-DCAST: [[CONTBB]]
+  // CHECK-DCAST: ret
+  static_cast<B&>(a);
+}
+
+// CHECK-DCAST-LABEL: define void @_Z4abrrO1A
+void abrr(A &&a) {
+  // CHECK-DCAST: [[P:%[^ ]*]] = call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"1B")
+  // CHECK-DCAST-NEXT: br i1 [[P]], label %[[CONTBB:[^ ]*]], label %[[TRAPBB:[^ ]*]]
+
+  // CHECK-DCAST: [[TRAPBB]]
+  // CHECK-DCAST-NEXT: call void @llvm.trap()
+  // CHECK-DCAST-NEXT: unreachable
+
+  // CHECK-DCAST: [[CONTBB]]
+  // CHECK-DCAST: ret
+  static_cast<B&&>(a);
+}
+
+// CHECK-UCAST-LABEL: define void @_Z3vbpPv
+void vbp(void *p) {
+  // CHECK-UCAST: [[P:%[^ ]*]] = call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"1B")
+  // CHECK-UCAST-NEXT: br i1 [[P]], label %[[CONTBB:[^ ]*]], label %[[TRAPBB:[^ ]*]]
+
+  // CHECK-UCAST: [[TRAPBB]]
+  // CHECK-UCAST-NEXT: call void @llvm.trap()
+  // CHECK-UCAST-NEXT: unreachable
+
+  // CHECK-UCAST: [[CONTBB]]
+  // CHECK-UCAST: ret
+  static_cast<B*>(p);
+}
+
+// CHECK-UCAST-LABEL: define void @_Z3vbrRc
+void vbr(char &r) {
+  // CHECK-UCAST: [[P:%[^ ]*]] = call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"1B")
+  // CHECK-UCAST-NEXT: br i1 [[P]], label %[[CONTBB:[^ ]*]], label %[[TRAPBB:[^ ]*]]
+
+  // CHECK-UCAST: [[TRAPBB]]
+  // CHECK-UCAST-NEXT: call void @llvm.trap()
+  // CHECK-UCAST-NEXT: unreachable
+
+  // CHECK-UCAST: [[CONTBB]]
+  // CHECK-UCAST: ret
+  reinterpret_cast<B&>(r);
+}
+
+// CHECK-UCAST-LABEL: define void @_Z4vbrrOc
+void vbrr(char &&r) {
+  // CHECK-UCAST: [[P:%[^ ]*]] = call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"1B")
+  // CHECK-UCAST-NEXT: br i1 [[P]], label %[[CONTBB:[^ ]*]], label %[[TRAPBB:[^ ]*]]
+
+  // CHECK-UCAST: [[TRAPBB]]
+  // CHECK-UCAST-NEXT: call void @llvm.trap()
+  // CHECK-UCAST-NEXT: unreachable
+
+  // CHECK-UCAST: [[CONTBB]]
+  // CHECK-UCAST: ret
+  reinterpret_cast<B&&>(r);
+}
+
+// CHECK-UCAST-LABEL: define void @_Z3vcpPv
+// CHECK-UCAST-STRICT-LABEL: define void @_Z3vcpPv
+void vcp(void *p) {
+  // CHECK-UCAST: [[P:%[^ ]*]] = call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"1A")
+  // CHECK-UCAST-STRICT: [[P:%[^ ]*]] = call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"1C")
+  static_cast<C*>(p);
+}

diff --git a/test/Driver/fsanitize.c b/test/Driver/fsanitize.c
index b4cbc39735001c4f8d5e6c5f13393d7da286e363..0db37d126b016fd58bae6189b63e23b5a75ec0e6 100644 (file)
--- a/test/Driver/fsanitize.c
+++ b/test/Driver/fsanitize.c
@@ -201,8 +201,13 @@
 // CHECK-FSAN-UBSAN-DARWIN: unsupported option '-fsanitize=function' for target 'x86_64-apple-darwin10'
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-vptr -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
-// CHECK-CFI: -emit-llvm-bc{{.*}}-fsanitize=cfi-vptr
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-derived-cast -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-DCAST
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-unrelated-cast -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-UCAST
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-vptr -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-VPTR
+// CHECK-CFI: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast,cfi-unrelated-cast,cfi-vptr
+// CHECK-CFI-DCAST: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast
+// CHECK-CFI-UCAST: -emit-llvm-bc{{.*}}-fsanitize=cfi-unrelated-cast
+// CHECK-CFI-VPTR: -emit-llvm-bc{{.*}}-fsanitize=cfi-vptr
 
 // RUN: %clang_cl -fsanitize=address -c -MDd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL
 // RUN: %clang_cl -fsanitize=address -c -MTd -### -- %s 2>&1 | FileCheck %s -check-prefix=CHECK-ASAN-DEBUGRTL