From ec60588d36f06acdd6a1b3c5959d2102f4f5b230 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Ond=C5=99ej=20Lyson=C4=9Bk?= <olysonek@redhat.com>

Date: Tue, 6 Feb 2018 13:30:44 +0100

Subject: [PATCH] Add new filename generation algorithm for STOU command

A new configuration option 'better_stou' can be used to enable

a better algorithm for generating unique filenames.

Resolves: rhbz#1479237

---

 parseconf.c   |   1 +

 postlogin.c   | 177 +++++++++++++++++++++++++++++++++++++++++++++++++---------

 sysutil.c     |   3 +

 sysutil.h     |   3 +-

 tunables.c    |   2 +

 tunables.h    |   3 +

 vsftpd.conf.5 |  10 ++++

 7 files changed, 172 insertions(+), 27 deletions(-)

diff --git a/parseconf.c b/parseconf.c

index 33a1349..47b54f1 100644

--- a/parseconf.c

+++ b/parseconf.c

@@ -111,6 +111,7 @@ parseconf_bool_array[] =

   { "http_enable", &tunable_http_enable },

   { "seccomp_sandbox", &tunable_seccomp_sandbox },

   { "allow_writeable_chroot", &tunable_allow_writeable_chroot },

+  { "better_stou", &tunable_better_stou },

   { 0, 0 }

 };

diff --git a/postlogin.c b/postlogin.c

index 869c2a2..3b50af2 100644

--- a/postlogin.c

+++ b/postlogin.c

@@ -29,6 +29,7 @@

 #include "opts.h"

 #include <errno.h>

+#include <stdio.h>

 /* Private local functions */

 static void handle_pwd(struct vsf_session* p_sess);

@@ -1021,6 +1022,115 @@ handle_stor(struct vsf_session* p_sess)

   handle_upload_common(p_sess, 0, 0);

 }

+/* Based on __gen_tempname() from glibc - thanks, glibc! Relicensed

+ * from LGPL2.1+ to GPL2.

+ */

+static int

+create_unique_file(struct vsf_session* p_sess, struct mystr* p_outstr,

+                   const struct mystr* p_base_str,

+                   int (*access_checker)(const struct mystr*))

+{

+  struct mystr s_result = INIT_MYSTR;

+  const int suffix_len = 6;

+  unsigned int count;

+  static unsigned long long int value;

+  unsigned long long int random_time_bits;

+  int fd = -1;

+  /* These are the characters used in temporary file names.  */

+  struct mystr s_letters = INIT_MYSTR;

+  unsigned int s_letters_len;

+  int base_len;

+

+  str_alloc_text(&s_letters,

+      "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789");

+  s_letters_len = str_getlen(&s_letters);

+

+  /* A lower bound on the number of temporary files to attempt to

+     generate.  The maximum total number of temporary file names that

+     can exist for a given template is 62**6.  It should never be

+     necessary to try all of these combinations.  Instead if a reasonable

+     number of names is tried (we define reasonable as 62**3) fail to

+     give the system administrator the chance to remove the problems.  */

+#define ATTEMPTS_MIN (62 * 62 * 62)

+

+  /* The number of times to attempt to generate a temporary file. */

+#if ATTEMPTS_MIN < TMP_MAX

+  unsigned int attempts = TMP_MAX;

+#else

+  unsigned int attempts = ATTEMPTS_MIN;

+#endif

+#undef ATTEMPTS_MIN

+

+  {

+    long sec = vsf_sysutil_get_time_sec();

+    long usec = vsf_sysutil_get_time_usec();

+    random_time_bits = ((unsigned long long int) usec << 16) ^ sec;

+    value += random_time_bits ^ vsf_sysutil_getpid();

+  }

+

+  if (str_isempty(p_base_str))

+  {

+    const char *base = "STOU.";

+    base_len = vsf_sysutil_strlen(base);

+    str_reserve(&s_result, base_len + suffix_len);

+    str_alloc_text(&s_result, base);

+  }

+  else

+  {

+    str_reserve(&s_result, str_getlen(p_base_str) + suffix_len + 1);

+    str_copy(&s_result, p_base_str);

+    str_append_char(&s_result, '.');

+    base_len = str_getlen(&s_result);

+  }

+

+  for (count = 0; count < attempts; value += 7777, ++count)

+  {

+    unsigned long long v = value;

+    int i;

+    str_trunc(&s_result, base_len);

+    for (i = 0; i < suffix_len; ++i)

+    {

+      char c;

+      c = str_get_char_at(&s_letters, v % s_letters_len);

+      v /= s_letters_len;

+      str_append_char(&s_result, c);

+    }

+    if (!access_checker(&s_result))

+    {

+      /* If we generate a filename which is not allowed, we fail immediatelly,

+       * without trying any other possibilities. This is to prevent attackers

+       * from keeping us busy.

+       */

+      vsf_cmdio_write(p_sess, FTP_NOPERM, "Permission denied.");

+      break;

+    }

+    fd = str_create_exclusive(&s_result);

+    if (vsf_sysutil_retval_is_error(fd))

+    {

+      if (kVSFSysUtilErrEXIST == vsf_sysutil_get_error())

+      {

+        continue;

+      }

+      else

+      {

+        vsf_cmdio_write(p_sess, FTP_UPLOADFAIL, "Could not create file.");

+        break;

+      }

+    }

+    else

+    {

+      break;

+    }

+  }

+  if (!vsf_sysutil_retval_is_error(fd))

+  {

+    str_copy(p_outstr, &s_result);

+  }

+  str_free(&s_letters);

+  str_free(&s_result);

+  return fd;

+}

+

 static void

 handle_upload_common(struct vsf_session* p_sess, int is_append, int is_unique)

 {

@@ -1042,41 +1152,56 @@ handle_upload_common(struct vsf_session* p_sess, int is_append, int is_unique)

     return;

   }

   resolve_tilde(&p_sess->ftp_arg_str, p_sess);

-  p_filename = &p_sess->ftp_arg_str;

-  if (is_unique)

-  {

-    get_unique_filename(&s_filename, p_filename);

-    p_filename = &s_filename;

-  }

   vsf_log_start_entry(p_sess, kVSFLogEntryUpload);

   str_copy(&p_sess->log_str, &p_sess->ftp_arg_str);

   prepend_path_to_filename(&p_sess->log_str);

-  if (!vsf_access_check_file(p_filename))

-  {

-    vsf_cmdio_write(p_sess, FTP_NOPERM, "Permission denied.");

-    return;

-  }

-  /* NOTE - actual file permissions will be governed by the tunable umask */

-  /* XXX - do we care about race between create and chown() of anonymous

-   * upload?

-   */

-  if (is_unique || (p_sess->is_anonymous && !tunable_anon_other_write_enable))

+  p_filename = &p_sess->ftp_arg_str;

+  if (is_unique && tunable_better_stou)

   {

-    new_file_fd = str_create_exclusive(p_filename);

+    new_file_fd = create_unique_file(p_sess, &s_filename, p_filename,

+                                     vsf_access_check_file);

+    if (vsf_sysutil_retval_is_error(new_file_fd))

+    {

+      return;

+    }

+    p_filename = &s_filename;

   }

   else

   {

-    /* For non-anonymous, allow open() to overwrite or append existing files */

-    new_file_fd = str_create(p_filename);

-    if (!is_append && offset == 0)

+    if (is_unique)

     {

-      do_truncate = 1;

+      get_unique_filename(&s_filename, p_filename);

+      p_filename = &s_filename;

+    }

+    if (!vsf_access_check_file(p_filename))

+    {

+      vsf_cmdio_write(p_sess, FTP_NOPERM, "Permission denied.");

+      return;

+    }

+    /* NOTE - actual file permissions will be governed by the tunable umask */

+    /* XXX - do we care about race between create and chown() of anonymous

+     * upload?

+     */

+    if (is_unique || (p_sess->is_anonymous && !tunable_anon_other_write_enable))

+    {

+      new_file_fd = str_create_exclusive(p_filename);

+    }

+    else

+    {

+      /* For non-anonymous, allow open() to overwrite or append existing

+       * files

+       */

+      new_file_fd = str_create(p_filename);

+      if (!is_append && offset == 0)

+      {

+        do_truncate = 1;

+      }

+    }

+    if (vsf_sysutil_retval_is_error(new_file_fd))

+    {

+      vsf_cmdio_write(p_sess, FTP_UPLOADFAIL, "Could not create file.");

+      return;

     }

-  }

-  if (vsf_sysutil_retval_is_error(new_file_fd))

-  {

-    vsf_cmdio_write(p_sess, FTP_UPLOADFAIL, "Could not create file.");

-    return;

   }

   created = 1;

   vsf_sysutil_fstat(new_file_fd, &s_p_statbuf);

diff --git a/sysutil.c b/sysutil.c

index e97f3bd..e6d88dc 100644

--- a/sysutil.c

+++ b/sysutil.c

@@ -1663,6 +1663,9 @@ vsf_sysutil_get_error(void)

     case ENOENT:

       retval = kVSFSysUtilErrNOENT;

       break;

+    case EEXIST:

+      retval = kVSFSysUtilErrEXIST;

+      break;

     default:

       break;

   }

diff --git a/sysutil.h b/sysutil.h

index b745d4a..15d4020 100644

--- a/sysutil.h

+++ b/sysutil.h

@@ -18,7 +18,8 @@ enum EVSFSysUtilError

   kVSFSysUtilErrINVAL,

   kVSFSysUtilErrOPNOTSUPP,

   kVSFSysUtilErrACCES,

-  kVSFSysUtilErrNOENT

+  kVSFSysUtilErrNOENT,

+  kVSFSysUtilErrEXIST

 };

 enum EVSFSysUtilError vsf_sysutil_get_error(void);

diff --git a/tunables.c b/tunables.c

index 9680528..5ec2bdc 100644

--- a/tunables.c

+++ b/tunables.c

@@ -92,6 +92,7 @@ int tunable_ftp_enable;

 int tunable_http_enable;

 int tunable_seccomp_sandbox;

 int tunable_allow_writeable_chroot;

+int tunable_better_stou;

 unsigned int tunable_accept_timeout;

 unsigned int tunable_connect_timeout;

@@ -239,6 +240,7 @@ tunables_load_defaults()

   tunable_http_enable = 0;

   tunable_seccomp_sandbox = 0;

   tunable_allow_writeable_chroot = 0;

+  tunable_better_stou = 0;

   tunable_accept_timeout = 60;

   tunable_connect_timeout = 60;

diff --git a/tunables.h b/tunables.h

index a466427..85ea1a8 100644

--- a/tunables.h

+++ b/tunables.h

@@ -93,6 +93,9 @@ extern int tunable_ftp_enable;                /* Allow FTP protocol */

 extern int tunable_http_enable;               /* Allow HTTP protocol */

 extern int tunable_seccomp_sandbox;           /* seccomp filter sandbox */

 extern int tunable_allow_writeable_chroot;    /* Allow misconfiguration */

+extern int tunable_better_stou;               /* Use better file name generation

+                                               * algorithm for the STOU command

+					       */

 /* Integer/numeric defines */

 extern unsigned int tunable_accept_timeout;

diff --git a/vsftpd.conf.5 b/vsftpd.conf.5

index 43b0435..e9ae474 100644

--- a/vsftpd.conf.5

+++ b/vsftpd.conf.5

@@ -65,6 +65,16 @@ creates an 'etc' directory in the new root directory, they could potentially

 trick the C library into loading a user-created configuration file from the

 /etc/ directory.

+Default: NO

+.TP

+.B better_stou

+Use a better file name generation algorithm for the STOU command. The default

+original algorithm simply adds an increasing number suffix to the file name,

+which is prone to race conditions if multiple uploaders use the STOU command

+with the same file name simultaneously, which can result in failure of the

+command. The new algorithm adds a unique random six character suffix to

+the file name, which works much better in face of concurrent uploads.

+

 Default: NO

 .TP

 .B anon_mkdir_write_enable

-- 

2.14.3