commit 7f323feefed7754f293f6c8ab219a827c5f701bb
author Dan Field <dnfield@google.com> Wed May 15 17:53:49 2019 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Wed May 15 17:53:49 2019 +0000
tree e4ad7527a9376b4803c4214b3fd0a8c373dcf904
parent 1c469617c513c60986e60e13b8113377a1e92905

[dart:io] Fix iOS https leaks

Add move constructor and equality operators to
ScopedCFType, add specialized destructor for arrays.

Add release to ScopedCFType.

Use ScopedCFType for SecCertificateRef.

Bug: https://github.com/dart-lang/sdk/issues/36935
Change-Id: I7b40f23fb8f0d7ab0835587e040362f883f86bd5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/102560
Reviewed-by: Zach Anderson <zra@google.com>
Auto-Submit: Dan Field <dnfield@google.com>
Commit-Queue: Zach Anderson <zra@google.com>

runtime/bin/security_context_macos.cc

diff --git a/runtime/bin/security_context_macos.cc b/runtime/bin/security_context_macos.cc
index 2964d67..a9afb58 100644
--- a/runtime/bin/security_context_macos.cc
+++ b/runtime/bin/security_context_macos.cc
@@ -38,8 +38,18 @@
   T* ptr() { return &obj_; }
   const T get() const { return obj_; }
 
+  DART_WARN_UNUSED_RESULT T release() {
+    T temp = obj_;
+    obj_ = NULL;
+    return temp;
+  }
+
   void set(T obj) { obj_ = obj; }
 
+  bool operator==(T other) { return other == get(); }
+
+  bool operator!=(T other) { return other != get(); }
+
  private:
   T obj_;
 
@@ -47,6 +57,20 @@
   DISALLOW_COPY_AND_ASSIGN(ScopedCFType);
 };
 
+static void releaseObjects(const void* val, void* context) {
+  CFRelease(val);
+}
+
+template <>
+ScopedCFType<CFMutableArrayRef>::~ScopedCFType() {
+  if (obj_ != NULL) {
+    CFIndex count = 0;
+    CFArrayApplyFunction(obj_, CFRangeMake(0, CFArrayGetCount(obj_)),
+                         releaseObjects, &count);
+    CFRelease(obj_);
+  }
+}
+
 typedef ScopedCFType<CFMutableArrayRef> ScopedCFMutableArrayRef;
 typedef ScopedCFType<CFDataRef> ScopedCFDataRef;
 typedef ScopedCFType<CFStringRef> ScopedCFStringRef;
@@ -60,10 +84,10 @@
   }
   int length = i2d_X509(cert, NULL);
   if (length < 0) {
-    return 0;
+    return NULL;
   }
-  auto deb_cert = std::unique_ptr<unsigned char[]>(new unsigned char[length]);
-  unsigned char* temp = deb_cert.get();
+  auto deb_cert = std::make_unique<unsigned char[]>(length);
+  auto temp = deb_cert.get();
   if (i2d_X509(cert, &temp) != length) {
     return NULL;
   }
@@ -72,12 +96,7 @@
   // Implementation here:
   // https://opensource.apple.com/source/libsecurity_keychain/libsecurity_keychain-55050.2/lib/SecCertificate.cpp.auto.html
   ScopedCFDataRef cert_buf(CFDataCreate(NULL, deb_cert.get(), length));
-  SecCertificateRef auth_cert =
-      SecCertificateCreateWithData(NULL, cert_buf.get());
-  if (auth_cert == NULL) {
-    return NULL;
-  }
-  return auth_cert;
+  return SecCertificateCreateWithData(NULL, cert_buf.get());
 }
 
 static int CertificateVerificationCallback(X509_STORE_CTX* ctx, void* arg) {
@@ -93,11 +112,11 @@
     cert_chain.set(CFArrayCreateMutable(NULL, num_certs, NULL));
     X509* ca;
     while ((ca = sk_X509_shift(user_provided_certs)) != NULL) {
-      SecCertificateRef cert = CreateSecCertificateFromX509(ca);
+      ScopedSecCertificateRef cert(CreateSecCertificateFromX509(ca));
       if (cert == NULL) {
         return ctx->verify_cb(0, ctx);
       }
-      CFArrayAppendValue(cert_chain.get(), cert);
+      CFArrayAppendValue(cert_chain.get(), cert.release());
       ++current_cert;
 
       if (current_cert == num_certs) {
@@ -115,11 +134,11 @@
   if (store->objs != NULL) {
     for (uintptr_t i = 0; i < sk_X509_OBJECT_num(store->objs); ++i) {
       X509* ca = sk_X509_OBJECT_value(store->objs, i)->data.x509;
-      SecCertificateRef cert = CreateSecCertificateFromX509(ca);
+      ScopedSecCertificateRef cert(CreateSecCertificateFromX509(ca));
       if (cert == NULL) {
         return ctx->verify_cb(0, ctx);
       }
-      CFArrayAppendValue(trusted_certs.get(), cert);
+      CFArrayAppendValue(trusted_certs.get(), cert.release());
     }
   }