commit d50f1c8e3de4dd901f40f50530fe317c3cb7e82a
author Adam Langley <agl@google.com> Mon Nov 07 14:06:19 2016 -0800
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Wed Nov 09 00:52:25 2016 +0000
tree 88a19980975c880d1a43f62da898ccf3c295edc4
parent 123db570097d6373ed501cf693452dc33fc532cb

Address review comments from https://boringssl-review.googlesource.com/#/c/11920/2

In https://boringssl-review.googlesource.com/#/c/11920/2, I addressed a
number of comments but then forgot to upload the change before
submitting it. This change contains the changes that should have been
included in that commit.

Change-Id: Ib70548e791f80abf07a734e071428de8ebedb907
Reviewed-on: https://boringssl-review.googlesource.com/12160
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

crypto/x509/x509_test.cc

diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index b6ba2e6..c39d98d 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -782,7 +782,7 @@
     return false;
   }
 
-  bssl::UniquePtr<X509> root(d2i_X509_from_buffer(buf.get()));
+  bssl::UniquePtr<X509> root(X509_parse_from_buffer(buf.get()));
   if (!root) {
     return false;
   }
@@ -824,7 +824,7 @@
   }
 
   bssl::UniquePtr<X509> root_trailing_data(
-      d2i_X509_from_buffer(buf_trailing_data.get()));
+      X509_parse_from_buffer(buf_trailing_data.get()));
   if (root_trailing_data) {
     fprintf(stderr, "TestFromBuffer: trailing data was not rejected.\n");
     return false;
@@ -846,7 +846,7 @@
     return false;
   }
 
-  bssl::UniquePtr<X509> root(d2i_X509_from_buffer(buf.get()));
+  bssl::UniquePtr<X509> root(X509_parse_from_buffer(buf.get()));
   if (!root) {
     return false;
   }
@@ -857,8 +857,8 @@
 
   if (i2d_X509(root.get(), nullptr) != static_cast<long>(data_len)) {
     fprintf(stderr,
-            "TestFromBuffer: i2d_X509 gives different answer before marking as "
-            "modified.\n");
+            "TestFromBufferModified: i2d_X509 gives different answer before "
+            "marking as modified.\n");
     return false;
   }
 
@@ -866,8 +866,8 @@
 
   if (i2d_X509(root.get(), nullptr) == static_cast<long>(data_len)) {
     fprintf(stderr,
-            "TestFromBuffer: i2d_X509 gives same answer after marking as "
-            "modified.\n");
+            "TestFromBufferModified: i2d_X509 gives same answer after marking "
+            "as modified.\n");
     return false;
   }
 
@@ -887,7 +887,7 @@
     return false;
   }
 
-  bssl::UniquePtr<X509> root(d2i_X509_from_buffer(buf.get()));
+  bssl::UniquePtr<X509> root(X509_parse_from_buffer(buf.get()));
   if (!root) {
     return false;
   }
@@ -903,7 +903,13 @@
   X509 *ret = d2i_X509(&x509p, &inp, data2_len);
   if (ret != root.get()) {
     fprintf(stderr,
-            "TestFromBuffer: d2i_X509 parsed into a different object.\n");
+            "TestFromBufferReused: d2i_X509 parsed into a different object.\n");
+    return false;
+  }
+
+  if (root->buf != nullptr) {
+    fprintf(stderr,
+            "TestFromBufferReused: d2i_X509 didn't clear |buf| pointer.\n");
     return false;
   }
 
@@ -911,19 +917,18 @@
   // following will trigger a use-after-free.
   data2.reset();
 
-  const long i2d_len = i2d_X509(root.get(), nullptr);
+  uint8_t *i2d = nullptr;
+  int i2d_len = i2d_X509(root.get(), &i2d);
   if (i2d_len < 0) {
     return false;
   }
-  std::unique_ptr<uint8_t[]> i2d(new uint8_t[i2d_len]);
-  uint8_t *outp = i2d.get();
-  i2d_X509(root.get(), &outp);
+  bssl::UniquePtr<uint8_t> i2d_storage(i2d);
 
   if (!PEMToDER(&data2, &data2_len, kLeafPEM)) {
     return false;
   }
   if (i2d_len != static_cast<long>(data2_len) ||
-      memcmp(data2.get(), i2d.get(), i2d_len) != 0) {
+      memcmp(data2.get(), i2d, i2d_len) != 0) {
     fprintf(stderr, "TestFromBufferReused: i2d gave wrong result.\n");
     return false;
   }