Fix coverity-reported errors in ocspapitest
authorBenjamin Kaduk <bkaduk@akamai.com>
Thu, 7 Dec 2017 20:14:47 +0000 (14:14 -0600)
committerBen Kaduk <kaduk@mit.edu>
Fri, 8 Dec 2017 15:16:36 +0000 (09:16 -0600)
Avoid memory leaks in error paths, and correctly apply
parentheses to function calls in a long if-chain.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4873)

test/ocspapitest.c

index e76f724..aa477a8 100644 (file)
@@ -51,7 +51,8 @@ static OCSP_BASICRESP *make_dummy_resp(void)
     const unsigned char namestr[] = "openssl.example.com";
     unsigned char keybytes[128] = {7};
     OCSP_BASICRESP *bs = OCSP_BASICRESP_new();
     const unsigned char namestr[] = "openssl.example.com";
     unsigned char keybytes[128] = {7};
     OCSP_BASICRESP *bs = OCSP_BASICRESP_new();
-    OCSP_CERTID *cid;
+    OCSP_BASICRESP *bs_out = NULL;
+    OCSP_CERTID *cid = NULL;
     ASN1_TIME *thisupd = ASN1_TIME_set(NULL, time(NULL));
     ASN1_TIME *nextupd = ASN1_TIME_set(NULL, time(NULL) + 200);
     X509_NAME *name = X509_NAME_new();
     ASN1_TIME *thisupd = ASN1_TIME_set(NULL, time(NULL));
     ASN1_TIME *nextupd = ASN1_TIME_set(NULL, time(NULL) + 200);
     X509_NAME *name = X509_NAME_new();
@@ -60,9 +61,9 @@ static OCSP_BASICRESP *make_dummy_resp(void)
 
     if (!X509_NAME_add_entry_by_NID(name, NID_commonName, MBSTRING_ASC,
                                    namestr, -1, -1, 1)
 
     if (!X509_NAME_add_entry_by_NID(name, NID_commonName, MBSTRING_ASC,
                                    namestr, -1, -1, 1)
-        || !ASN1_BIT_STRING_set(key, keybytes, sizeof(keybytes)
-        || !ASN1_INTEGER_set_uint64(serial, (uint64_t)1)))
-        return NULL;
+        || !ASN1_BIT_STRING_set(key, keybytes, sizeof(keybytes))
+        || !ASN1_INTEGER_set_uint64(serial, (uint64_t)1))
+        goto err;
     cid = OCSP_cert_id_new(EVP_sha256(), name, key, serial);
     if (!TEST_ptr(bs)
         || !TEST_ptr(thisupd)
     cid = OCSP_cert_id_new(EVP_sha256(), name, key, serial);
     if (!TEST_ptr(bs)
         || !TEST_ptr(thisupd)
@@ -71,23 +72,28 @@ static OCSP_BASICRESP *make_dummy_resp(void)
         || !TEST_true(OCSP_basic_add1_status(bs, cid,
                                              V_OCSP_CERTSTATUS_UNKNOWN,
                                              0, NULL, thisupd, nextupd)))
         || !TEST_true(OCSP_basic_add1_status(bs, cid,
                                              V_OCSP_CERTSTATUS_UNKNOWN,
                                              0, NULL, thisupd, nextupd)))
-        return NULL;
+        goto err;
+    bs_out = bs;
+    bs = NULL;
+ err:
     ASN1_TIME_free(thisupd);
     ASN1_TIME_free(nextupd);
     ASN1_BIT_STRING_free(key);
     ASN1_INTEGER_free(serial);
     OCSP_CERTID_free(cid);
     ASN1_TIME_free(thisupd);
     ASN1_TIME_free(nextupd);
     ASN1_BIT_STRING_free(key);
     ASN1_INTEGER_free(serial);
     OCSP_CERTID_free(cid);
+    OCSP_BASICRESP_free(bs);
     X509_NAME_free(name);
     X509_NAME_free(name);
-    return bs;
+    return bs_out;
 }
 
 #ifndef OPENSSL_NO_OCSP
 static int test_resp_signer(void)
 {
 }
 
 #ifndef OPENSSL_NO_OCSP
 static int test_resp_signer(void)
 {
-    OCSP_BASICRESP *bs;
+    OCSP_BASICRESP *bs = NULL;
     X509 *signer = NULL, *tmp;
     EVP_PKEY *key = NULL;
     X509 *signer = NULL, *tmp;
     EVP_PKEY *key = NULL;
-    STACK_OF(X509) *extra_certs;
+    STACK_OF(X509) *extra_certs = NULL;
+    int ret = 0;
 
     /*
      * Test a response with no certs at all; get the signer from the
 
     /*
      * Test a response with no certs at all; get the signer from the
@@ -101,10 +107,10 @@ static int test_resp_signer(void)
         || !TEST_true(sk_X509_push(extra_certs, signer))
         || !TEST_true(OCSP_basic_sign(bs, signer, key, EVP_sha1(),
                                       NULL, OCSP_NOCERTS)))
         || !TEST_true(sk_X509_push(extra_certs, signer))
         || !TEST_true(OCSP_basic_sign(bs, signer, key, EVP_sha1(),
                                       NULL, OCSP_NOCERTS)))
-        return 0;
+        goto err;
     if (!TEST_true(OCSP_resp_get0_signer(bs, &tmp, extra_certs))
         || !TEST_int_eq(X509_cmp(tmp, signer), 0))
     if (!TEST_true(OCSP_resp_get0_signer(bs, &tmp, extra_certs))
         || !TEST_int_eq(X509_cmp(tmp, signer), 0))
-        return 0;
+        goto err;
     OCSP_BASICRESP_free(bs);
 
     /* Do it again but include the signer cert */
     OCSP_BASICRESP_free(bs);
 
     /* Do it again but include the signer cert */
@@ -113,15 +119,17 @@ static int test_resp_signer(void)
     if (!TEST_ptr(bs)
         || !TEST_true(OCSP_basic_sign(bs, signer, key, EVP_sha1(),
                                       NULL, 0)))
     if (!TEST_ptr(bs)
         || !TEST_true(OCSP_basic_sign(bs, signer, key, EVP_sha1(),
                                       NULL, 0)))
-        return 0;
+        goto err;
     if (!TEST_true(OCSP_resp_get0_signer(bs, &tmp, NULL))
         || !TEST_int_eq(X509_cmp(tmp, signer), 0))
     if (!TEST_true(OCSP_resp_get0_signer(bs, &tmp, NULL))
         || !TEST_int_eq(X509_cmp(tmp, signer), 0))
-        return 0;
+        goto err;
+    ret = 1;
+ err:
     OCSP_BASICRESP_free(bs);
     sk_X509_free(extra_certs);
     X509_free(signer);
     EVP_PKEY_free(key);
     OCSP_BASICRESP_free(bs);
     sk_X509_free(extra_certs);
     X509_free(signer);
     EVP_PKEY_free(key);
-    return 1;
+    return ret;
 }
 #endif
 
 }
 #endif