From 0d0b0a45bcbe4cba107d6359f6f4ca70865047dc Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 4 May 2019 15:58:21 +0100 Subject: [PATCH] Fix server-triggered DoS in keyboard-interactive auth. If a server sends a very large number as the num-prompts field, we'd loop round calling get_string and get_bool, which would run off the end of the buffer but still carefully return legal default values (the empty string and false), so we'd carry on piling pointless stuff into s->cur_prompt and using up lots of memory. Coverity pointed this out by warning that an untrusted server-provided value was being used as a loop bound. I'm not convinced that's an error in every case, but I must admit it pointed out a useful thing here! The fix is to check the error indicator on the BinarySource after reading each prompt from the input packet. Then we'll only keep allocating memory as long as there's actually data in the packet. --- ssh2userauth.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ssh2userauth.c b/ssh2userauth.c index 77c339b8..522d1c3e 100644 --- a/ssh2userauth.c +++ b/ssh2userauth.c @@ -1252,6 +1252,13 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl) ptrlen prompt = get_string(pktin); bool echo = get_bool(pktin); + if (get_err(pktin)) { + ssh_proto_error( + s->ppl.ssh, "Server sent truncated " + "SSH_MSG_USERAUTH_INFO_REQUEST packet"); + return; + } + sb = strbuf_new(); if (!prompt.len) { put_datapl(sb, PTRLEN_LITERAL(