From 1eb578a488a71284d6b18e46df301e54805f2c35 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 13 Jun 2015 15:22:03 +0100 Subject: [PATCH] Add missing null-pointer checks in key exchange. Assorted calls to ssh_pkt_getstring in handling the later parts of key exchange (post-KEXINIT) were not checked for NULL afterwards, so that a variety of badly formatted key exchange packets would cause a crash rather than a sensible error message. None of these is an exploitable vulnerability - the server can only force a clean null-deref crash, not an access to actually interesting memory. Thanks to '3unnym00n' for pointing out one of these, causing me to find all the rest of them too. --- ssh.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/ssh.c b/ssh.c index 4da449c2..46f1d76d 100644 --- a/ssh.c +++ b/ssh.c @@ -6778,6 +6778,10 @@ static void do_ssh2_transport(Ssh ssh, const void *vin, int inlen, } set_busy_status(ssh->frontend, BUSY_CPU); /* cogitate */ ssh_pkt_getstring(pktin, &s->hostkeydata, &s->hostkeylen); + if (!s->hostkeydata) { + bombout(("unable to parse key exchange reply packet")); + crStopV; + } s->hkey = ssh->hostkey->newkey(ssh->hostkey, s->hostkeydata, s->hostkeylen); s->f = ssh2_pkt_getmp(pktin); @@ -6786,6 +6790,10 @@ static void do_ssh2_transport(Ssh ssh, const void *vin, int inlen, crStopV; } ssh_pkt_getstring(pktin, &s->sigdata, &s->siglen); + if (!s->sigdata) { + bombout(("unable to parse key exchange reply packet")); + crStopV; + } { const char *err = dh_validate_f(ssh->kex_ctx, s->f); @@ -6857,6 +6865,10 @@ static void do_ssh2_transport(Ssh ssh, const void *vin, int inlen, } ssh_pkt_getstring(pktin, &s->hostkeydata, &s->hostkeylen); + if (!s->hostkeydata) { + bombout(("unable to parse ECDH reply packet")); + crStopV; + } hash_string(ssh->kex->hash, ssh->exhash, s->hostkeydata, s->hostkeylen); s->hkey = ssh->hostkey->newkey(ssh->hostkey, s->hostkeydata, s->hostkeylen); @@ -6879,6 +6891,10 @@ static void do_ssh2_transport(Ssh ssh, const void *vin, int inlen, char *keydata; int keylen; ssh_pkt_getstring(pktin, &keydata, &keylen); + if (!keydata) { + bombout(("unable to parse ECDH reply packet")); + crStopV; + } hash_string(ssh->kex->hash, ssh->exhash, keydata, keylen); s->K = ssh_ecdhkex_getkey(s->eckey, keydata, keylen); if (!s->K) { @@ -6889,6 +6905,10 @@ static void do_ssh2_transport(Ssh ssh, const void *vin, int inlen, } ssh_pkt_getstring(pktin, &s->sigdata, &s->siglen); + if (!s->sigdata) { + bombout(("unable to parse key exchange reply packet")); + crStopV; + } ssh_ecdhkex_freekey(s->eckey); } else { @@ -6906,6 +6926,10 @@ static void do_ssh2_transport(Ssh ssh, const void *vin, int inlen, } ssh_pkt_getstring(pktin, &s->hostkeydata, &s->hostkeylen); + if (!s->hostkeydata) { + bombout(("unable to parse RSA public key packet")); + crStopV; + } hash_string(ssh->kex->hash, ssh->exhash, s->hostkeydata, s->hostkeylen); s->hkey = ssh->hostkey->newkey(ssh->hostkey, @@ -6914,6 +6938,10 @@ static void do_ssh2_transport(Ssh ssh, const void *vin, int inlen, { char *keydata; ssh_pkt_getstring(pktin, &keydata, &s->rsakeylen); + if (!keydata) { + bombout(("unable to parse RSA public key packet")); + crStopV; + } s->rsakeydata = snewn(s->rsakeylen, char); memcpy(s->rsakeydata, keydata, s->rsakeylen); } @@ -6990,6 +7018,10 @@ static void do_ssh2_transport(Ssh ssh, const void *vin, int inlen, } ssh_pkt_getstring(pktin, &s->sigdata, &s->siglen); + if (!s->sigdata) { + bombout(("unable to parse signature packet")); + crStopV; + } sfree(s->rsakeydata); }