Fixes to memory management in the elliptic curve code.

There was an error-handling path testing the wrong variable; an
inappropriate call to ec_point_free in decodepoint() (in fact, that
function always gets passed a pointer to an ec_point structure that's
not a dynamically allocated block at all or not in its own right, so
we should have just cleared its contents without freeing the structure
itself); a missing return on an error path which would have caused the
same structure to be freed a second time; and two missing freebn in
ecdsa_sign.

Patch due to Tim Kosse.

diff --git a/sshecc.c b/sshecc.c
index 7d877bade133d12292fa6b960dc7f686332d7938..10c4fc114506fe89cc5fc5ea4b966acd33c65c0b 100644 (file)
--- a/sshecc.c
+++ b/sshecc.c
@@ -628,7 +628,7 @@ static struct ec_point *ecp_double(const struct ec_point *a, const int aminus3)
         }
         XmZ2 = modsub(a->x, Z2, a->curve->p);
         freebn(Z2);
-        if (!XpZ2) {
+        if (!XmZ2) {
             freebn(S);
             freebn(XpZ2);
             return NULL;
@@ -1434,7 +1434,10 @@ static int decodepoint(char *p, int length, struct ec_point *point)
 
     /* Verify the point is on the curve */
     if (!ec_point_verify(point)) {
-        ec_point_free(point);
+        freebn(point->x);
+        point->x = NULL;
+        freebn(point->y);
+        point->y = NULL;
         return 0;
     }
 
@@ -1714,6 +1717,7 @@ static void *ecdsa_openssh_createkey(unsigned char **blob, int *len)
         /* Private key doesn't make the public key on the given curve */
         ecdsa_freekey(ec);
         ec_point_free(publicKey);
+        return NULL;
     }
 
     ec_point_free(publicKey);
@@ -1947,6 +1951,9 @@ static unsigned char *ecdsa_sign(void *key, char *data, int datalen,
     for (i = slen; i--;)
         *p++ = bignum_byte(s, i);
 
+    freebn(r);
+    freebn(s);
+
     return buf;
 }