Fix initialization sequence to prevent freeing of unitialized objects.

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

PR: 459

3 files changed, 26 insertions, 6 deletions

diff --git a/CHANGES b/CHANGES
index c3176727e5..2fd057c41e 100644
--- 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 313c06fa3f..70d60d9e29 100644
--- 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 215da3892a..ba1c56121c 100644
--- 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 ||