components/proftpd/patches/15825705.patch
author Craig Mohrman <craig.mohrman@oracle.com>
Fri, 04 Apr 2014 21:11:46 -0700
branchs11-update
changeset 3049 64dc23b0ff81
parent 2966 93b92606f5e1
permissions -rw-r--r--
16858016 php 5.2 and php 5.3 are getting tpnos

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;