Revamp mod_exp_nif()

* Add error checking for OpenSSL calls.
* Change dlen from unsigned to signed since BN_num_bytes is int.
* Add bounds checking before casting.
* Consolidate all freeing to one location on error or success.

1 files changed, 51 insertions, 25 deletions

diff --git a/lib/crypto/c_src/bn.c b/lib/crypto/c_src/bn.c
index 75ddd4c2f2..9a426f4f8d 100644
--- a/lib/crypto/c_src/bn.c
+++ b/lib/crypto/c_src/bn.c
@@ -94,41 +94,67 @@ ERL_NIF_TERM bin_from_bn(ErlNifEnv* env, const BIGNUM *bn)
 
 ERL_NIF_TERM mod_exp_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
 {/* (Base,Exponent,Modulo,bin_hdr) */
-    BIGNUM *bn_base=NULL, *bn_exponent=NULL, *bn_modulo=NULL, *bn_result;
-    BN_CTX *bn_ctx;
+    BIGNUM *bn_base = NULL, *bn_exponent = NULL, *bn_modulo = NULL, *bn_result = NULL;
+    BN_CTX *bn_ctx = NULL;
     unsigned char* ptr;
-    unsigned dlen;
+    int dlen;
     unsigned bin_hdr; /* return type: 0=plain binary, 4: mpint */
     unsigned extra_byte;
     ERL_NIF_TERM ret;
 
-    if (!get_bn_from_bin(env, argv[0], &bn_base)
-	|| !get_bn_from_bin(env, argv[1], &bn_exponent)
-	|| !get_bn_from_bin(env, argv[2], &bn_modulo)
-	|| !enif_get_uint(env,argv[3],&bin_hdr) || (bin_hdr & ~4)) {
+    if (argc != 4)
+        goto bad_arg;
+    if (!get_bn_from_bin(env, argv[0], &bn_base))
+        goto bad_arg;
+    if (!get_bn_from_bin(env, argv[1], &bn_exponent))
+        goto bad_arg;
+    if (!get_bn_from_bin(env, argv[2], &bn_modulo))
+        goto bad_arg;
+    if (!enif_get_uint(env, argv[3], &bin_hdr))
+        goto bad_arg;
+    if (bin_hdr != 0 && bin_hdr != 4)
+        goto bad_arg;
+
+    if ((bn_result = BN_new()) == NULL)
+        goto err;
+    if ((bn_ctx = BN_CTX_new()) == NULL)
+        goto err;
+
+    if (!BN_mod_exp(bn_result, bn_base, bn_exponent, bn_modulo, bn_ctx))
+        goto err;
 
-	if (bn_base) BN_free(bn_base);
-	if (bn_exponent) BN_free(bn_exponent);
-	if (bn_modulo) BN_free(bn_modulo);
-	return enif_make_badarg(env);
-    }
-    bn_result = BN_new();
-    bn_ctx = BN_CTX_new();
-    BN_mod_exp(bn_result, bn_base, bn_exponent, bn_modulo, bn_ctx);
     dlen = BN_num_bytes(bn_result);
-    extra_byte = bin_hdr && BN_is_bit_set(bn_result, dlen*8-1);
-    ptr = enif_make_new_binary(env, bin_hdr+extra_byte+dlen, &ret);
+    if (dlen < 0 || dlen > INT_MAX / 8)
+        goto bad_arg;
+    extra_byte = bin_hdr && BN_is_bit_set(bn_result, dlen * 8 - 1);
+
+    if ((ptr = enif_make_new_binary(env, bin_hdr + extra_byte + (unsigned int)dlen, &ret)) == NULL)
+        goto err;
+
     if (bin_hdr) {
-	put_int32(ptr, extra_byte+dlen);
-	ptr[4] = 0; /* extra zeroed byte to ensure a positive mpint */
-	ptr += bin_hdr + extra_byte;
+        put_int32(ptr, extra_byte + (unsigned int)dlen);
+        ptr[4] = 0; /* extra zeroed byte to ensure a positive mpint */
+        ptr += bin_hdr + extra_byte;
     }
+
     BN_bn2bin(bn_result, ptr);
-    BN_free(bn_result);
-    BN_CTX_free(bn_ctx);
-    BN_free(bn_modulo);
-    BN_free(bn_exponent);
-    BN_free(bn_base);
+    goto done;
+
+ bad_arg:
+ err:
+    ret = enif_make_badarg(env);
+
+ done:
+    if (bn_base)
+        BN_free(bn_base);
+    if (bn_exponent)
+        BN_free(bn_exponent);
+    if (bn_modulo)
+        BN_free(bn_modulo);
+    if (bn_result)
+        BN_free(bn_result);
+    if (bn_ctx)
+        BN_CTX_free(bn_ctx);
     return ret;
 }