From 40843b432ae8975c171d78aa2ecc68b8977ed189 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 10 Feb 2019 08:08:50 +0000 Subject: [PATCH] dss_sign(): fix a theoretically possible overflow. I computed hash + x*r by first computing x*r, and then using mp_add_into to add the hash to it in the same bignum. But if the result of x*r had been allocated an mp_int only just large enough to contain it, then the addition of the hash might have made it overflow and generated a bogus signature. I've never seen that happen, and for all I know word sizes may make it completely impossible. But it's a theoretical possibility, and easy to fix now that I've happened to spot it in passing. --- sshdss.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sshdss.c b/sshdss.c index bc2f044a..2a66c977 100644 --- a/sshdss.c +++ b/sshdss.c @@ -434,10 +434,11 @@ static void dss_sign(ssh_key *key, ptrlen data, unsigned flags, BinarySink *bs) mp_free(gkp); mp_int *hash = mp_from_bytes_be(make_ptrlen(digest, 20)); - mp_int *hxr = mp_mul(dss->x, r); - mp_add_into(hxr, hxr, hash); /* hash + x*r */ + mp_int *xr = mp_mul(dss->x, r); + mp_int *hxr = mp_add(xr, hash); /* hash + x*r */ mp_int *s = mp_modmul(kinv, hxr, dss->q); /* s = k^-1 * (hash+x*r) mod q */ mp_free(hxr); + mp_free(xr); mp_free(kinv); mp_free(k); mp_free(hash);