commit 29723828eca43b2dfa5fe476edc4e01ca8072d2a
author David Benjamin <davidben@google.com> Wed Nov 23 12:30:36 2022 -0500
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Dec 07 17:28:38 2022 +0000
tree e411d71a6c7d9b089c4886718b132b604c6ff781
parent 28f96c2686459add7acedcd97cb841030bdda019

Rewrite i2o_ECPublicKey with CBB_finish_i2d.

Less code, and internally handles overflows. (Although this one cannot
overflow.)

Bug: 516
Change-Id: I3c2718075689d2815a43534a578a323c52787223
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55452
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/ec_extra/ec_asn1.c

diff --git a/crypto/ec_extra/ec_asn1.c b/crypto/ec_extra/ec_asn1.c
index 43132a6..ee75165 100644
--- a/crypto/ec_extra/ec_asn1.c
+++ b/crypto/ec_extra/ec_asn1.c
@@ -518,42 +518,18 @@
 }
 
 int i2o_ECPublicKey(const EC_KEY *key, uint8_t **outp) {
-  size_t buf_len = 0;
-  int new_buffer = 0;
-
   if (key == NULL) {
     OPENSSL_PUT_ERROR(EC, ERR_R_PASSED_NULL_PARAMETER);
     return 0;
   }
-
-  buf_len = EC_POINT_point2oct(key->group, key->pub_key, key->conv_form, NULL,
-                               0, NULL);
-
-  if (outp == NULL || buf_len == 0) {
-    // out == NULL => just return the length of the octet string
-    return buf_len;
+  CBB cbb;
+  if (!CBB_init(&cbb, 0) ||  //
+      !EC_POINT_point2cbb(&cbb, key->group, key->pub_key, key->conv_form,
+                          NULL)) {
+    CBB_cleanup(&cbb);
+    return -1;
   }
-
-  if (*outp == NULL) {
-    *outp = OPENSSL_malloc(buf_len);
-    if (*outp == NULL) {
-      OPENSSL_PUT_ERROR(EC, ERR_R_MALLOC_FAILURE);
-      return 0;
-    }
-    new_buffer = 1;
-  }
-  if (!EC_POINT_point2oct(key->group, key->pub_key, key->conv_form, *outp,
-                          buf_len, NULL)) {
-    OPENSSL_PUT_ERROR(EC, ERR_R_EC_LIB);
-    if (new_buffer) {
-      OPENSSL_free(*outp);
-      *outp = NULL;
-    }
-    return 0;
-  }
-
-  if (!new_buffer) {
-    *outp += buf_len;
-  }
-  return buf_len;
+  int ret = CBB_finish_i2d(&cbb, outp);
+  // Historically, this function used the wrong return value on error.
+  return ret > 0 ? ret : 0;
 }

include/openssl/ec_key.h

diff --git a/include/openssl/ec_key.h b/include/openssl/ec_key.h
index ee9c9f0..00986cf 100644
--- a/include/openssl/ec_key.h
+++ b/include/openssl/ec_key.h
@@ -361,7 +361,7 @@
                                        long len);
 
 // i2o_ECPublicKey marshals an EC point from |key|, as described in
-// |i2d_SAMPLE|.
+// |i2d_SAMPLE|, except it returns zero on error instead of a negative value.
 //
 // Use |EC_POINT_point2cbb| instead.
 OPENSSL_EXPORT int i2o_ECPublicKey(const EC_KEY *key, unsigned char **outp);