From b9d0371c47c1b0b0bb856fee6bb770627aa50a11 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Tue, 19 Feb 2019 19:38:15 +0000
Subject: [PATCH] Add validation of remote keys in ECC Diffie-Hellman.

In both the Weierstrass and Montgomery forms, we now check that the
provided curve point isn't a silly one, like the identity or a torsion
point, which will give little or no variation in the possible outputs
of key exchange.
---
 sshecc.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/sshecc.c b/sshecc.c
index f3eddc64..80965685 100644
--- a/sshecc.c
+++ b/sshecc.c
@@ -1329,6 +1329,12 @@ static mp_int *ssh_ecdhkex_w_getkey(ecdh_key *dh, ptrlen remoteKey)
     if (!remote_p)
         return NULL;
 
+    if (ecc_weierstrass_is_identity(remote_p)) {
+        /* Not a sensible Diffie-Hellman input value */
+        ecc_weierstrass_point_free(remote_p);
+        return NULL;
+    }
+
     WeierstrassPoint *p = ecc_weierstrass_multiply(remote_p, dh->private);
 
     mp_int *x;
@@ -1343,6 +1349,21 @@ static mp_int *ssh_ecdhkex_w_getkey(ecdh_key *dh, ptrlen remoteKey)
 static mp_int *ssh_ecdhkex_m_getkey(ecdh_key *dh, ptrlen remoteKey)
 {
     mp_int *remote_x = mp_from_bytes_le(remoteKey);
+    if (mp_eq_integer(remote_x, 0)) {
+        /*
+         * The libssh spec for Curve25519 key exchange says that
+         * 'every possible public key maps to a valid ECC Point' and
+         * therefore no validation needs to be done on the server's
+         * provided x-coordinate. However, I don't believe it: an
+         * x-coordinate of zero doesn't work sensibly, because you end
+         * up dividing by zero in the doubling formula
+         * (x+1)^2(x-1)^2/(4(x^3+ax^2+x)). (Put another way, although
+         * that point P is not the _identity_ of the curve, it is a
+         * torsion point such that 2P is the identity.)
+         */
+        mp_free(remote_x);
+        return NULL;
+    }
     MontgomeryPoint *remote_p = ecc_montgomery_point_new(
         dh->curve->m.mc, remote_x);
     mp_free(remote_x);