From dbd9a07fd089c81b6470b9cc7d33bc9040b0da78 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 5 May 2019 10:10:54 +0100 Subject: [PATCH] keyboard-interactive auth: use a uint32 for num_prompts. While testing the previous fix, I also noticed that s->num_prompts is an ordinary signed int. So if the server sends a _really_ large value, it'll be treated as negative. That's kind of harmless: our loop wouldn't read any prompts at all from the packet, and then it would send back the same nonsense count with no responses. But it's inelegant: now, if the server violates the protocol in this way, we respond by sending an even wronger packet in return. Changed the type of num_prompts and the loop variable to uint32_t, so now we'll respond by actually trying to read that many prompts, which will fail by reaching the new error check. I think that's a more sensible way to handle this one. --- ssh2userauth.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ssh2userauth.c b/ssh2userauth.c index 522d1c3e..aaaadd0b 100644 --- a/ssh2userauth.c +++ b/ssh2userauth.c @@ -57,7 +57,7 @@ struct ssh2_userauth_state { strbuf *last_methods_string; bool kbd_inter_refused; prompts_t *cur_prompt; - int num_prompts; + uint32_t num_prompts; const char *username; char *locally_allocated_username; char *password; @@ -1231,7 +1231,6 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl) ptrlen name, inst; strbuf *sb; - int i; /* * We've got a fresh USERAUTH_INFO_REQUEST. @@ -1248,7 +1247,7 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl) * Get any prompt(s) from the packet. */ s->num_prompts = get_uint32(pktin); - for (i = 0; i < s->num_prompts; i++) { + for (uint32_t i = 0; i < s->num_prompts; i++) { ptrlen prompt = get_string(pktin); bool echo = get_bool(pktin); @@ -1366,7 +1365,7 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl) s->pktout = ssh_bpp_new_pktout( s->ppl.bpp, SSH2_MSG_USERAUTH_INFO_RESPONSE); put_uint32(s->pktout, s->num_prompts); - for (i=0; i < s->num_prompts; i++) { + for (uint32_t i = 0; i < s->num_prompts; i++) { put_stringz(s->pktout, s->cur_prompt->prompts[i]->result); }