Avoid a NULL ptr deref if group is not set

We should only copy parameters and keys if the group is set. Otherwise
they don't really make any sense. Previously we copied the private key
regardless of whether the group was set...but if it wasn't a NULL ptr
deref could occur. It's unclear whether we could ever get into that
situation, but since we were already checking it for the public key we
should be consistent.

Reviewed-by: Rich Salz <rsalz@openssl.org>

diff --git a/crypto/ec/ec_key.c b/crypto/ec/ec_key.c
index 22c6535e307b256b8e7ca22d2d6e2107c1a78fa2..31ed8a58a87c6c1a25e420deee37348c2ade0bfd 100644 (file)
--- a/crypto/ec/ec_key.c
+++ b/crypto/ec/ec_key.c
@@ -148,28 +148,29 @@ EC_KEY *EC_KEY_copy(EC_KEY *dest, EC_KEY *src)
             return NULL;
         if (!EC_GROUP_copy(dest->group, src->group))
             return NULL;
-    }
-    /*  copy the public key */
-    if (src->pub_key != NULL && src->group != NULL) {
-        EC_POINT_free(dest->pub_key);
-        dest->pub_key = EC_POINT_new(src->group);
-        if (dest->pub_key == NULL)
-            return NULL;
-        if (!EC_POINT_copy(dest->pub_key, src->pub_key))
-            return NULL;
-    }
-    /* copy the private key */
-    if (src->priv_key != NULL) {
-        if (dest->priv_key == NULL) {
-            dest->priv_key = BN_new();
-            if (dest->priv_key == NULL)
+
+        /*  copy the public key */
+        if (src->pub_key != NULL) {
+            EC_POINT_free(dest->pub_key);
+            dest->pub_key = EC_POINT_new(src->group);
+            if (dest->pub_key == NULL)
+                return NULL;
+            if (!EC_POINT_copy(dest->pub_key, src->pub_key))
+                return NULL;
+        }
+        /* copy the private key */
+        if (src->priv_key != NULL) {
+            if (dest->priv_key == NULL) {
+                dest->priv_key = BN_new();
+                if (dest->priv_key == NULL)
+                    return NULL;
+            }
+            if (!BN_copy(dest->priv_key, src->priv_key))
+                return NULL;
+            if (src->group->meth->keycopy
+                && src->group->meth->keycopy(dest, src) == 0)
                 return NULL;
         }
-        if (!BN_copy(dest->priv_key, src->priv_key))
-            return NULL;
-        if (src->group->meth->keycopy
-            && src->group->meth->keycopy(dest, src) == 0)
-            return NULL;
     }