implement rdar://5739832 - operator new should check for overflow in multiply,

causing clang to compile this code into something that correctly throws a
length error, fixing a potential integer overflow security attack:

void *test(long N) {
  return new int[N];
}

int main() {
  test(1L << 62);
}

We do this even when exceptions are disabled, because it is better for the
code to abort than for the attack to succeed.

This is heavily based on a patch that Fariborz wrote.

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

diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index fa5ac8fb14f9d271d1656276bef6ea5e43a57bba..d6a34562e55992126bb075ae59e460f3385f0a6f 100644 (file)
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -1315,10 +1315,9 @@ llvm::BasicBlock *CodeGenFunction::getTrapBB() {
 
   // If we are not optimzing, don't collapse all calls to trap in the function
   // to the same call, that way, in the debugger they can see which operation
-  // did in fact fail.  If we are optimizing, we collpase all call to trap down
+  // did in fact fail.  If we are optimizing, we collapse all calls to trap down
   // to just one per function to save on codesize.
-  if (GCO.OptimizationLevel
-      && TrapBB)
+  if (GCO.OptimizationLevel && TrapBB)
     return TrapBB;
 
   llvm::BasicBlock *Cont = 0;

diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp
index f9c4b30d0bac870223d57e9afe6211352ed3a8f7..cc34b3d75c6545cc848dc36c575fae1c85acf462 100644 (file)
--- a/lib/CodeGen/CGExprCXX.cpp
+++ b/lib/CodeGen/CGExprCXX.cpp
@@ -13,6 +13,7 @@
 
 #include "CodeGenFunction.h"
 #include "CGObjCRuntime.h"
+#include "llvm/Intrinsics.h"
 using namespace clang;
 using namespace CodeGen;
 
@@ -435,11 +436,26 @@ static llvm::Value *EmitCXXNewAllocSize(ASTContext &Context,
   // Emit the array size expression.
   NumElements = CGF.EmitScalarExpr(E->getArraySize());
   
-  // Multiply with the type size.
-  llvm::Value *V = 
-    CGF.Builder.CreateMul(NumElements, 
-                          llvm::ConstantInt::get(SizeTy, 
-                                                 TypeSize.getQuantity()));
+  // Multiply with the type size.  This multiply can overflow, e.g. in:
+  //   new double[n]
+  // where n is 2^30 on a 32-bit machine or 2^62 on a 64-bit machine.  Because
+  // of this, we need to detect the overflow and ensure that an exception is
+  // called by invoking std::__throw_length_error.
+  llvm::Value *UMulF = CGF.CGM.getIntrinsic(llvm::Intrinsic::umul_with_overflow, 
+                                            &SizeTy, 1);
+  llvm::Value *MulRes = CGF.Builder.CreateCall2(UMulF, NumElements, 
+                                                llvm::ConstantInt::get(SizeTy, 
+                                                    TypeSize.getQuantity()));
+  // Branch on the overflow bit to the overflow block, which is lazily created.
+  llvm::Value *DidOverflow = CGF.Builder.CreateExtractValue(MulRes, 1);
+  
+  llvm::BasicBlock *NormalBB = CGF.createBasicBlock("no_overflow");
+  
+  CGF.Builder.CreateCondBr(DidOverflow, CGF.getThrowLengthErrorBB(), NormalBB);
+  CGF.EmitBlock(NormalBB);
+  
+  // Get the normal result of the multiplication.
+  llvm::Value *V = CGF.Builder.CreateExtractValue(MulRes, 0);
 
   // And add the cookie padding if necessary.
   if (!CookiePadding.isZero())

diff --git a/lib/CodeGen/CodeGenFunction.cpp b/lib/CodeGen/CodeGenFunction.cpp
index eb6c4361be89c4f696872a029ed56c8df39f45cd..5e5e2a49cc2b2460bb5a4d886f9066599b1763c7 100644 (file)
--- a/lib/CodeGen/CodeGenFunction.cpp
+++ b/lib/CodeGen/CodeGenFunction.cpp
@@ -36,7 +36,7 @@ CodeGenFunction::CodeGenFunction(CodeGenModule &cgm)
     DidCallStackSave(false), UnreachableBlock(0),
     CXXThisDecl(0), CXXThisValue(0), CXXVTTDecl(0), CXXVTTValue(0),
     ConditionalBranchLevel(0), TerminateLandingPad(0), TerminateHandler(0),
-    TrapBB(0) {
+    TrapBB(0), ThrowLengthErrorBB(0) {
       
   // Get some frequently used types.
   LLVMPointerWidth = Target.getPointerWidth(0);
@@ -155,6 +155,13 @@ void CodeGenFunction::FinishFunction(SourceLocation EndLoc) {
     Builder.ClearInsertionPoint();
   }
   
+  // If someone called operator new[] and needs a throw_length_error block, emit
+  // it at the end of the function.
+  if (ThrowLengthErrorBB) {
+    EmitBlock(ThrowLengthErrorBB);
+    Builder.ClearInsertionPoint();
+  }
+  
   // Remove the AllocaInsertPt instruction, which is just a convenience for us.
   llvm::Instruction *Ptr = AllocaInsertPt;
   AllocaInsertPt = 0;
@@ -178,6 +185,37 @@ void CodeGenFunction::FinishFunction(SourceLocation EndLoc) {
     EmitDeclMetadata();
 }
 
+/// getThrowLengthErrorBB - Create a basic block that will call
+/// std::__throw_length_error to throw a std::length_error exception.
+llvm::BasicBlock *CodeGenFunction::getThrowLengthErrorBB() {
+  if (ThrowLengthErrorBB) return ThrowLengthErrorBB;
+  
+  llvm::IRBuilder<>::InsertPoint SavedIP = Builder.saveIP();
+  
+  ThrowLengthErrorBB = createBasicBlock("throw_length_error");
+  Builder.SetInsertPoint(ThrowLengthErrorBB);
+  
+  // Call to void std::__throw_length_error("length_error");
+  const llvm::Type *ResultType = Builder.getVoidTy();
+  const llvm::Type *PtrToInt8Ty = Builder.getInt8PtrTy();
+  std::vector<const llvm::Type*> ArgTys(1, PtrToInt8Ty);
+  llvm::Constant *Fn = 
+   CGM.CreateRuntimeFunction(llvm::FunctionType::get(ResultType, ArgTys, false),
+                             "_ZSt20__throw_length_errorPKc");
+    
+  llvm::Value *C = CGM.GetAddrOfConstantCString("length_error");
+  C = Builder.CreateStructGEP(C, 0, "arraydecay");
+  llvm::CallInst *TheCall = Builder.CreateCall(Fn, C);
+  TheCall->setDoesNotReturn();
+
+  Builder.CreateUnreachable();
+  
+  
+  Builder.restoreIP(SavedIP);
+  return ThrowLengthErrorBB;
+}
+
+
 /// ShouldInstrumentFunction - Return true if the current function should be
 /// instrumented with __cyg_profile_func_* calls
 bool CodeGenFunction::ShouldInstrumentFunction() {

diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index 778604b2177d890a6b36d459d65cddfc8d57af9f..1a117cab96209fea37020eec146ddefcb9ffac6d 100644 (file)
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -656,7 +656,7 @@ private:
 
   llvm::BasicBlock *TerminateLandingPad;
   llvm::BasicBlock *TerminateHandler;
-  llvm::BasicBlock *TrapBB;
+  llvm::BasicBlock *TrapBB, *ThrowLengthErrorBB;
 
 public:
   CodeGenFunction(CodeGenModule &cgm);
@@ -1542,7 +1542,11 @@ public:
 
   /// getTrapBB - Create a basic block that will call the trap intrinsic.  We'll
   /// generate a branch around the created basic block as necessary.
-  llvm::BasicBlock* getTrapBB();
+  llvm::BasicBlock *getTrapBB();
+  
+  /// getThrowLengthErrorBB - Create a basic block that will call
+  /// std::__throw_length_error to throw a std::length_error exception.
+  llvm::BasicBlock *getThrowLengthErrorBB();
   
   /// EmitCallArg - Emit a single call argument.
   RValue EmitCallArg(const Expr *E, QualType ArgType);

diff --git a/test/CodeGenCXX/operator-new.cpp b/test/CodeGenCXX/operator-new.cpp
index f718faebef000147614b8c042e7e36fc8cbc244e..f5cb2fb6c5d3d15bcafebb10e9cd125aba093266 100644 (file)
--- a/test/CodeGenCXX/operator-new.cpp
+++ b/test/CodeGenCXX/operator-new.cpp
@@ -11,7 +11,21 @@ public:
 };
 
 void f1() {
-  // CHECK-SANE: declare noalias i8* @_Znwj(
-  // CHECK-SANENOT: declare i8* @_Znwj(
+  // SANE: declare noalias i8* @_Znwj(
+  // SANENOT: declare i8* @_Znwj(
   new teste();
 }
+
+
+// rdar://5739832 - operator new should check for overflow in multiply.
+void *f2(long N) {
+  return new int[N];
+  
+// SANE: call{{.*}}@llvm.umul.with.overflow
+// SANE: extractvalue
+// SANE: br i1{{.*}}, label %throw_length_error, label %no_overflow
+
+// SANE: throw_length_error:
+// SANE: call void @_ZSt20__throw_length_errorPKc
+// SANE: unreachable
+}