http://bugs.proftpd.org/show_bug.cgi?id=4014

I addition, this patch backports the pr_str_get_nbytes() function.

diff --git a/include/options.h b/include/options.h

index 96fa35d..0b29bf7 100644

--- a/include/options.h

+++ b/include/options.h

@@ -110,6 +110,13 @@

 # define PR_TUNABLE_XFER_BUFFER_SIZE	PR_TUNABLE_BUFFER_SIZE

 #endif

+/* Maximum FTP command size.  For details on this size of 512KB, see

+ * the Bug#4014 discussion.

+ */

+#ifndef PR_TUNABLE_CMD_BUFFER_SIZE

+# define PR_TUNABLE_CMD_BUFFER_SIZE    (512 * 1024)

+#endif

+

 /* Maximum path length.  GNU HURD (and some others) do not define

  * MAXPATHLEN.  POSIX' PATH_MAX is mandated to be at least 256 

  * (according to some), so 1K, in the absense of MAXPATHLEN, should be

diff --git a/include/str.h b/include/str.h

index b4fed7c..1cf8724 100644

--- a/include/str.h

+++ b/include/str.h

@@ -39,6 +39,7 @@ char *pstrndup(pool *, const char *, size_t);

 char *pr_str_strip(pool *, char *);

 char *pr_str_strip_end(char *, char *);

+int pr_str_get_nbytes(const char *, const char *, off_t *);

 char *pr_str_get_word(char **, int);

 #define PR_STR_FL_PRESERVE_COMMENTS		0x0001

diff --git a/modules/mod_core.c b/modules/mod_core.c

index 18a47c2..922f4d1 100644

--- a/modules/mod_core.c

+++ b/modules/mod_core.c

@@ -2240,18 +2240,44 @@ MODRET set_allowforeignaddress(cmd_rec *cmd) {

 }

 MODRET set_commandbuffersize(cmd_rec *cmd) {

-  int size = 0;

+  int res;

+  size_t size = 0;

+  off_t nbytes = 0;

   config_rec *c = NULL;

+  const char *units = NULL;

+

+  if (cmd->argc < 2 || cmd->argc > 3) {

+    CONF_ERROR(cmd, "wrong number of parameters")

+  }

-  CHECK_ARGS(cmd, 1);

   CHECK_CONF(cmd, CONF_ROOT|CONF_VIRTUAL|CONF_GLOBAL);

-  /* NOTE: need to add checks for maximum possible sizes, negative sizes. */

-  size = atoi(cmd->argv[1]);

+  if (cmd->argc == 3) {

+    units = cmd->argv[2];

+  }

+

+  if (pr_str_get_nbytes(cmd->argv[1], units, &nbytes) < 0) {

+    CONF_ERROR(cmd, pstrcat(cmd->tmp_pool, "unable to parse: ",

+      cmd->argv[1], " ", units, ": ", strerror(errno), NULL));

+  }

+

+  if (nbytes > PR_TUNABLE_CMD_BUFFER_SIZE) {

+    char max[1024];

+

+    snprintf(max, sizeof(max)-1, "%lu", (unsigned long)

+      PR_TUNABLE_CMD_BUFFER_SIZE);

+    max[sizeof(max)-1] = '\0';

+

+    CONF_ERROR(cmd, pstrcat(cmd->tmp_pool, "size ", cmd->argv[1], units,

+      "exceeds max size ", max, NULL));

+  }

+

+  /* Possible truncation here, but only for an absurdly large size. */

+  size = (size_t) nbytes;

   c = add_config_param(cmd->argv[0], 1, NULL);

-  c->argv[0] = pcalloc(c->pool, sizeof(int));

-  *((int *) c->argv[0]) = size;

+  c->argv[0] = pcalloc(c->pool, sizeof(size_t));

+  *((size_t *) c->argv[0]) = size;

   return PR_HANDLED(cmd);

 }

diff --git a/src/main.c b/src/main.c

index 3e6d637..660e14b 100644

--- a/src/main.c

+++ b/src/main.c

@@ -466,42 +466,21 @@ static int _dispatch(cmd_rec *cmd, int cmd_type, int validate, char *match) {

   return success;

 }

-/* Returns the appropriate maximum buffer length to use for FTP commands

- * from the client, taking the CommandBufferSize directive into account.

+/* Returns the appropriate maximum buffer size to use for FTP commands

+ * from the client.

  */

-static long get_max_cmd_len(size_t buflen) {

-  long res;

-  int *bufsz = NULL;

-  size_t default_cmd_bufsz;

-

-  /* It's possible for the admin to select a PR_TUNABLE_BUFFER_SIZE which

-   * is smaller than PR_DEFAULT_CMD_BUFSZ.  We need to handle such cases

-   * properly.

-   */

-  default_cmd_bufsz = PR_DEFAULT_CMD_BUFSZ;

-  if (default_cmd_bufsz > buflen) {

-    default_cmd_bufsz = buflen;

-  }

- 

+static size_t get_max_cmd_sz(void) {

+  size_t res;

+  size_t *bufsz = NULL;

+

   bufsz = get_param_ptr(main_server->conf, "CommandBufferSize", FALSE);

   if (bufsz == NULL) {

-    res = default_cmd_bufsz;

-

-  } else if (*bufsz <= 0) {

-    pr_log_pri(PR_LOG_WARNING, "invalid CommandBufferSize size (%d) given, "

-      "using default buffer size (%lu) instead", *bufsz,

-      (unsigned long) default_cmd_bufsz);

-    res = default_cmd_bufsz;

-

-  } else if (*bufsz + 1 > buflen) {

-    pr_log_pri(PR_LOG_WARNING, "invalid CommandBufferSize size (%d) given, "

-      "using default buffer size (%lu) instead", *bufsz,

-      (unsigned long) default_cmd_bufsz);

-    res = default_cmd_bufsz;

+    res = PR_DEFAULT_CMD_BUFSZ;

   } else {

-    pr_log_debug(DEBUG1, "setting CommandBufferSize to %d", *bufsz);

-    res = (long) *bufsz;

+    pr_log_debug(DEBUG1, "setting CommandBufferSize to %lu",

+      (unsigned long) *bufsz);

+    res = *bufsz;

   }

   return res;

@@ -509,21 +488,29 @@ static long get_max_cmd_len(size_t buflen) {

 int pr_cmd_read(cmd_rec **res) {

   static long cmd_bufsz = -1;

-  char buf[PR_DEFAULT_CMD_BUFSZ+1] = {'\0'};

+  static char *cmd_buf = NULL;

   char *cp;

-  size_t buflen;

+  size_t cmd_buflen;

   if (res == NULL) {

     errno = EINVAL;

     return -1;

   }

+  if (cmd_bufsz == -1) {

+    cmd_bufsz = get_max_cmd_sz();

+  }

+

+  if (cmd_buf == NULL) {

+    cmd_buf = pcalloc(session.pool, cmd_bufsz + 1);

+  }

+

   while (TRUE) {

     pr_signals_handle();

-    memset(buf, '\0', sizeof(buf));

+    memset(cmd_buf, '\0', cmd_bufsz);

-    if (pr_netio_telnet_gets(buf, sizeof(buf)-1, session.c->instrm,

+    if (pr_netio_telnet_gets(cmd_buf, cmd_bufsz, session.c->instrm,

         session.c->outstrm) == NULL) {

       if (errno == E2BIG) {

@@ -544,9 +531,6 @@ int pr_cmd_read(cmd_rec **res) {

     break;

   }

-  if (cmd_bufsz == -1)

-    cmd_bufsz = get_max_cmd_len(sizeof(buf));

-

   /* This strlen(3) is guaranteed to terminate; the last byte of buf is

    * always NUL, since pr_netio_telnet_gets() is told that the buf size is

    * one byte less than it really is.

@@ -554,26 +538,28 @@ int pr_cmd_read(cmd_rec **res) {

    * If the strlen(3) says that the length is less than the cmd_bufsz, then

    * there is no need to truncate the buffer by inserting a NUL.

    */

-  buflen = strlen(buf);

-  if (buflen > (cmd_bufsz - 1)) {

+  cmd_buflen = strlen(cmd_buf);

+  if (cmd_buflen > cmd_bufsz) {

     pr_log_debug(DEBUG0, "truncating incoming command length (%lu bytes) to "

       "CommandBufferSize %lu; use the CommandBufferSize directive to increase "

-      "the allowed command length", (unsigned long) buflen,

+      "the allowed command length", (unsigned long) cmd_buflen,

       (unsigned long) cmd_bufsz);

-    buf[cmd_bufsz - 1] = '\0';

+    cmd_buf[cmd_bufsz-1] = '\0';

   }

-  if (buflen &&

-      (buf[buflen-1] == '\n' || buf[buflen-1] == '\r')) {

-    buf[buflen-1] = '\0';

-    buflen--;

+  if (cmd_buflen &&

+      (cmd_buf[cmd_buflen-1] == '\n' || cmd_buf[cmd_buflen-1] == '\r')) {

+    cmd_buf[cmd_buflen-1] = '\0';

+    cmd_buflen--;

-    if (buflen &&

-        (buf[buflen-1] == '\n' || buf[buflen-1] =='\r'))

-      buf[buflen-1] = '\0';

+    if (cmd_buflen &&

+        (cmd_buf[cmd_buflen-1] == '\n' || cmd_buf[cmd_buflen-1] =='\r')) {

+      cmd_buf[cmd_buflen-1] = '\0';

+      cmd_buflen--;

+    }

   }

-  cp = buf;

+  cp = cmd_buf;

   if (*cp == '\r')

     cp++;

@@ -587,11 +573,11 @@ int pr_cmd_read(cmd_rec **res) {

      * command handlers themselves, via cmd->arg.  This small hack

      * reduces the burden on SITE module developers, however.

      */

-    if (strncasecmp(cp, C_SITE, 4) == 0)

+    if (strncasecmp(cp, C_SITE, 4) == 0) {

       flags |= PR_STR_FL_PRESERVE_WHITESPACE;

+    }

     cmd = make_ftp_cmd(session.pool, cp, flags);

-

     if (cmd) {

       *res = cmd;

     } 

diff --git a/src/str.c b/src/str.c

index d243a17..4f327bf 100644

--- a/src/str.c

+++ b/src/str.c

@@ -367,6 +367,81 @@ char *pr_str_strip_end(char *s, char *ch) {

   return s;

 }

+int pr_str_get_nbytes(const char *str, const char *units, off_t *nbytes) {

+  off_t sz;

+  char *ptr = NULL;

+  float factor = 0.0;

+

+  if (str == NULL) {

+    errno = EINVAL;

+    return -1;

+  }

+

+  /* No negative numbers. */

+  if (*str == '-') {

+    errno = EINVAL;

+    return -1;

+  }

+

+  if (units == NULL ||

+      *units == '\0') {

+    factor = 1.0;

+

+  } else if (strncasecmp(units, "KB", 3) == 0) {

+    factor = 1024.0;

+

+  } else if (strncasecmp(units, "MB", 3) == 0) {

+    factor = 1024.0 * 1024.0;

+

+  } else if (strncasecmp(units, "GB", 3) == 0) {

+    factor = 1024.0 * 1024.0 * 1024.0;

+

+  } else if (strncasecmp(units, "TB", 3) == 0) {

+    factor = 1024.0 * 1024.0 * 1024.0 * 1024.0;

+

+  } else if (strncasecmp(units, "B", 2) == 0) {

+    factor = 1.0;

+

+  } else {

+    errno = EINVAL;

+    return -1;

+  }

+

+  errno = 0;

+

+#ifdef HAVE_STRTOULL

+  sz = strtoull(str, &ptr, 10);

+#else

+  sz = strtoul(str, &ptr, 10);

+#endif /* !HAVE_STRTOULL */

+

+  if (errno == ERANGE) {

+    return -1;

+  }

+

+  if (ptr != NULL && *ptr) {

+    /* Error parsing the given string */

+    errno = EINVAL;

+    return -1;

+  }

+

+  /* Don't bother applying the factor if the result will overflow the result. */

+#ifdef ULLONG_MAX

+  if (sz > (ULLONG_MAX / factor)) {

+#else

+  if (sz > (ULONG_MAX / factor)) {

+#endif /* !ULLONG_MAX */

+    errno = ERANGE;

+    return -1;

+  }

+

+  if (nbytes != NULL) {

+    *nbytes = (off_t) (sz * factor);

+  }

+

+  return 0;

+}

+

 char *pr_str_get_word(char **cp, int flags) {

   char *res, *dst;

   char quote_mode = 0;

diff --git a/tests/t/lib/ProFTPD/Tests/Config/CommandBufferSize.pm b/tests/t/lib/ProFTPD/Tests/Config/CommandBufferSize.pm

index ed4672a..a57c898 100644

--- a/tests/t/lib/ProFTPD/Tests/Config/CommandBufferSize.pm

+++ b/tests/t/lib/ProFTPD/Tests/Config/CommandBufferSize.pm

@@ -94,6 +94,8 @@ sub cmdbuffersz_small {

     die("Can't open $test_path: $!");

   }

+  my $idle_timeout = 3;

+

   my $config = {

     PidFile => $pid_file,

     ScoreboardFile => $scoreboard_file,

@@ -103,6 +105,7 @@ sub cmdbuffersz_small {

     AuthGroupFile => $auth_group_file,

     CommandBufferSize => $cmdbufsz,

+    TimeoutIdle => $idle_timeout,

     IfModules => {

       'mod_delay.c' => {

@@ -128,44 +131,16 @@ sub cmdbuffersz_small {

   defined(my $pid = fork()) or die("Can't fork: $!");

   if ($pid) {

     eval {

-      my $client = ProFTPD::TestSuite::FTP->new('127.0.0.1', $port);

+      my $client = ProFTPD::TestSuite::FTP->new('127.0.0.1', $port, 0, 1);

       $client->login($user, $passwd);

-      my $conn = $client->list_raw($test_file);

-      unless ($conn) {

-        die("Failed to LIST $test_file: " . $client->response_code() . " " .

-          $client->response_msg());

+      # Since our filename is longer than the CommandBufferSize, proftpd

+      # should simply ignore this.  It will fail because of the idle timeout

+      # first.

+      eval { $client->stat($test_file) };

+      unless ($@) {

+        die("STAT command succeeded unexpectedly");

       }

-

-      my $buf;

-      $conn->read($buf, 8192, 30);

-      eval { $conn->close() };

-

-      my $resp_code = $client->response_code();

-      my $resp_msg = $client->response_msg();

-

-      # CommandBufferSize works by truncating any input longer than the

-      # configured length.  (It should arguably reject such longer input,

-      # but that is a different consideration.)

-      #

-      # Since our file name is longer than the CommandBufferSize, it means

-      # the path will be truncated, and the LIST should return 450.

-

-      my $expected;

-

-      $expected = 450;

-      $self->assert($expected == $resp_code,

-        test_msg("Expected $expected, got $resp_code"));

-

-      # This length is CommandBufferSize - "LIST"(4) - " "(1) - 1 for

-      # the NUL reserved in the code.  Thus CommandBufferSize - 6.

-      my $truncated_name = ("A" x ($cmdbufsz - 6));

-

-      $expected = "$truncated_name: No such file or directory";

-      $self->assert($expected eq $resp_msg,

-        test_msg("Expected '$expected', got '$resp_msg'"));

-

-      $client->quit();

     };

     if ($@) {

@@ -176,7 +151,7 @@ sub cmdbuffersz_small {

     $wfh->flush();

   } else {

-    eval { server_wait($config_file, $rfh) };

+    eval { server_wait($config_file, $rfh, 15) };

     if ($@) {

       warn($@);

       exit 1;