Re: [openssl.org #459] [bug] DSA BN_init() bugs in 0.9.6h and 0.9.7

[Nils Larsch]

Ivan D Nestlerode via RT wrote:

I sent this to openssl-dev previously, but I think it got lost in
the noise there (since it didn't go through rt).

In OpenSSL 0.9.6h, there are a couple of BN_init() bugs in crypto/dsa/dsa_ossl.c. The BN_init() calls in question are in the functions:
dsa_do_sign() (lines 113-114)
dsa_sign_setup() (line 187)
dsa_do_verify() (lines 239-241)

In all cases, the BN_init() calls need to be moved before the first
if statement (so that they are the first functions executed). As written,
if you goto the err label before doing the BN_init() call you could cause
a crash when you attempt to free a garbage pointer.

The same bugs exist in 0.9.7 but on slightly different line numbers.

The same bug is in the ecdsa code in 0.9.8-dev (see attached patch for
the latest snapshot (== openssl-SNAP-20030114.tar.gz)).

Regards,
Nils

diff -ru openssl-SNAP-20030114/crypto/dsa/dsa_ossl.c dev/crypto/dsa/dsa_ossl.c
--- openssl-SNAP-20030114/crypto/dsa/dsa_ossl.c Mon Nov  4 15:01:33 2002
+++ dev/crypto/dsa/dsa_ossl.c   Wed Jan 15 09:59:39 2003
@@ -106,13 +106,15 @@
        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;
 
@@ -178,6 +180,9 @@
                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;
@@ -185,7 +190,6 @@
        else
                ctx=ctx_in;
 
-       BN_init(&k);
        if ((r=BN_new()) == NULL) goto err;
        kinv=NULL;
 
@@ -241,10 +245,11 @@
                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 -ru openssl-SNAP-20030114/crypto/ecdsa/ecs_ossl.c dev/crypto/ecdsa/ecs_ossl.c
--- openssl-SNAP-20030114/crypto/ecdsa/ecs_ossl.c       Mon Nov  4 15:01:38 2002
+++ dev/crypto/ecdsa/ecs_ossl.c Wed Jan 15 10:01:34 2003
@@ -94,6 +94,9 @@
                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 @@
        do
        {
                /* get random k */      
-               BN_init(&k);
                do
                        if (!BN_rand_range(&k,order))
                        {
@@ -223,6 +225,8 @@
        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 @@
                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 ||