Index: cfe/trunk/docs/ControlFlowIntegrity.rst =================================================================== --- cfe/trunk/docs/ControlFlowIntegrity.rst +++ cfe/trunk/docs/ControlFlowIntegrity.rst @@ -58,6 +58,60 @@ 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 ------ Index: cfe/trunk/docs/UsersManual.rst =================================================================== --- cfe/trunk/docs/UsersManual.rst +++ cfe/trunk/docs/UsersManual.rst @@ -968,6 +968,12 @@ ``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 + `. + - ``-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 Index: cfe/trunk/include/clang/Basic/Sanitizers.def =================================================================== --- cfe/trunk/include/clang/Basic/Sanitizers.def +++ cfe/trunk/include/clang/Basic/Sanitizers.def @@ -79,8 +79,11 @@ 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 Index: cfe/trunk/lib/CodeGen/CGClass.cpp =================================================================== --- cfe/trunk/lib/CodeGen/CGClass.cpp +++ cfe/trunk/lib/CodeGen/CGClass.cpp @@ -2093,7 +2093,96 @@ 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(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(); + if (!ClassTy) + return; + + const CXXRecordDecl *ClassDecl = cast(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; Index: cfe/trunk/lib/CodeGen/CGExpr.cpp =================================================================== --- cfe/trunk/lib/CodeGen/CGExpr.cpp +++ cfe/trunk/lib/CodeGen/CGExpr.cpp @@ -3031,6 +3031,9 @@ 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 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: { Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp =================================================================== --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp @@ -1355,6 +1355,13 @@ 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()) + CGF.EmitVTablePtrCheckForCast(PT->getPointeeType(), Src, + /*MayBeNull=*/true); + } + return Builder.CreateBitCast(Src, DstTy); } case CK_AddressSpaceConversion: { @@ -1384,6 +1391,10 @@ 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: Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h =================================================================== --- cfe/trunk/lib/CodeGen/CodeGenFunction.h +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h @@ -1288,10 +1288,20 @@ /// 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, Index: cfe/trunk/lib/Driver/SanitizerArgs.cpp =================================================================== --- cfe/trunk/lib/Driver/SanitizerArgs.cpp +++ cfe/trunk/lib/Driver/SanitizerArgs.cpp @@ -48,7 +48,7 @@ RecoverableByDefault = Undefined | Integer, Unrecoverable = Address | Unreachable | Return, LegacyFsanitizeRecoverMask = Undefined | Integer, - NeedsLTO = CFIVptr, + NeedsLTO = CFIDerivedCast | CFIUnrelatedCast | CFIVptr, }; } @@ -150,7 +150,7 @@ } bool SanitizerArgs::needsLTO() const { - return hasOneOf(Sanitizers, CFIVptr); + return hasOneOf(Sanitizers, NeedsLTO); } void SanitizerArgs::clear() { Index: cfe/trunk/test/CodeGenCXX/cfi-cast.cpp =================================================================== --- cfe/trunk/test/CodeGenCXX/cfi-cast.cpp +++ cfe/trunk/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(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(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(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(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(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(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(p); +} Index: cfe/trunk/test/Driver/fsanitize.c =================================================================== --- cfe/trunk/test/Driver/fsanitize.c +++ cfe/trunk/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