commit 39d7ee9c8262d9cd3338735bf3e95649857375e5
author Theo Buehler <theorbuehler@gmail.com> Thu Oct 19 06:50:23 2023 +0200
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Oct 19 16:49:34 2023 +0000
tree f746d56d5ee593981eef09d7851b7317df42ed0d
parent 9e6144382ca4752591910b38b71a3301d97999df

Error check X509_ALGOR_set0()

If |param_type| is different from |V_ASN1_UNDEF|, there will usually
be a call to |ASN1_TYPE_new| which allocates and can thus fail. The
result of a failure is that |pval| will leak, which is the case in
both callers in the RSA-PSS code.

This changeset leaves out the call in |X509_ALGOR_set_md|, which
is a void function. This could be fixed in three ways: change its
signature to allow error checking, call |X509_ALGOR_set0| up front
to preallocate, or inline the function in its only internal caller
and remove it from the public API.

Change-Id: I25ed3593947f9ee58208b980a95730d37789c9e1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63585
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/x509/algorithm.c

diff --git a/crypto/x509/algorithm.c b/crypto/x509/algorithm.c
index 16235ee..2d3f4d3 100644
--- a/crypto/x509/algorithm.c
+++ b/crypto/x509/algorithm.c
@@ -116,8 +116,7 @@
   // it.
   int paramtype =
       (EVP_PKEY_id(pkey) == EVP_PKEY_RSA) ? V_ASN1_NULL : V_ASN1_UNDEF;
-  X509_ALGOR_set0(algor, OBJ_nid2obj(sign_nid), paramtype, NULL);
-  return 1;
+  return X509_ALGOR_set0(algor, OBJ_nid2obj(sign_nid), paramtype, NULL);
 }
 
 int x509_digest_verify_init(EVP_MD_CTX *ctx, const X509_ALGOR *sigalg,

crypto/x509/rsa_pss.c

diff --git a/crypto/x509/rsa_pss.c b/crypto/x509/rsa_pss.c
index 9e69663..5974bfa 100644
--- a/crypto/x509/rsa_pss.c
+++ b/crypto/x509/rsa_pss.c
@@ -145,7 +145,9 @@
   if (!*palg) {
     goto err;
   }
-  X509_ALGOR_set0(*palg, OBJ_nid2obj(NID_mgf1), V_ASN1_SEQUENCE, stmp);
+  if (!X509_ALGOR_set0(*palg, OBJ_nid2obj(NID_mgf1), V_ASN1_SEQUENCE, stmp)) {
+    goto err;
+  }
   stmp = NULL;
 
 err:
@@ -235,7 +237,9 @@
     goto err;
   }
 
-  X509_ALGOR_set0(algor, OBJ_nid2obj(NID_rsassaPss), V_ASN1_SEQUENCE, os);
+  if (!X509_ALGOR_set0(algor, OBJ_nid2obj(NID_rsassaPss), V_ASN1_SEQUENCE, os)) {
+    goto err;
+  }
   os = NULL;
   ret = 1;