Fix initialization sequence to prevent freeing of unitialized objects.

Submitted by: Nils Larsch <nla@trustcenter.de>

PR: 459

diff --git a/CHANGES b/CHANGES
index c3176727e5a54700756d33e82521e8e666309574..2fd057c41e5e0213b20f11d1cf35fd6764de0893 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,12 @@
 
  Changes between 0.9.7 and 0.9.8  [xx XXX xxxx]
 
+  *) ECDSA routines: under certain error conditions uninitialized BN objects
+     could be freed. Solution: make sure initialization is performed early
+     enough. (Reported and fix supplied by Nils Larsch <nla@trustcenter.de>
+     via PR#459)
+     [Lutz Jaenicke]
+
   *) Key-generation can now be implemented in RSA_METHOD, DSA_METHOD
      and DH_METHOD (eg. by ENGINE implementations) to override the normal
      software implementations. For DSA and DH, parameter generation can
@@ -375,6 +381,12 @@ TODO: bug: pad  x  with leading zeros if necessary
  
  Changes between 0.9.7 and 0.9.7a  [XX xxx 2003]
 
+  *) DSA routines: under certain error conditions uninitialized BN objects
+     could be freed. Solution: make sure initialization is performed early
+     enough. (Reported and fix supplied by Ivan D Nestlerode <nestler@MIT.EDU>,
+     Nils Larsch <nla@trustcenter.de> via PR#459)
+     [Lutz Jaenicke]
+
   *) Another fix for SSLv2 session ID handling: the session ID was incorrectly
      checked on reconnect on the client side, therefore session resumption
      could still fail with a "ssl session id is different" error. This

diff --git a/crypto/dsa/dsa_ossl.c b/crypto/dsa/dsa_ossl.c
index 313c06fa3ff00bff9fb92d93999a01dd11ccfcf9..70d60d9e2962088a2afd6b449987e05af2257a95 100644 (file)
--- a/crypto/dsa/dsa_ossl.c
+++ b/crypto/dsa/dsa_ossl.c
@@ -108,13 +108,15 @@ static DSA_SIG *dsa_do_sign(const unsigned char *dgst, int dlen, DSA *dsa)
        int i,reason=ERR_R_BN_LIB;
        DSA_SIG *ret=NULL;
 
+       BN_init(&m);
+       BN_init(&xr);
+
        if (!dsa->p || !dsa->q || !dsa->g)
                {
                reason=DSA_R_MISSING_PARAMETERS;
                goto err;
                }
-       BN_init(&m);
-       BN_init(&xr);
+
        s=BN_new();
        if (s == NULL) goto err;
 
@@ -180,6 +182,9 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp)
                DSAerr(DSA_F_DSA_SIGN_SETUP,DSA_R_MISSING_PARAMETERS);
                return 0;
                }
+
+       BN_init(&k);
+
        if (ctx_in == NULL)
                {
                if ((ctx=BN_CTX_new()) == NULL) goto err;
@@ -187,7 +192,6 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp)
        else
                ctx=ctx_in;
 
-       BN_init(&k);
        if ((r=BN_new()) == NULL) goto err;
        kinv=NULL;
 
@@ -243,11 +247,12 @@ static int dsa_do_verify(const unsigned char *dgst, int dgst_len, DSA_SIG *sig,
                return -1;
                }
 
-       if ((ctx=BN_CTX_new()) == NULL) goto err;
        BN_init(&u1);
        BN_init(&u2);
        BN_init(&t1);
 
+       if ((ctx=BN_CTX_new()) == NULL) goto err;
+
        if (BN_is_zero(sig->r) || BN_get_sign(sig->r) ||
            BN_ucmp(sig->r, dsa->q) >= 0)
                {

diff --git a/crypto/ecdsa/ecs_ossl.c b/crypto/ecdsa/ecs_ossl.c
index 215da3892af8e9485c78b670f96d211d279fef76..ba1c56121c0dc8069e7e367d71ef9aad797122cf 100644 (file)
--- a/crypto/ecdsa/ecs_ossl.c
+++ b/crypto/ecdsa/ecs_ossl.c
@@ -94,6 +94,9 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp,
                ECDSAerr(ECDSA_F_ECDSA_SIGN_SETUP, ERR_R_PASSED_NULL_PARAMETER);
                return 0;
        }
+
+       BN_init(&k);
+
        if (ctx_in == NULL) 
        {
                if ((ctx=BN_CTX_new()) == NULL)
@@ -134,7 +137,6 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, BIGNUM **kinvp,
        do
        {
                /* get random k */      
-               BN_init(&k);
                do
                        if (!BN_rand_range(&k,order))
                        {
@@ -223,6 +225,8 @@ static ECDSA_SIG *ecdsa_do_sign(const unsigned char *dgst, int dgst_len,
        ECDSA_SIG *ret=NULL;
        ECDSA_DATA *ecdsa;
 
+       BN_init(&xr);
+
        ecdsa = ecdsa_check(eckey);
 
        if (!eckey || !eckey->group || !eckey->pub_key || !eckey->priv_key 
@@ -231,7 +235,6 @@ static ECDSA_SIG *ecdsa_do_sign(const unsigned char *dgst, int dgst_len,
                ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ERR_R_PASSED_NULL_PARAMETER);
                goto err;
        }
-       BN_init(&xr);
 
        if ((ctx = BN_CTX_new()) == NULL || (order = BN_new()) == NULL ||
                (tmp = BN_new()) == NULL || (m = BN_new()) == NULL ||