This is an automated email from the ASF dual-hosted git repository. tomaswolf pushed a commit to branch rc-2.20.0 in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 7704e7efbb80dce8036cb28f50907282248f63a7 Author: Thomas Wolf <[email protected]> AuthorDate: Tue Sep 22 19:01:39 2026 +0200 Improve sanity checks in UserAuthKeyboardInteractive There already was a sanity check about the number of challenges on the client side -- but it allowed up to 32768 challenges. That's clearly nonsense. Limit this to the minimum of whatever may fit into the SSH packet and 100. On the server side, there was a similar sanity check for the number of challenge responses, equally nonsensical. But there, RFC 4256 even mandates that the number of responses must match the number of challenges sent exactly, so implement that. --- .../auth/keyboard/UserAuthKeyboardInteractive.java | 22 +++++++++++++++++++--- .../auth/keyboard/UserAuthKeyboardInteractive.java | 11 +++++++---- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/keyboard/UserAuthKeyboardInteractive.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/keyboard/UserAuthKeyboardInteractive.java index c004e22d3..81361634b 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/auth/keyboard/UserAuthKeyboardInteractive.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/keyboard/UserAuthKeyboardInteractive.java @@ -39,6 +39,11 @@ import org.apache.sshd.core.CoreModuleProperties; public class UserAuthKeyboardInteractive extends AbstractUserAuth { public static final String NAME = UserAuthKeyboardInteractiveFactory.NAME; + // Still way too high to be practical (typical servers send maybe 1 to 3 challenges), but high enough to allow even + // really esoteric use cases while being low enough to be very comfortably able to allocate the arrays for the + // prompts and echo flags. + private static final int MAX_PROMPTS = 100; + private final AtomicBoolean requestPending = new AtomicBoolean(false); private Iterator<String> passwords; private int maxAttempts; @@ -120,11 +125,17 @@ public class UserAuthKeyboardInteractive extends AbstractUserAuth { String instruction = buffer.getString(); String lang = buffer.getString(); int num = buffer.getInt(); + // Minimum length for one prompt is 6 bytes: 4 bytes string length of the prompt and at least one byte of data, + // plus one byte for the echo boolean. + int maxPrompts = buffer.available() / 6; + // Now maxPrompts can still be rather large (roughly 43690 -- we accept SSH packets up to 256kB). That many + // prompts make no sense. + maxPrompts = Math.min(maxPrompts, MAX_PROMPTS); // Protect against malicious or corrupted packets - if ((num < 0) || (num > SshConstants.SSH_REQUIRED_PAYLOAD_PACKET_LENGTH_SUPPORT)) { - log.error("processAuthDataRequest({})[{}] illogical challenges count ({}) for name={}, instruction={}", + if ((num < 0) || (num > maxPrompts)) { + log.warn("processAuthDataRequest({})[{}] illogical challenges count ({}) for name={}, instruction={}", session, service, num, name, instruction); - throw new IndexOutOfBoundsException("Illogical challenges count: " + num); + return false; } boolean debugEnabled = log.isDebugEnabled(); @@ -148,6 +159,11 @@ public class UserAuthKeyboardInteractive extends AbstractUserAuth { } } + if (buffer.available() > 0) { + log.warn("processAuthDataRequest({})[{}] {} trailing garbage bytes at end of SSH_MSG_USERAUTH_INFO_REQUEST", + session, service, num, buffer.available()); + return false; + } String[] rep = getUserResponses(name, instruction, lang, prompt, echo); if (rep == null) { if (debugEnabled) { diff --git a/sshd-core/src/main/java/org/apache/sshd/server/auth/keyboard/UserAuthKeyboardInteractive.java b/sshd-core/src/main/java/org/apache/sshd/server/auth/keyboard/UserAuthKeyboardInteractive.java index 5022e7e51..0d179a3b4 100644 --- a/sshd-core/src/main/java/org/apache/sshd/server/auth/keyboard/UserAuthKeyboardInteractive.java +++ b/sshd-core/src/main/java/org/apache/sshd/server/auth/keyboard/UserAuthKeyboardInteractive.java @@ -39,6 +39,8 @@ import org.apache.sshd.server.session.ServerSession; public class UserAuthKeyboardInteractive extends AbstractUserAuth { public static final String NAME = UserAuthKeyboardInteractiveFactory.NAME; + private int numberOfPrompts = -1; + public UserAuthKeyboardInteractive() { super(NAME); } @@ -93,6 +95,7 @@ public class UserAuthKeyboardInteractive extends AbstractUserAuth { challenge.getLanguageTag(), GenericUtils.size(challenge.getPrompts())); } + numberOfPrompts = challenge.getPrompts().size(); // Prompt for password buffer = session.createBuffer(SshConstants.SSH_MSG_USERAUTH_INFO_REQUEST); challenge.append(buffer); @@ -109,10 +112,10 @@ public class UserAuthKeyboardInteractive extends AbstractUserAuth { } int num = buffer.getInt(); - // Protect against malicious or corrupted packets - if ((num < 0) || (num > SshConstants.SSH_REQUIRED_PAYLOAD_PACKET_LENGTH_SUPPORT)) { - log.error("doValidateAuthResponse({}@{}) illogical response count: {}", username, session, num); - throw new IndexOutOfBoundsException("Illogical response count: " + num); + if (num != numberOfPrompts) { + // RFC 4256: if the num-responses field does not match the num-prompts field in the request message, the + // server MUST send a failure message. + return false; } List<String> responses = (num <= 0) ? Collections.emptyList() : new ArrayList<>(num);
