From 26fe1e26c0f7ab42440332882295667d4a0ac500 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. (cherry picked from commit 1eb578a488a71284d6b18e46df301e54805f2c35) Conflicts: ssh.c Cherry-picker's notes: the main conflict arose because the original commit also made fixes to the ECDH branch of the big key exchange if statement, which doesn't exist on this branch. Also there was a minor and purely textual conflict, when an error check was added right next to a function call that had acquired an extra parameter on master. --- ssh.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/ssh.c b/ssh.c index dd969622..a37b0441 100644 --- a/ssh.c +++ b/ssh.c @@ -6668,13 +6668,20 @@ static void do_ssh2_transport(Ssh ssh, void *vin, int inlen, } set_busy_status(ssh->frontend, BUSY_CPU); /* cogitate */ ssh_pkt_getstring(pktin, &s->hostkeydata, &s->hostkeylen); - s->hkey = ssh->hostkey->newkey(s->hostkeydata, s->hostkeylen); + if (!s->hostkeydata) { + bombout(("unable to parse key exchange reply packet")); + crStopV; + } s->f = ssh2_pkt_getmp(pktin); if (!s->f) { bombout(("unable to parse key exchange reply packet")); 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); @@ -6723,6 +6730,10 @@ static void do_ssh2_transport(Ssh ssh, 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(s->hostkeydata, s->hostkeylen); @@ -6730,6 +6741,10 @@ static void do_ssh2_transport(Ssh ssh, 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); } @@ -6806,6 +6821,10 @@ static void do_ssh2_transport(Ssh ssh, 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); }