Revamp get_dss_public_key()

* Simplify logic by having incremental allocation and only free on error on one place.
* Add error checking on all OpenSSL calls.
* Make it explicit when you need to be careful with non-reference counted pointers.
  - set0 calls will use the pointer values without ref counting.
  - On success, set pointers to NULL to avoid double frees since the struct
    is now responsible for freeing the resources.

1 files changed, 46 insertions, 18 deletions

diff --git a/lib/crypto/c_src/dss.c b/lib/crypto/c_src/dss.c
index 934b33d87c..9bf8eb3ce0 100644
--- a/lib/crypto/c_src/dss.c
+++ b/lib/crypto/c_src/dss.c
@@ -94,23 +94,51 @@ int get_dss_public_key(ErlNifEnv* env, ERL_NIF_TERM key, DSA *dsa)
     ERL_NIF_TERM head, tail;
     BIGNUM *dsa_p = NULL, *dsa_q = NULL, *dsa_g = NULL, *dsa_y = NULL;
 
-    if (!enif_get_list_cell(env, key, &head, &tail)
-	|| !get_bn_from_bin(env, head, &dsa_p)
-	|| !enif_get_list_cell(env, tail, &head, &tail)
-	|| !get_bn_from_bin(env, head, &dsa_q)
-	|| !enif_get_list_cell(env, tail, &head, &tail)
-	|| !get_bn_from_bin(env, head, &dsa_g)
-	|| !enif_get_list_cell(env, tail, &head, &tail)
-	|| !get_bn_from_bin(env, head, &dsa_y)
-	|| !enif_is_empty_list(env,tail)) {
-	if (dsa_p) BN_free(dsa_p);
-	if (dsa_q) BN_free(dsa_q);
-	if (dsa_g) BN_free(dsa_g);
-	if (dsa_y) BN_free(dsa_y);
-	return 0;
-    }
-
-    DSA_set0_pqg(dsa, dsa_p, dsa_q, dsa_g);
-    DSA_set0_key(dsa, dsa_y, NULL);
+    if (!enif_get_list_cell(env, key, &head, &tail))
+        goto err;
+    if (!get_bn_from_bin(env, head, &dsa_p))
+        goto err;
+
+    if (!enif_get_list_cell(env, tail, &head, &tail))
+        goto err;
+    if (!get_bn_from_bin(env, head, &dsa_q))
+        goto err;
+
+    if (!enif_get_list_cell(env, tail, &head, &tail))
+        goto err;
+    if (!get_bn_from_bin(env, head, &dsa_g))
+        goto err;
+
+    if (!enif_get_list_cell(env, tail, &head, &tail))
+        goto err;
+    if (!get_bn_from_bin(env, head, &dsa_y))
+        goto err;
+
+    if (!enif_is_empty_list(env,tail))
+        goto err;
+
+    if (!DSA_set0_pqg(dsa, dsa_p, dsa_q, dsa_g))
+        goto err;
+    /* dsa takes ownership on success */
+    dsa_p = NULL;
+    dsa_q = NULL;
+    dsa_g = NULL;
+
+    if (!DSA_set0_key(dsa, dsa_y, NULL))
+        goto err;
+    /* dsa takes ownership on success */
+    dsa_y = NULL;
+
     return 1;
+
+ err:
+    if (dsa_p)
+        BN_free(dsa_p);
+    if (dsa_q)
+        BN_free(dsa_q);
+    if (dsa_g)
+        BN_free(dsa_g);
+    if (dsa_y)
+        BN_free(dsa_y);
+    return 0;
 }