Blob Blame History Raw
--- contrib/mod_sftp_pam.c
+++ contrib/mod_sftp_pam.c
@@ -169,22 +169,13 @@ static int sftppam_converse(int nmsgs, P
     return PAM_CONV_ERR;
   }
 
-  if (sftp_kbdint_recv_response(sftppam_driver.driver_pool, &recvd_count,
-      &recvd_responses) < 0) {
+  if (sftp_kbdint_recv_response(sftppam_driver.driver_pool, list->nelts,
+      &recvd_count, &recvd_responses) < 0) {
     pr_trace_msg(trace_channel, 3,
       "error receiving keyboard-interactive responses: %s", strerror(errno));
     return PAM_CONV_ERR;
   }
 
-  /* Make sure that the count of responses matches the challenge count. */
-  if (recvd_count != list->nelts) {
-    (void) pr_log_writefile(sftp_logfd, MOD_SFTP_PAM_VERSION,
-      "sent %d %s, but received %u %s", nmsgs,
-      list->nelts != 1 ? "challenges" : "challenge", recvd_count,
-      recvd_count != 1 ? "responses" : "response");
-    return PAM_CONV_ERR;
-  }
-
   res = calloc(nmsgs, sizeof(struct pam_response));
   if (res == NULL) {
     pr_log_pri(PR_LOG_CRIT, "Out of memory!");
--- contrib/mod_sftp/kbdint.c
+++ contrib/mod_sftp/kbdint.c
@@ -31,6 +31,8 @@
 #include "utf8.h"
 #include "kbdint.h"
 
+#define SFTP_KBDINT_MAX_RESPONSES	500
+
 struct kbdint_driver {
   struct kbdint_driver *next, *prev;
 
@@ -252,8 +254,8 @@ int sftp_kbdint_send_challenge(const cha
   return res;
 }
 
-int sftp_kbdint_recv_response(pool *p, unsigned int *count,
-    const char ***responses) {
+int sftp_kbdint_recv_response(pool *p, unsigned int expected_count,
+    unsigned int *rcvd_count, const char ***responses) {
   register unsigned int i;
   char *buf;
   cmd_rec *cmd;
@@ -264,7 +266,7 @@ int sftp_kbdint_recv_response(pool *p, u
   int res;
 
   if (p == NULL ||
-      count == NULL ||
+      rcvd_count == NULL ||
       responses == NULL) {
     errno = EINVAL;
     return -1;
@@ -299,6 +301,29 @@ int sftp_kbdint_recv_response(pool *p, u
 
   resp_count = sftp_msg_read_int(pkt->pool, &buf, &buflen);
 
+  /* Ensure that the number of responses sent by the client is the same
+   * as the number of challenges sent, lest a malicious client attempt to
+   * trick us into allocating too much memory (Bug#3973).
+   */
+  if (resp_count != expected_count) {
+    (void) pr_log_writefile(sftp_logfd, MOD_SFTP_VERSION,
+      "sent %lu %s, but received %lu %s", (unsigned long) expected_count,
+      expected_count != 1 ? "challenges" : "challenge",
+      (unsigned long) resp_count, resp_count != 1 ? "responses" : "response");
+    destroy_pool(pkt->pool);
+    errno = EPERM;
+    return -1;
+  }
+
+  if (resp_count > SFTP_KBDINT_MAX_RESPONSES) {
+    (void) pr_log_writefile(sftp_logfd, MOD_SFTP_VERSION,
+      "received too many responses (%lu > max %lu), rejecting",
+      (unsigned long) resp_count, (unsigned long) SFTP_KBDINT_MAX_RESPONSES);
+    destroy_pool(pkt->pool);
+    errno = EPERM;
+    return -1;
+  }
+
   list = make_array(p, resp_count, sizeof(char *));
   for (i = 0; i < resp_count; i++) {
     char *resp;
@@ -307,7 +332,7 @@ int sftp_kbdint_recv_response(pool *p, u
     *((char **) push_array(list)) = pstrdup(p, sftp_utf8_decode_str(p, resp));
   }
 
-  *count = (unsigned int) resp_count;
+  *rcvd_count = (unsigned int) resp_count;
   *responses = ((const char **) list->elts);
   return 0;
 }
--- contrib/mod_sftp/mod_sftp.h.in
+++ contrib/mod_sftp/mod_sftp.h.in
@@ -160,7 +160,8 @@ int sftp_kbdint_register_driver(const ch
 int sftp_kbdint_unregister_driver(const char *name);
 int sftp_kbdint_send_challenge(const char *, const char *, unsigned int,
   sftp_kbdint_challenge_t *);
-int sftp_kbdint_recv_response(pool *, unsigned int *, const char ***);
+int sftp_kbdint_recv_response(pool *, unsigned int, unsigned int *,
+  const char ***);
 
 /* API for modules that which to register keystores, for the
  * SFTPAuthorizedHostKeys and SFTPAuthorizedUserKeys directives.