1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-26 09:42:25 +00:00

Fix another crash at KEXINIT time, ahem.

This is the same code I previously fixed for failing to check NULL
pointers coming back from ssh_pkt_getstring if the server's KEXINIT
ended early, leading to an embarrassing segfault in place of a fatal
error message. But I've now also had it pointed out to me that the
fatal error message passes the string as %s, which is inappropriate
because (being read straight out of the middle of an SSH packet) it
isn't necessarily zero-terminated!

This is still just an embarrassing segfault in place of a fatal error
message, and not exploitable as far as I can see, because the string
is passed to a dupprintf, which will either read off the end of
allocated address space and segfault non-exploitably, or else it will
find a NUL after all and carefully allocate enough space to format an
error message containing all of the previous junk. But still, how
embarrassing to have messed up the same code _twice_.

[originally from svn r10211]
This commit is contained in:
Simon Tatham 2014-07-28 17:47:36 +00:00
parent 440962281a
commit 4b2a2060bb

48
ssh.c
View File

@ -6226,6 +6226,10 @@ static void do_ssh2_transport(Ssh ssh, void *vin, int inlen,
pktin->savedpos += 16; /* skip garbage cookie */ pktin->savedpos += 16; /* skip garbage cookie */
ssh_pkt_getstring(pktin, &str, &len); /* key exchange algorithms */ ssh_pkt_getstring(pktin, &str, &len); /* key exchange algorithms */
if (!str) {
bombout(("KEXINIT packet was incomplete"));
crStopV;
}
preferred = NULL; preferred = NULL;
for (i = 0; i < s->n_preferred_kex; i++) { for (i = 0; i < s->n_preferred_kex; i++) {
@ -6245,8 +6249,8 @@ static void do_ssh2_transport(Ssh ssh, void *vin, int inlen,
break; break;
} }
if (!ssh->kex) { if (!ssh->kex) {
bombout(("Couldn't agree a key exchange algorithm (available: %s)", bombout(("Couldn't agree a key exchange algorithm"
str ? str : "(null)")); " (available: %.*s)", len, str));
crStopV; crStopV;
} }
/* /*
@ -6256,6 +6260,10 @@ static void do_ssh2_transport(Ssh ssh, void *vin, int inlen,
*/ */
s->guessok = first_in_commasep_string(preferred, str, len); s->guessok = first_in_commasep_string(preferred, str, len);
ssh_pkt_getstring(pktin, &str, &len); /* host key algorithms */ ssh_pkt_getstring(pktin, &str, &len); /* host key algorithms */
if (!str) {
bombout(("KEXINIT packet was incomplete"));
crStopV;
}
for (i = 0; i < lenof(hostkey_algs); i++) { for (i = 0; i < lenof(hostkey_algs); i++) {
if (in_commasep_string(hostkey_algs[i]->name, str, len)) { if (in_commasep_string(hostkey_algs[i]->name, str, len)) {
ssh->hostkey = hostkey_algs[i]; ssh->hostkey = hostkey_algs[i];
@ -6263,14 +6271,18 @@ static void do_ssh2_transport(Ssh ssh, void *vin, int inlen,
} }
} }
if (!ssh->hostkey) { if (!ssh->hostkey) {
bombout(("Couldn't agree a host key algorithm (available: %s)", bombout(("Couldn't agree a host key algorithm"
str ? str : "(null)")); " (available: %.*s)", len, str));
crStopV; crStopV;
} }
s->guessok = s->guessok && s->guessok = s->guessok &&
first_in_commasep_string(hostkey_algs[0]->name, str, len); first_in_commasep_string(hostkey_algs[0]->name, str, len);
ssh_pkt_getstring(pktin, &str, &len); /* client->server cipher */ ssh_pkt_getstring(pktin, &str, &len); /* client->server cipher */
if (!str) {
bombout(("KEXINIT packet was incomplete"));
crStopV;
}
for (i = 0; i < s->n_preferred_ciphers; i++) { for (i = 0; i < s->n_preferred_ciphers; i++) {
const struct ssh2_ciphers *c = s->preferred_ciphers[i]; const struct ssh2_ciphers *c = s->preferred_ciphers[i];
if (!c) { if (!c) {
@ -6287,12 +6299,16 @@ static void do_ssh2_transport(Ssh ssh, void *vin, int inlen,
break; break;
} }
if (!s->cscipher_tobe) { if (!s->cscipher_tobe) {
bombout(("Couldn't agree a client-to-server cipher (available: %s)", bombout(("Couldn't agree a client-to-server cipher"
str ? str : "(null)")); " (available: %.*s)", len, str));
crStopV; crStopV;
} }
ssh_pkt_getstring(pktin, &str, &len); /* server->client cipher */ ssh_pkt_getstring(pktin, &str, &len); /* server->client cipher */
if (!str) {
bombout(("KEXINIT packet was incomplete"));
crStopV;
}
for (i = 0; i < s->n_preferred_ciphers; i++) { for (i = 0; i < s->n_preferred_ciphers; i++) {
const struct ssh2_ciphers *c = s->preferred_ciphers[i]; const struct ssh2_ciphers *c = s->preferred_ciphers[i];
if (!c) { if (!c) {
@ -6309,12 +6325,16 @@ static void do_ssh2_transport(Ssh ssh, void *vin, int inlen,
break; break;
} }
if (!s->sccipher_tobe) { if (!s->sccipher_tobe) {
bombout(("Couldn't agree a server-to-client cipher (available: %s)", bombout(("Couldn't agree a server-to-client cipher"
str ? str : "(null)")); " (available: %.*s)", len, str));
crStopV; crStopV;
} }
ssh_pkt_getstring(pktin, &str, &len); /* client->server mac */ ssh_pkt_getstring(pktin, &str, &len); /* client->server mac */
if (!str) {
bombout(("KEXINIT packet was incomplete"));
crStopV;
}
for (i = 0; i < s->nmacs; i++) { for (i = 0; i < s->nmacs; i++) {
if (in_commasep_string(s->maclist[i]->name, str, len)) { if (in_commasep_string(s->maclist[i]->name, str, len)) {
s->csmac_tobe = s->maclist[i]; s->csmac_tobe = s->maclist[i];
@ -6322,6 +6342,10 @@ static void do_ssh2_transport(Ssh ssh, void *vin, int inlen,
} }
} }
ssh_pkt_getstring(pktin, &str, &len); /* server->client mac */ ssh_pkt_getstring(pktin, &str, &len); /* server->client mac */
if (!str) {
bombout(("KEXINIT packet was incomplete"));
crStopV;
}
for (i = 0; i < s->nmacs; i++) { for (i = 0; i < s->nmacs; i++) {
if (in_commasep_string(s->maclist[i]->name, str, len)) { if (in_commasep_string(s->maclist[i]->name, str, len)) {
s->scmac_tobe = s->maclist[i]; s->scmac_tobe = s->maclist[i];
@ -6329,6 +6353,10 @@ static void do_ssh2_transport(Ssh ssh, void *vin, int inlen,
} }
} }
ssh_pkt_getstring(pktin, &str, &len); /* client->server compression */ ssh_pkt_getstring(pktin, &str, &len); /* client->server compression */
if (!str) {
bombout(("KEXINIT packet was incomplete"));
crStopV;
}
for (i = 0; i < lenof(compressions) + 1; i++) { for (i = 0; i < lenof(compressions) + 1; i++) {
const struct ssh_compress *c = const struct ssh_compress *c =
i == 0 ? s->preferred_comp : compressions[i - 1]; i == 0 ? s->preferred_comp : compressions[i - 1];
@ -6345,6 +6373,10 @@ static void do_ssh2_transport(Ssh ssh, void *vin, int inlen,
} }
} }
ssh_pkt_getstring(pktin, &str, &len); /* server->client compression */ ssh_pkt_getstring(pktin, &str, &len); /* server->client compression */
if (!str) {
bombout(("KEXINIT packet was incomplete"));
crStopV;
}
for (i = 0; i < lenof(compressions) + 1; i++) { for (i = 0; i < lenof(compressions) + 1; i++) {
const struct ssh_compress *c = const struct ssh_compress *c =
i == 0 ? s->preferred_comp : compressions[i - 1]; i == 0 ? s->preferred_comp : compressions[i - 1];