From 9ba9d81b1c1645ed56a79b841e3fb63e5cbd7617 Mon Sep 17 00:00:00 2001 From: "Dr. Matthias St. Pierre" Date: Sun, 9 Sep 2018 16:19:19 +0200 Subject: [PATCH] test/dhtest.c: fix resource leak Reported by Coverity Scan (CID 1439136) [extended tests] Reviewed-by: Nicola Tuveri Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/7155) --- test/dhtest.c | 68 +++++++++++++++++++++++++++++---------------------- 1 file changed, 39 insertions(+), 29 deletions(-) diff --git a/test/dhtest.c b/test/dhtest.c index 956d081f04..5b2fd6795f 100644 --- a/test/dhtest.c +++ b/test/dhtest.c @@ -26,10 +26,10 @@ static int cb(int p, int n, BN_GENCB *arg); static int dh_test(void) { - DH *dh; - BIGNUM *p, *q, *g; + DH *dh = NULL; + BIGNUM *p = NULL, *q = NULL, *g = NULL; const BIGNUM *p2, *q2, *g2; - BIGNUM *priv_key; + BIGNUM *priv_key = NULL; const BIGNUM *pub_key2, *priv_key2; BN_GENCB *_cb = NULL; DH *a = NULL; @@ -49,7 +49,7 @@ static int dh_test(void) || !TEST_ptr(q = BN_new()) || !TEST_ptr(g = BN_new()) || !TEST_ptr(priv_key = BN_new())) - goto err; + goto err1; /* * I) basic tests @@ -60,40 +60,40 @@ static int dh_test(void) || !TEST_true(BN_set_word(q, 2039L)) || !TEST_true(BN_set_word(g, 3L)) || !TEST_true(DH_set0_pqg(dh, p, q, g))) - goto err; + goto err1; /* test the combined getter for p, q, and g */ DH_get0_pqg(dh, &p2, &q2, &g2); if (!TEST_ptr_eq(p2, p) || !TEST_ptr_eq(q2, q) || !TEST_ptr_eq(g2, g)) - goto err; + goto err2; /* test the simple getters for p, q, and g */ if (!TEST_ptr_eq(DH_get0_p(dh), p2) || !TEST_ptr_eq(DH_get0_q(dh), q2) || !TEST_ptr_eq(DH_get0_g(dh), g2)) - goto err; + goto err2; /* set the private key only*/ if (!TEST_true(BN_set_word(priv_key, 1234L)) || !TEST_true(DH_set0_key(dh, NULL, priv_key))) - goto err; + goto err2; /* test the combined getter for pub_key and priv_key */ DH_get0_key(dh, &pub_key2, &priv_key2); if (!TEST_ptr_eq(pub_key2, NULL) || !TEST_ptr_eq(priv_key2, priv_key)) - goto err; + goto err3; /* test the simple getters for pub_key and priv_key */ if (!TEST_ptr_eq(DH_get0_pub_key(dh), pub_key2) || !TEST_ptr_eq(DH_get0_priv_key(dh), priv_key2)) - goto err; + goto err3; /* now generate a key pair ... */ if (!DH_generate_key(dh)) - goto err; + goto err3; /* ... and check whether the private key was reused: */ @@ -101,14 +101,14 @@ static int dh_test(void) DH_get0_key(dh, &pub_key2, &priv_key2); if (!TEST_ptr(pub_key2) || !TEST_ptr_eq(priv_key2, priv_key)) - goto err; + goto err3; /* test it the simple getters for pub_key and priv_key */ if (!TEST_ptr_eq(DH_get0_pub_key(dh), pub_key2) || !TEST_ptr_eq(DH_get0_priv_key(dh), priv_key2)) - goto err; + goto err3; - /* check whether the public key was calculated correclty */ + /* check whether the public key was calculated correctly */ TEST_uint_eq(BN_get_word(pub_key2), 3331L); /* @@ -117,32 +117,32 @@ static int dh_test(void) /* generate a DH group ... */ if (!TEST_ptr(_cb = BN_GENCB_new())) - goto err; + goto err3; BN_GENCB_set(_cb, &cb, NULL); if (!TEST_ptr(a = DH_new()) || !TEST_true(DH_generate_parameters_ex(a, 64, DH_GENERATOR_5, _cb))) - goto err; + goto err3; /* ... and check whether it is valid */ if (!DH_check(a, &i)) - goto err; + goto err3; if (!TEST_false(i & DH_CHECK_P_NOT_PRIME) || !TEST_false(i & DH_CHECK_P_NOT_SAFE_PRIME) || !TEST_false(i & DH_UNABLE_TO_CHECK_GENERATOR) || !TEST_false(i & DH_NOT_SUITABLE_GENERATOR)) - goto err; + goto err3; DH_get0_pqg(a, &ap, NULL, &ag); /* now create another copy of the DH group for the peer */ if (!TEST_ptr(b = DH_new())) - goto err; + goto err3; if (!TEST_ptr(bp = BN_dup(ap)) || !TEST_ptr(bg = BN_dup(ag)) || !TEST_true(DH_set0_pqg(b, bp, NULL, bg))) - goto err; + goto err3; bp = bg = NULL; /* @@ -150,43 +150,53 @@ static int dh_test(void) */ if (!DH_generate_key(a)) - goto err; + goto err3; DH_get0_key(a, &apub_key, NULL); if (!DH_generate_key(b)) - goto err; + goto err3; DH_get0_key(b, &bpub_key, &bpriv_key); /* Also test with a private-key-only copy of |b|. */ if (!TEST_ptr(c = DHparams_dup(b)) || !TEST_ptr(cpriv_key = BN_dup(bpriv_key)) || !TEST_true(DH_set0_key(c, NULL, cpriv_key))) - goto err; + goto err3; cpriv_key = NULL; alen = DH_size(a); if (!TEST_ptr(abuf = OPENSSL_malloc(alen)) || !TEST_true((aout = DH_compute_key(abuf, bpub_key, a)) != -1)) - goto err; + goto err3; blen = DH_size(b); if (!TEST_ptr(bbuf = OPENSSL_malloc(blen)) || !TEST_true((bout = DH_compute_key(bbuf, apub_key, b)) != -1)) - goto err; + goto err3; clen = DH_size(c); if (!TEST_ptr(cbuf = OPENSSL_malloc(clen)) || !TEST_true((cout = DH_compute_key(cbuf, apub_key, c)) != -1)) - goto err; + goto err3; if (!TEST_true(aout >= 4) || !TEST_mem_eq(abuf, aout, bbuf, bout) || !TEST_mem_eq(abuf, aout, cbuf, cout)) - goto err; + goto err3; ret = 1; - - err: + goto success; + + err1: + /* an error occurred before p,q,g were assigned to dh */ + BN_free(p); + BN_free(q); + BN_free(g); + err2: + /* an error occured before priv_key was assigned to dh */ + BN_free(priv_key); + err3: + success: OPENSSL_free(abuf); OPENSSL_free(bbuf); OPENSSL_free(cbuf); -- 2.34.1