Remove typedef and check sk_push return value in openssl(1) cms

- Remove typedef and use 'struct cms_key_param' instead
- Check return value of sk_X509_push and sk_OPENSSL_STRING_push
- Add a blank line to separate variable declarations from code

comments from jsing@

1 files changed, 33 insertions, 23 deletions

diff --git a/src/usr.bin/openssl/cms.c b/src/usr.bin/openssl/cms.c
index 71e52fe52c..a4deb06188 100644
--- a/src/usr.bin/openssl/cms.c
+++ b/src/usr.bin/openssl/cms.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: cms.c,v 1.14 2019/11/18 11:34:41 inoguchi Exp $ */
+/* $OpenBSD: cms.c,v 1.15 2019/11/18 12:43:27 inoguchi Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project.
  */
@@ -99,12 +99,10 @@ static int cms_set_pkey_param(EVP_PKEY_CTX *pctx,
 
 int verify_err = 0;
 
-typedef struct cms_key_param_st cms_key_param;
-
-struct cms_key_param_st {
+struct cms_key_param {
         int idx;
         STACK_OF(OPENSSL_STRING) *param;
-        cms_key_param *next;
+        struct cms_key_param *next;
 };
 
 int
@@ -142,7 +140,7 @@ cms_main(int argc, char **argv)
         unsigned char *pwri_pass = NULL, *pwri_tmp = NULL;
         size_t secret_keylen = 0, secret_keyidlen = 0;
 
-        cms_key_param *key_first = NULL, *key_param = NULL;
+        struct cms_key_param *key_first = NULL, *key_param = NULL;
 
         ASN1_OBJECT *econtent_type = NULL;
 
@@ -278,14 +276,16 @@ cms_main(int argc, char **argv)
                         args++;
                         if (!rr_from)
                                 rr_from = sk_OPENSSL_STRING_new_null();
-                        sk_OPENSSL_STRING_push(rr_from, *args);
+                        if (!sk_OPENSSL_STRING_push(rr_from, *args))
+                                goto end;
                 } else if (!strcmp(*args, "-receipt_request_to")) {
                         if (!args[1])
                                 goto argerr;
                         args++;
                         if (!rr_to)
                                 rr_to = sk_OPENSSL_STRING_new_null();
-                        sk_OPENSSL_STRING_push(rr_to, *args);
+                        if (!sk_OPENSSL_STRING_push(rr_to, *args))
+                                goto end;
                 } else if (!strcmp(*args, "-print")) {
                         noout = 1;
                         print = 1;
@@ -351,12 +351,14 @@ cms_main(int argc, char **argv)
                                 if (!sksigners)
                                         sksigners =
                                             sk_OPENSSL_STRING_new_null();
-                                sk_OPENSSL_STRING_push(sksigners, signerfile);
+                                if (!sk_OPENSSL_STRING_push(sksigners, signerfile))
+                                        goto end;
                                 if (!keyfile)
                                         keyfile = signerfile;
                                 if (!skkeys)
                                         skkeys = sk_OPENSSL_STRING_new_null();
-                                sk_OPENSSL_STRING_push(skkeys, keyfile);
+                                if (!sk_OPENSSL_STRING_push(skkeys, keyfile))
+                                        goto end;
                                 keyfile = NULL;
                         }
                         signerfile = *++args;
@@ -371,7 +373,8 @@ cms_main(int argc, char **argv)
                                     NULL, "recipient certificate file");
                                 if (cert == NULL)
                                         goto end;
-                                sk_X509_push(encerts, cert);
+                                if (!sk_X509_push(encerts, cert))
+                                        goto end;
                                 cert = NULL;
                         } else {
                                 recipfile = *++args;
@@ -402,11 +405,13 @@ cms_main(int argc, char **argv)
                                 if (!sksigners)
                                         sksigners =
                                             sk_OPENSSL_STRING_new_null();
-                                sk_OPENSSL_STRING_push(sksigners, signerfile);
+                                if (!sk_OPENSSL_STRING_push(sksigners, signerfile))
+                                        goto end;
                                 signerfile = NULL;
                                 if (!skkeys)
                                         skkeys = sk_OPENSSL_STRING_new_null();
-                                sk_OPENSSL_STRING_push(skkeys, keyfile);
+                                if (!sk_OPENSSL_STRING_push(skkeys, keyfile))
+                                        goto end;
                         }
                         keyfile = *++args;
                 } else if (!strcmp(*args, "-keyform")) {
@@ -431,8 +436,8 @@ cms_main(int argc, char **argv)
                                 goto argerr;
                         }
                         if (key_param == NULL || key_param->idx != keyidx) {
-                                cms_key_param *nparam;
-                                if ((nparam = malloc(sizeof(cms_key_param))) == NULL)
+                                struct cms_key_param *nparam;
+                                if ((nparam = malloc(sizeof(struct cms_key_param))) == NULL)
                                         goto end;
                                 nparam->idx = keyidx;
                                 if ((nparam->param = sk_OPENSSL_STRING_new_null()) == NULL)
@@ -444,7 +449,8 @@ cms_main(int argc, char **argv)
                                         key_param->next = nparam;
                                 key_param = nparam;
                         }
-                        sk_OPENSSL_STRING_push(key_param->param, *++args);
+                        if (!sk_OPENSSL_STRING_push(key_param->param, *++args))
+                                goto end;
                 } else if (!strcmp(*args, "-rctform")) {
                         if (!args[1])
                                 goto argerr;
@@ -509,12 +515,14 @@ cms_main(int argc, char **argv)
                 if (signerfile) {
                         if (!sksigners)
                                 sksigners = sk_OPENSSL_STRING_new_null();
-                        sk_OPENSSL_STRING_push(sksigners, signerfile);
+                        if (!sk_OPENSSL_STRING_push(sksigners, signerfile))
+                                goto end;
                         if (!skkeys)
                                 skkeys = sk_OPENSSL_STRING_new_null();
                         if (!keyfile)
                                 keyfile = signerfile;
-                        sk_OPENSSL_STRING_push(skkeys, keyfile);
+                        if (!sk_OPENSSL_STRING_push(skkeys, keyfile))
+                                goto end;
                 }
                 if (!sksigners) {
                         BIO_printf(bio_err,
@@ -580,7 +588,7 @@ cms_main(int argc, char **argv)
                 BIO_printf(bio_err, "-inform arg    input format SMIME (default), PEM or DER\n");
                 BIO_printf(bio_err, "-inkey file    input private key (if not signer or recipient)\n");
                 BIO_printf(bio_err, "-keyform arg   input private key format (PEM)\n");
-                BIO_printf (bio_err, "-keyopt nm:v  set public key parameters\n");
+                BIO_printf(bio_err, "-keyopt nm:v   set public key parameters\n");
                 BIO_printf(bio_err, "-out file      output file\n");
                 BIO_printf(bio_err, "-outform arg   output format SMIME (default), PEM or DER\n");
                 BIO_printf(bio_err, "-content file  supply or override content for detached signature\n");
@@ -641,7 +649,8 @@ cms_main(int argc, char **argv)
                         if (!(cert = load_cert(bio_err, *args, FORMAT_PEM,
                             NULL, "recipient certificate file")))
                                 goto end;
-                        sk_X509_push(encerts, cert);
+                        if (!sk_X509_push(encerts, cert))
+                                goto end;
                         cert = NULL;
                         args++;
                 }
@@ -786,7 +795,7 @@ cms_main(int argc, char **argv)
                         goto end;
                 for (i = 0; i < sk_X509_num(encerts); i++) {
                         CMS_RecipientInfo *ri;
-                        cms_key_param *kparam;
+                        struct cms_key_param *kparam;
                         int tflags = flags;
                         X509 *x = sk_X509_value(encerts, i);
                         for (kparam = key_first; kparam; kparam = kparam->next) {
@@ -877,7 +886,7 @@ cms_main(int argc, char **argv)
                         flags |= CMS_REUSE_DIGEST;
                 for (i = 0; i < sk_OPENSSL_STRING_num(sksigners); i++) {
                         CMS_SignerInfo *si;
-                        cms_key_param *kparam;
+                        struct cms_key_param *kparam;
                         int tflags = flags;
                         signerfile = sk_OPENSSL_STRING_value(sksigners, i);
                         keyfile = sk_OPENSSL_STRING_value(skkeys, i);
@@ -1048,7 +1057,7 @@ cms_main(int argc, char **argv)
         sk_OPENSSL_STRING_free(rr_to);
         sk_OPENSSL_STRING_free(rr_from);
         for (key_param = key_first; key_param;) {
-                cms_key_param *tparam;
+                struct cms_key_param *tparam;
                 sk_OPENSSL_STRING_free(key_param->param);
                 tparam = key_param->next;
                 free(key_param);
@@ -1240,6 +1249,7 @@ cms_set_pkey_param(EVP_PKEY_CTX *pctx, STACK_OF(OPENSSL_STRING) *param)
 {
         char *keyopt;
         int i;
+
         if (sk_OPENSSL_STRING_num(param) <= 0)
                 return 1;
         for (i = 0; i < sk_OPENSSL_STRING_num(param); i++) {